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

Whitespace in commits #14650

Merged
merged 6 commits into from
Feb 13, 2021
Merged
Show file tree
Hide file tree
Changes from all 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
7 changes: 4 additions & 3 deletions routers/repo/commit.go
Original file line number Diff line number Diff line change
Expand Up @@ -304,11 +304,12 @@ func Diff(ctx *context.Context) {
ctx.Data["CommitStatus"] = models.CalcCommitStatus(statuses)
ctx.Data["CommitStatuses"] = statuses

diff, err := gitdiff.GetDiffCommit(repoPath,
diff, err := gitdiff.GetDiffCommitWithWhitespaceBehavior(repoPath,
commitID, setting.Git.MaxGitDiffLines,
setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles)
setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles,
gitdiff.GetWhitespaceFlag(ctx.Data["WhitespaceBehavior"].(string)))
if err != nil {
ctx.NotFound("GetDiffCommit", err)
ctx.NotFound("GetDiffCommitWithWhitespaceBehavior", err)
return
}

Expand Down
13 changes: 8 additions & 5 deletions routers/repo/compare.go
Original file line number Diff line number Diff line change
Expand Up @@ -411,7 +411,8 @@ func PrepareCompareDiff(
headRepo *models.Repository,
headGitRepo *git.Repository,
compareInfo *git.CompareInfo,
baseBranch, headBranch string) bool {
baseBranch, headBranch string,
whitespaceBehavior string) bool {

var (
repo = ctx.Repo.Repository
Expand Down Expand Up @@ -442,11 +443,11 @@ func PrepareCompareDiff(
return true
}

diff, err := gitdiff.GetDiffRange(models.RepoPath(headUser.Name, headRepo.Name),
diff, err := gitdiff.GetDiffRangeWithWhitespaceBehavior(models.RepoPath(headUser.Name, headRepo.Name),
compareInfo.MergeBase, headCommitID, setting.Git.MaxGitDiffLines,
setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles)
setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, whitespaceBehavior)
if err != nil {
ctx.ServerError("GetDiffRange", err)
ctx.ServerError("GetDiffRangeWithWhitespaceBehavior", err)
return false
}
ctx.Data["Diff"] = diff
Expand Down Expand Up @@ -530,12 +531,14 @@ func getBranchesForRepo(user *models.User, repo *models.Repository) (bool, []str
// CompareDiff show different from one commit to another commit
func CompareDiff(ctx *context.Context) {
headUser, headRepo, headGitRepo, compareInfo, baseBranch, headBranch := ParseCompareInfo(ctx)

if ctx.Written() {
return
}
defer headGitRepo.Close()

nothingToCompare := PrepareCompareDiff(ctx, headUser, headRepo, headGitRepo, compareInfo, baseBranch, headBranch)
nothingToCompare := PrepareCompareDiff(ctx, headUser, headRepo, headGitRepo, compareInfo, baseBranch, headBranch,
gitdiff.GetWhitespaceFlag(ctx.Data["WhitespaceBehavior"].(string)))
if ctx.Written() {
return
}
Expand Down
14 changes: 5 additions & 9 deletions routers/repo/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -581,12 +581,6 @@ 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 @@ -629,7 +623,7 @@ func ViewPullFiles(ctx *context.Context) {
diff, err := gitdiff.GetDiffRangeWithWhitespaceBehavior(diffRepoPath,
startCommitID, endCommitID, setting.Git.MaxGitDiffLines,
setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles,
whitespaceFlags[ctx.Data["WhitespaceBehavior"].(string)])
gitdiff.GetWhitespaceFlag(ctx.Data["WhitespaceBehavior"].(string)))
if err != nil {
ctx.ServerError("GetDiffRangeWithWhitespaceBehavior", err)
return
Expand Down Expand Up @@ -993,7 +987,8 @@ func CompareAndPullRequestPost(ctx *context.Context) {

// This stage is already stop creating new pull request, so it does not matter if it has
// something to compare or not.
PrepareCompareDiff(ctx, headUser, headRepo, headGitRepo, prInfo, baseBranch, headBranch)
PrepareCompareDiff(ctx, headUser, headRepo, headGitRepo, prInfo, baseBranch, headBranch,
gitdiff.GetWhitespaceFlag(ctx.Data["WhitespaceBehavior"].(string)))
if ctx.Written() {
return
}
Expand All @@ -1003,7 +998,8 @@ func CompareAndPullRequestPost(ctx *context.Context) {
}

if util.IsEmptyString(form.Title) {
PrepareCompareDiff(ctx, headUser, headRepo, headGitRepo, prInfo, baseBranch, headBranch)
PrepareCompareDiff(ctx, headUser, headRepo, headGitRepo, prInfo, baseBranch, headBranch,
gitdiff.GetWhitespaceFlag(ctx.Data["WhitespaceBehavior"].(string)))
if ctx.Written() {
return
}
Expand Down
8 changes: 4 additions & 4 deletions routers/routes/web.go
Original file line number Diff line number Diff line change
Expand Up @@ -695,8 +695,8 @@ func RegisterRoutes(m *web.Route) {
m.Get("/{id}", repo.MilestoneIssuesAndPulls)
}, reqRepoIssuesOrPullsReader, context.RepoRef())
m.Combo("/compare/*", repo.MustBeNotEmpty, reqRepoCodeReader, repo.SetEditorconfigIfExists).
Get(ignSignIn, repo.SetDiffViewStyle, repo.CompareDiff).
Post(reqSignIn, context.RepoMustNotBeArchived(), reqRepoPullsReader, repo.MustAllowPulls, bindIgnErr(auth.CreateIssueForm{}), repo.CompareAndPullRequestPost)
Get(ignSignIn, repo.SetDiffViewStyle, repo.SetWhitespaceBehavior, repo.CompareDiff).
Post(reqSignIn, context.RepoMustNotBeArchived(), reqRepoPullsReader, repo.MustAllowPulls, bindIgnErr(auth.CreateIssueForm{}), repo.SetWhitespaceBehavior, repo.CompareAndPullRequestPost)
}, context.RepoAssignment(), context.UnitTypes())

// Grouping for those endpoints that do require authentication
Expand Down Expand Up @@ -885,7 +885,7 @@ func RegisterRoutes(m *web.Route) {
m.Get("/{page}", repo.Wiki)
m.Get("/_pages", repo.WikiPages)
m.Get("/{page}/_revision", repo.WikiRevision)
m.Get("/commit/{sha:[a-f0-9]{7,40}}", repo.SetEditorconfigIfExists, repo.SetDiffViewStyle, repo.Diff)
m.Get("/commit/{sha:[a-f0-9]{7,40}}", repo.SetEditorconfigIfExists, repo.SetDiffViewStyle, repo.SetWhitespaceBehavior, repo.Diff)
m.Get("/commit/{sha:[a-f0-9]{7,40}}.{:patch|diff}", repo.RawDiff)

m.Group("", func() {
Expand Down Expand Up @@ -977,7 +977,7 @@ func RegisterRoutes(m *web.Route) {

m.Group("", func() {
m.Get("/graph", repo.Graph)
m.Get("/commit/{sha:([a-f0-9]{7,40})$}", repo.SetEditorconfigIfExists, repo.SetDiffViewStyle, repo.Diff)
m.Get("/commit/{sha:([a-f0-9]{7,40})$}", repo.SetEditorconfigIfExists, repo.SetDiffViewStyle, repo.SetWhitespaceBehavior, repo.Diff)
}, repo.MustBeNotEmpty, context.RepoRef(), reqRepoCodeReader)

m.Group("/src", func() {
Expand Down
19 changes: 18 additions & 1 deletion services/gitdiff/gitdiff.go
Original file line number Diff line number Diff line change
Expand Up @@ -967,7 +967,13 @@ func GetDiffRangeWithWhitespaceBehavior(repoPath, beforeCommitID, afterCommitID

// GetDiffCommit builds a Diff representing the given commitID.
func GetDiffCommit(repoPath, commitID string, maxLines, maxLineCharacters, maxFiles int) (*Diff, error) {
return GetDiffRange(repoPath, "", commitID, maxLines, maxLineCharacters, maxFiles)
return GetDiffRangeWithWhitespaceBehavior(repoPath, "", commitID, maxLines, maxLineCharacters, maxFiles, "")
}

// GetDiffCommitWithWhitespaceBehavior builds a Diff representing the given commitID.
// The whitespaceBehavior is either an empty string or a git flag
func GetDiffCommitWithWhitespaceBehavior(repoPath, commitID string, maxLines, maxLineCharacters, maxFiles int, whitespaceBehavior string) (*Diff, error) {
return GetDiffRangeWithWhitespaceBehavior(repoPath, "", commitID, maxLines, maxLineCharacters, maxFiles, whitespaceBehavior)
}

// CommentAsDiff returns c.Patch as *Diff
Expand Down Expand Up @@ -995,3 +1001,14 @@ func CommentMustAsDiff(c *models.Comment) *Diff {
}
return diff
}

// GetWhitespaceFlag returns git diff flag for treating whitespaces
func GetWhitespaceFlag(whiteSpaceBehavior string) string {
whitespaceFlags := map[string]string{
"ignore-all": "-w",
"ignore-change": "-b",
"ignore-eol": "--ignore-space-at-eol",
"": ""}

return whitespaceFlags[whiteSpaceBehavior]
}
12 changes: 2 additions & 10 deletions templates/repo/diff/box.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,7 @@
<div class="diff-detail-box diff-box 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}}
{{template "repo/diff/whitespace_dropdown" .}}
{{template "repo/diff/options_dropdown" .}}
{{if and .PageIsPullFiles $.SignedUserID (not .IsArchived)}}
{{template "repo/diff/new_review" .}}
Expand All @@ -22,11 +18,7 @@
{{svg "octicon-diff" 16 "mr-2"}}{{.i18n.Tr "repo.diff.stats_desc" .Diff.NumFiles .Diff.TotalAddition .Diff.TotalDeletion | Str2html}}
</div>
<div class="diff-detail-actions df ac">
{{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}}
{{template "repo/diff/whitespace_dropdown" .}}
{{template "repo/diff/options_dropdown" .}}
{{if and .PageIsPullFiles $.SignedUserID (not .IsArchived)}}
{{template "repo/diff/new_review" .}}
Expand Down