Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add whitespace handling to PR-comparsion #4683

Merged
merged 5 commits into from
Aug 14, 2018
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 21 additions & 10 deletions models/git_diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -633,6 +633,13 @@ func ParsePatch(maxLines, maxLineCharacters, maxFiles int, reader io.Reader) (*D
// passing the empty string as beforeCommitID returns a diff from the
// parent commit.
func GetDiffRange(repoPath, beforeCommitID, afterCommitID string, maxLines, maxLineCharacters, maxFiles int) (*Diff, error) {
return GetDiffRangeWithWhitespaceBehavior(repoPath, beforeCommitID, afterCommitID, maxLines, maxLineCharacters, maxFiles, "")
}

// GetDiffRangeWithWhitespaceBehavior builds a Diff between two commits of a repository.
// Passing the empty string as beforeCommitID returns a diff from the parent commit.
// The whitespaceBehavior is either an empty string or a git flag
func GetDiffRangeWithWhitespaceBehavior(repoPath, beforeCommitID, afterCommitID string, maxLines, maxLineCharacters, maxFiles int, whitespaceBehavior string) (*Diff, error) {
gitRepo, err := git.OpenRepository(repoPath)
if err != nil {
return nil, err
Expand All @@ -644,17 +651,21 @@ func GetDiffRange(repoPath, beforeCommitID, afterCommitID string, maxLines, maxL
}

var cmd *exec.Cmd
// if "after" commit given
if len(beforeCommitID) == 0 {
// First commit of repository.
if commit.ParentCount() == 0 {
cmd = exec.Command("git", "show", afterCommitID)
} else {
c, _ := commit.Parent(0)
cmd = exec.Command("git", "diff", "-M", c.ID.String(), afterCommitID)
}
if len(beforeCommitID) == 0 && commit.ParentCount() == 0 {
cmd = exec.Command("git", "show", afterCommitID)
} else {
cmd = exec.Command("git", "diff", "-M", beforeCommitID, afterCommitID)
actualBeforeCommitID := beforeCommitID
if len(actualBeforeCommitID) == 0 {
parentCommit, _ := commit.Parent(0)
actualBeforeCommitID = parentCommit.ID.String()
}
diffArgs := []string{"diff", "-M"}
if len(whitespaceBehavior) != 0 {
diffArgs = append(diffArgs, whitespaceBehavior)
}
diffArgs = append(diffArgs, actualBeforeCommitID)
diffArgs = append(diffArgs, afterCommitID)
cmd = exec.Command("git", diffArgs...)
}
cmd.Dir = repoPath
cmd.Stderr = os.Stderr
Expand Down
5 changes: 5 additions & 0 deletions options/locale/locale_en-US.ini
Original file line number Diff line number Diff line change
Expand Up @@ -1149,6 +1149,11 @@ diff.data_not_available = Diff Content Not Available
diff.show_diff_stats = Show Diff Stats
diff.show_split_view = Split View
diff.show_unified_view = Unified View
diff.whitespace_button = Whitespace
diff.whitespace_show_everything = Show all changes
diff.whitespace_ignore_all_whitespace = Ignore whitespace when comparing lines
diff.whitespace_ignore_amount_changes = Ignore changes in amount of whitespace
diff.whitespace_ignore_at_eol = Ignore changes in whitespace at EOL
diff.stats_desc = <strong> %d changed files</strong> with <strong>%d additions</strong> and <strong>%d deletions</strong>
diff.bin = BIN
diff.view_file = View File
Expand Down
11 changes: 11 additions & 0 deletions routers/repo/middlewares.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,3 +50,14 @@ func SetDiffViewStyle(ctx *context.Context) {
ctx.ServerError("ErrUpdateDiffViewStyle", err)
}
}

// SetWhitespaceBehavior set whitespace behavior as render variable
func SetWhitespaceBehavior(ctx *context.Context) {
whitespaceBehavior := ctx.Query("whitespace")
switch whitespaceBehavior {
case "ignore-all", "ignore-eol", "ignore-change":
ctx.Data["WhitespaceBehavior"] = whitespaceBehavior
default:
ctx.Data["WhitespaceBehavior"] = ""
}
}
13 changes: 10 additions & 3 deletions routers/repo/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -384,6 +384,12 @@ func ViewPullFiles(ctx *context.Context) {
}
pull := issue.PullRequest

whitespaceFlags := map[string]string{
"ignore-all": "-w",
"ignore-change": "-b",
"ignore-eol": "--ignore-space-at-eol",
"": ""}

var (
diffRepoPath string
startCommitID string
Expand Down Expand Up @@ -449,11 +455,12 @@ func ViewPullFiles(ctx *context.Context) {
ctx.Data["Reponame"] = pull.HeadRepo.Name
}

diff, err := models.GetDiffRange(diffRepoPath,
diff, err := models.GetDiffRangeWithWhitespaceBehavior(diffRepoPath,
startCommitID, endCommitID, setting.Git.MaxGitDiffLines,
setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles)
setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles,
whitespaceFlags[ctx.Data["WhitespaceBehavior"].(string)])
if err != nil {
ctx.ServerError("GetDiffRange", err)
ctx.ServerError("GetDiffRangeWithWhitespaceBehavior", err)
return
}

Expand Down
2 changes: 1 addition & 1 deletion routers/routes/routes.go
Original file line number Diff line number Diff line change
Expand Up @@ -674,7 +674,7 @@ func RegisterRoutes(m *macaron.Macaron) {
m.Post("/merge", reqRepoWriter, bindIgnErr(auth.MergePullRequestForm{}), repo.MergePullRequest)
m.Post("/cleanup", context.RepoRef(), repo.CleanUpPullRequest)
m.Group("/files", func() {
m.Get("", context.RepoRef(), repo.SetEditorconfigIfExists, repo.SetDiffViewStyle, repo.ViewPullFiles)
m.Get("", context.RepoRef(), repo.SetEditorconfigIfExists, repo.SetDiffViewStyle, repo.SetWhitespaceBehavior, repo.ViewPullFiles)
m.Group("/reviews", func() {
m.Post("/comments", bindIgnErr(auth.CodeCommentForm{}), repo.CreateCodeComment)
m.Post("/submit", bindIgnErr(auth.SubmitReviewForm{}), repo.SubmitReview)
Expand Down
21 changes: 20 additions & 1 deletion templates/repo/diff/box.tmpl
Original file line number Diff line number Diff line change
@@ -1,12 +1,31 @@
{{if .DiffNotAvailable}}
<div class="diff-detail-box diff-box ui sticky">
<div>
<div class="ui right">
{{if .PageIsPullFiles}}
{{template "repo/diff/whitespace_dropdown" .}}
{{else}}
<a class="ui tiny basic toggle button" href="?style={{if .IsSplitStyle}}unified{{else}}split{{end}}">{{ if .IsSplitStyle }}{{.i18n.Tr "repo.diff.show_unified_view"}}{{else}}{{.i18n.Tr "repo.diff.show_split_view"}}{{end}}</a>
{{end}}
<a class="ui tiny basic toggle button" data-target="#diff-files">{{.i18n.Tr "repo.diff.show_diff_stats"}}</a>
{{if and .PageIsPullFiles $.SignedUserID}}
{{template "repo/diff/new_review" .}}
{{end}}
</div>
</div>
</div>
<h4>{{.i18n.Tr "repo.diff.data_not_available"}}</h4>
{{else}}
<div class="diff-detail-box diff-box ui sticky">
<div>
<i class="fa fa-retweet"></i>
{{.i18n.Tr "repo.diff.stats_desc" .Diff.NumFiles .Diff.TotalAddition .Diff.TotalDeletion | Str2html}}
<div class="ui right">
<a class="ui tiny basic toggle button" href="?style={{if .IsSplitStyle}}unified{{else}}split{{end}}">{{ if .IsSplitStyle }}{{.i18n.Tr "repo.diff.show_unified_view"}}{{else}}{{.i18n.Tr "repo.diff.show_split_view"}}{{end}}</a>
{{if .PageIsPullFiles}}
{{template "repo/diff/whitespace_dropdown" .}}
{{else}}
<a class="ui tiny basic toggle button" href="?style={{if .IsSplitStyle}}unified{{else}}split{{end}}">{{ if .IsSplitStyle }}{{.i18n.Tr "repo.diff.show_unified_view"}}{{else}}{{.i18n.Tr "repo.diff.show_split_view"}}{{end}}</a>
{{end}}
<a class="ui tiny basic toggle button" data-target="#diff-files">{{.i18n.Tr "repo.diff.show_diff_stats"}}</a>
{{if and .PageIsPullFiles $.SignedUserID}}
{{template "repo/diff/new_review" .}}
Expand Down
23 changes: 23 additions & 0 deletions templates/repo/diff/whitespace_dropdown.tmpl
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<div class="ui dropdown tiny button">
{{.i18n.Tr "repo.diff.whitespace_button"}}
<i class="dropdown icon"></i>
<div class="menu">
<a class="item" href="?style={{if .IsSplitStyle}}split{{else}}unified{{end}}&whitespace=">
<i class="circle {{ if eq .WhitespaceBehavior "" }}dot{{else}}outline{{end}} icon"></i>
{{.i18n.Tr "repo.diff.whitespace_show_everything"}}
</a>
<a class="item" href="?style={{if .IsSplitStyle}}split{{else}}unified{{end}}&whitespace=ignore-all">
<i class="circle {{ if eq .WhitespaceBehavior "ignore-all" }}dot{{else}}outline{{end}} icon"></i>
{{.i18n.Tr "repo.diff.whitespace_ignore_all_whitespace"}}
</a>
<a class="item" href="?style={{if .IsSplitStyle}}split{{else}}unified{{end}}&whitespace=ignore-change">
<i class="circle {{ if eq .WhitespaceBehavior "ignore-change" }}dot{{else}}outline{{end}} icon"></i>
{{.i18n.Tr "repo.diff.whitespace_ignore_amount_changes"}}
</a>
<a class="item" href="?style={{if .IsSplitStyle}}split{{else}}unified{{end}}&whitespace=ignore-eol">
<i class="circle {{ if eq .WhitespaceBehavior "ignore-eol" }}dot{{else}}outline{{end}} icon"></i>
{{.i18n.Tr "repo.diff.whitespace_ignore_at_eol"}}
</a>
</div>
</div>
<a class="ui tiny basic toggle button" href="?style={{if .IsSplitStyle}}unified{{else}}split{{end}}&whitespace={{$.WhitespaceBehavior}}">{{ if .IsSplitStyle }}{{.i18n.Tr "repo.diff.show_unified_view"}}{{else}}{{.i18n.Tr "repo.diff.show_split_view"}}{{end}}</a>