From 743d22669a49b743f1963dc579294a092c4980c1 Mon Sep 17 00:00:00 2001 From: Andrey Nering Date: Wed, 29 Jun 2016 12:11:00 -0300 Subject: [PATCH] Re-work MAX_DIFF_LINES: supress diff per file, not the whole diff (#3174) --- conf/app.ini | 25 +++-- conf/locale/locale_en-US.ini | 2 + models/git_diff.go | 45 ++++----- modules/setting/setting.go | 8 +- routers/repo/commit.go | 6 +- routers/repo/pull.go | 6 +- templates/repo/diff_box.tmpl | 186 ++++++++++++++++++++--------------- 7 files changed, 160 insertions(+), 118 deletions(-) diff --git a/conf/app.ini b/conf/app.ini index bd6fb9bd6..7f7b32903 100644 --- a/conf/app.ini +++ b/conf/app.ini @@ -12,7 +12,7 @@ RUN_MODE = dev ROOT = SCRIPT_TYPE = bash ; Default ANSI charset -ANSI_CHARSET = +ANSI_CHARSET = ; Force every new repository to be private FORCE_PRIVATE = false ; Global maximum creation limit of repository per user, -1 means no limit @@ -70,7 +70,7 @@ SSH_PORT = 22 ; Port number builtin SSH server listens on SSH_LISTEN_PORT = %(SSH_PORT)s ; Root path of SSH directory, default is '~/.ssh', but you have to use '/home/git/.ssh'. -SSH_ROOT_PATH = +SSH_ROOT_PATH = ; Directory to create temporary files when test publick key using ssh-keygen, ; default is system temporary directory. SSH_KEY_TEST_PATH = @@ -169,18 +169,18 @@ SUBJECT = %(APP_NAME)s ; Gmail: smtp.gmail.com:587 ; QQ: smtp.qq.com:25 ; Note, if the port ends with "465", SMTPS will be used. Using STARTTLS on port 587 is recommended per RFC 6409. If the server supports STARTTLS it will always be used. -HOST = +HOST = ; Disable HELO operation when hostname are different. -DISABLE_HELO = +DISABLE_HELO = ; Custom hostname for HELO operation, default is from system. -HELO_HOSTNAME = +HELO_HOSTNAME = ; Do not verify the certificate of the server. Only use this for self-signed certificates -SKIP_VERIFY = +SKIP_VERIFY = ; Use client certificate USE_CERTIFICATE = false CERT_FILE = custom/mailer/cert.pem KEY_FILE = custom/mailer/key.pem -; Mail from address, RFC 5322. This can be just an email address, or the `"Name" ` format +; Mail from address, RFC 5322. This can be just an email address, or the `"Name" ` format FROM = ; Mailer user name and password USER = @@ -320,7 +320,7 @@ SCHEDULE = @every 24h TIMEOUT = 60s ; Arguments for command 'git fsck', e.g. "--unreachable --tags" ; see more on http://git-scm.com/docs/git-fsck/1.7.5 -ARGS = +ARGS = ; Check repository statistics [cron.check_repo_stats] @@ -328,10 +328,15 @@ RUN_AT_START = true SCHEDULE = @every 24h [git] -MAX_GIT_DIFF_LINES = 10000 +; Max number of lines allowed of a single file in diff view. +MAX_GIT_DIFF_LINES = 500 +; Max number of characters of a line allowed in diff view. +MAX_GIT_DIFF_LINE_CHARACTERS = 500 +; Max number of files shown in diff view. +MAX_GIT_DIFF_FILES = 100 ; Arguments for command 'git gc', e.g. "--aggressive --auto" ; see more on http://git-scm.com/docs/git-gc/1.7.5 -GC_ARGS = +GC_ARGS = ; Operation timeout in seconds [git.timeout] diff --git a/conf/locale/locale_en-US.ini b/conf/locale/locale_en-US.ini index 87453efa6..ad6f18323 100644 --- a/conf/locale/locale_en-US.ini +++ b/conf/locale/locale_en-US.ini @@ -690,6 +690,8 @@ diff.show_unified_view = Unified View diff.stats_desc = %d changed files with %d additions and %d deletions diff.bin = BIN diff.view_file = View File +diff.file_supressed = File diff supressed because it is too large +diff.too_many_files = Some files were not shown because too many files changed in this diff release.releases = Releases release.new_release = New Release diff --git a/models/git_diff.go b/models/git_diff.go index ab70139fb..f89320087 100644 --- a/models/git_diff.go +++ b/models/git_diff.go @@ -48,10 +48,10 @@ const ( ) type DiffLine struct { - LeftIdx int - RightIdx int - Type DiffLineType - Content string + LeftIdx int + RightIdx int + Type DiffLineType + Content string } func (d *DiffLine) GetType() int { @@ -161,6 +161,7 @@ type DiffFile struct { IsBin bool IsRenamed bool Sections []*DiffSection + IsIncomplete bool } func (diffFile *DiffFile) GetType() int { @@ -174,6 +175,7 @@ func (diffFile *DiffFile) GetHighlightClass() string { type Diff struct { TotalAddition, TotalDeletion int Files []*DiffFile + IsIncomplete bool } func (diff *Diff) NumFiles() int { @@ -182,7 +184,7 @@ func (diff *Diff) NumFiles() int { const DIFF_HEAD = "diff --git " -func ParsePatch(maxlines int, reader io.Reader) (*Diff, error) { +func ParsePatch(maxLines, maxLineCharacteres, maxFiles int, reader io.Reader) (*Diff, error) { var ( diff = &Diff{Files: make([]*DiffFile, 0)} @@ -193,15 +195,12 @@ func ParsePatch(maxlines int, reader io.Reader) (*Diff, error) { leftLine, rightLine int lineCount int + curFileLinesCount int ) input := bufio.NewReader(reader) isEOF := false - for { - if isEOF { - break - } - + for !isEOF { line, err := input.ReadString('\n') if err != nil { if err == io.EOF { @@ -216,20 +215,16 @@ func ParsePatch(maxlines int, reader io.Reader) (*Diff, error) { line = line[:len(line)-1] } - if strings.HasPrefix(line, "+++ ") || strings.HasPrefix(line, "--- ") { - continue - } else if len(line) == 0 { + if strings.HasPrefix(line, "+++ ") || strings.HasPrefix(line, "--- ") || len(line) == 0 { continue } + curFileLinesCount++ lineCount++ // Diff data too large, we only show the first about maxlines lines - if lineCount >= maxlines { - log.Warn("Diff data too large") - io.Copy(ioutil.Discard, reader) - diff.Files = nil - return diff, nil + if curFileLinesCount >= maxLines || len(line) >= maxLineCharacteres { + curFile.IsIncomplete = true } switch { @@ -304,6 +299,12 @@ func ParsePatch(maxlines int, reader io.Reader) (*Diff, error) { Sections: make([]*DiffSection, 0, 10), } diff.Files = append(diff.Files, curFile) + if len(diff.Files) >= maxFiles { + diff.IsIncomplete = true + io.Copy(ioutil.Discard, reader) + break + } + curFileLinesCount = 0 // Check file diff type. for { @@ -366,7 +367,7 @@ func ParsePatch(maxlines int, reader io.Reader) (*Diff, error) { return diff, nil } -func GetDiffRange(repoPath, beforeCommitID string, afterCommitID string, maxlines int) (*Diff, error) { +func GetDiffRange(repoPath, beforeCommitID string, afterCommitID string, maxLines, maxLineCharacteres, maxFiles int) (*Diff, error) { repo, err := git.OpenRepository(repoPath) if err != nil { return nil, err @@ -405,7 +406,7 @@ func GetDiffRange(repoPath, beforeCommitID string, afterCommitID string, maxline pid := process.Add(fmt.Sprintf("GetDiffRange (%s)", repoPath), cmd) defer process.Remove(pid) - diff, err := ParsePatch(maxlines, stdout) + diff, err := ParsePatch(maxLines, maxLineCharacteres, maxFiles, stdout) if err != nil { return nil, fmt.Errorf("ParsePatch: %v", err) } @@ -417,6 +418,6 @@ func GetDiffRange(repoPath, beforeCommitID string, afterCommitID string, maxline return diff, nil } -func GetDiffCommit(repoPath, commitId string, maxlines int) (*Diff, error) { - return GetDiffRange(repoPath, "", commitId, maxlines) +func GetDiffCommit(repoPath, commitId string, maxLines, maxLineCharacteres, maxFiles int) (*Diff, error) { + return GetDiffRange(repoPath, "", commitId, maxLines, maxLineCharacteres, maxFiles) } diff --git a/modules/setting/setting.go b/modules/setting/setting.go index ef81258da..d08d5be63 100644 --- a/modules/setting/setting.go +++ b/modules/setting/setting.go @@ -161,9 +161,11 @@ var ( // Git settings Git struct { - MaxGitDiffLines int - GcArgs []string `delim:" "` - Timeout struct { + MaxGitDiffLines int + MaxGitDiffLineCharacters int + MaxGitDiffFiles int + GcArgs []string `delim:" "` + Timeout struct { Migrate int Mirror int Clone int diff --git a/routers/repo/commit.go b/routers/repo/commit.go index 16f7b63d4..436a06ee9 100644 --- a/routers/repo/commit.go +++ b/routers/repo/commit.go @@ -157,7 +157,8 @@ func Diff(ctx *context.Context) { } diff, err := models.GetDiffCommit(models.RepoPath(userName, repoName), - commitID, setting.Git.MaxGitDiffLines) + commitID, setting.Git.MaxGitDiffLines, + setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles) if err != nil { ctx.Handle(404, "GetDiffCommit", err) return @@ -212,7 +213,8 @@ func CompareDiff(ctx *context.Context) { } diff, err := models.GetDiffRange(models.RepoPath(userName, repoName), beforeCommitID, - afterCommitID, setting.Git.MaxGitDiffLines) + afterCommitID, setting.Git.MaxGitDiffLines, + setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles) if err != nil { ctx.Handle(404, "GetDiffRange", err) return diff --git a/routers/repo/pull.go b/routers/repo/pull.go index 8ad4f3f8e..922d64ef4 100644 --- a/routers/repo/pull.go +++ b/routers/repo/pull.go @@ -348,7 +348,8 @@ func ViewPullFiles(ctx *context.Context) { } diff, err := models.GetDiffRange(diffRepoPath, - startCommitID, endCommitID, setting.Git.MaxGitDiffLines) + startCommitID, endCommitID, setting.Git.MaxGitDiffLines, + setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles) if err != nil { ctx.Handle(500, "GetDiffRange", err) return @@ -545,7 +546,8 @@ func PrepareCompareDiff( } diff, err := models.GetDiffRange(models.RepoPath(headUser.Name, headRepo.Name), - prInfo.MergeBase, headCommitID, setting.Git.MaxGitDiffLines) + prInfo.MergeBase, headCommitID, setting.Git.MaxGitDiffLines, + setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles) if err != nil { ctx.Handle(500, "GetDiffRange", err) return false diff --git a/templates/repo/diff_box.tmpl b/templates/repo/diff_box.tmpl index 455f07002..cfb3466cd 100644 --- a/templates/repo/diff_box.tmpl +++ b/templates/repo/diff_box.tmpl @@ -34,92 +34,120 @@ {{range $i, $file := .Diff.Files}} - {{$highlightClass := $file.GetHighlightClass}} -
-

-
- {{if $file.IsBin}} - {{$.i18n.Tr "repo.diff.bin"}} - {{else if not $file.IsRenamed}} - + {{.Addition}} - - - - - - {{.Deletion}} - {{end}} -
- {{if $file.IsRenamed}}{{$file.OldName}} → {{end}}{{$file.Name}} -
- {{if $file.IsDeleted}} - {{$.i18n.Tr "repo.diff.view_file"}} - {{else}} - {{$.i18n.Tr "repo.diff.view_file"}} - {{end}} -
-

-
- {{if not $file.IsRenamed}} - {{$isImage := (call $.IsImageFile $file.Name)}} - {{if and $isImage}} -
- -
- {{else}} -
- - - {{if $.IsSplitStyle}} - {{range $j, $section := .Sections}} - {{range $k, $line := .Lines}} - - - - - - + {{if $file.IsIncomplete}} +
+

+ {{$.i18n.Tr "repo.diff.file_supressed"}} +
+ {{if not $file.IsRenamed}} + + {{.Addition}} + + + + + - {{.Deletion}} + {{end}} +
+ {{$file.Name}} +

+
+ {{else}} + {{$highlightClass := $file.GetHighlightClass}} +
+

+
+ {{if $file.IsBin}} + {{$.i18n.Tr "repo.diff.bin"}} + {{else if not $file.IsRenamed}} + + {{.Addition}} + + + + + - {{.Deletion}} + {{end}} +
+ {{if $file.IsRenamed}}{{$file.OldName}} → {{end}}{{$file.Name}} +
+ {{if $file.IsDeleted}} + {{$.i18n.Tr "repo.diff.view_file"}} + {{else}} + {{$.i18n.Tr "repo.diff.view_file"}} + {{end}} +
+

+
+ {{if not $file.IsRenamed}} + {{$isImage := (call $.IsImageFile $file.Name)}} + {{if and $isImage}} +
+ +
+ {{else}} +
+
- {{if $line.LeftIdx}}{{$line.LeftIdx}}{{end}} - - 
{{if $line.LeftIdx}}{{$section.GetComputedInlineDiffFor $line}}{{end}}
- 		- {{if $line.RightIdx}}{{$line.RightIdx}}{{end}} - - 
{{if $line.RightIdx}}{{$section.GetComputedInlineDiffFor $line}}{{end}}
-
+ + {{if $.IsSplitStyle}} + {{range $j, $section := .Sections}} + {{range $k, $line := .Lines}} + + + + + + + {{end}} {{end}} - {{end}} - {{else}} - {{range $j, $section := .Sections}} - {{range $k, $line := .Lines}} - - {{if eq .GetType 4}} - - {{else}} - - - {{end}} - - + {{else}} + {{range $j, $section := .Sections}} + {{range $k, $line := .Lines}} + + {{if eq .GetType 4}} + + {{else}} + + + {{end}} + + + {{end}} {{end}} {{end}} - {{end}} - -
+ {{if $line.LeftIdx}}{{$line.LeftIdx}}{{end}} + + 
{{if $line.LeftIdx}}{{$section.GetComputedInlineDiffFor $line}}{{end}}
+ 		+ {{if $line.RightIdx}}{{$line.RightIdx}}{{end}} + + 
{{if $line.RightIdx}}{{$section.GetComputedInlineDiffFor $line}}{{end}}
+
- {{/* {{if gt $j 0}}{{end}} */}} - - {{if $line.LeftIdx}}{{$line.LeftIdx}}{{end}} - - {{if $line.RightIdx}}{{$line.RightIdx}}{{end}} - - 
{{$section.GetComputedInlineDiffFor $line}}
-
+ {{/* {{if gt $j 0}}{{end}} */}} + + {{if $line.LeftIdx}}{{$line.LeftIdx}}{{end}} + + {{if $line.RightIdx}}{{$line.RightIdx}}{{end}} + + 
{{$section.GetComputedInlineDiffFor $line}}
+
-
+ + +
+ {{end}} {{end}} - {{end}} +
- + {{end}}
{{end}} + + {{if .Diff.IsIncomplete}} +
+

+ {{$.i18n.Tr "repo.diff.too_many_files"}} +

+
+ {{end}} + {{if .IsSplitStyle}}