Refactor highlight and diff (#36599)

1. fix a performance regression when using line-by-line highlighting
* the root cause is that chroma's `lexers.Get` is slow and a lexer cache
is missing during recent changes
2. clarify the chroma lexer detection behavior
* now we fully manage our logic to detect lexer, and handle overriding
problems, everything is fully under control
3. clarify "code analyze" behavior, now only 2 usages:
* only use file name and language to detect lexer (very fast), mainly
for "diff" page which contains a lot of files
* if no lexer is detected by file name and language, use code content to
detect again (slow), mainly for "view file" or "blame" page, which can
get best result
4. fix git diff bug, it caused "broken pipe" error for large diff files

Author: wxiaoguang

modules/git/diff.go

@@ -28,44 +28,37 @@ const (
 
 // GetRawDiff dumps diff results of repository in given commit ID to io.Writer.
 func GetRawDiff(repo *Repository, commitID string, diffType RawDiffType, writer io.Writer) (retErr error) {
-	diffOutput, diffFinish, err := getRepoRawDiffForFile(repo.Ctx, repo, "", commitID, diffType, "")
+	cmd, err := getRepoRawDiffForFileCmd(repo.Ctx, repo, "", commitID, diffType, "")
 	if err != nil {
-		return err
+		return fmt.Errorf("getRepoRawDiffForFileCmd: %w", err)
 	}
-	defer func() {
-		err := diffFinish()
-		if retErr == nil {
-			retErr = err // only return command's error if no previous error
-		}
-	}()
-	_, err = io.Copy(writer, diffOutput)
-	return err
+	return cmd.WithStdoutCopy(writer).RunWithStderr(repo.Ctx)
 }
 
 // GetFileDiffCutAroundLine cuts the old or new part of the diff of a file around a specific line number
 func GetFileDiffCutAroundLine(
 	repo *Repository, startCommit, endCommit, treePath string,
 	line int64, old bool, numbersOfLine int,
-) (_ string, retErr error) {
-	diffOutput, diffFinish, err := getRepoRawDiffForFile(repo.Ctx, repo, startCommit, endCommit, RawDiffNormal, treePath)
+) (ret string, retErr error) {
+	cmd, err := getRepoRawDiffForFileCmd(repo.Ctx, repo, startCommit, endCommit, RawDiffNormal, treePath)
 	if err != nil {
-		return "", err
+		return "", fmt.Errorf("getRepoRawDiffForFileCmd: %w", err)
 	}
-	defer func() {
-		err := diffFinish()
-		if retErr == nil {
-			retErr = err // only return command's error if no previous error
-		}
-	}()
-	return CutDiffAroundLine(diffOutput, line, old, numbersOfLine)
+	stdoutReader, stdoutClose := cmd.MakeStdoutPipe()
+	defer stdoutClose()
+	cmd.WithPipelineFunc(func(ctx gitcmd.Context) error {
+		ret, err = CutDiffAroundLine(stdoutReader, line, old, numbersOfLine)
+		return err
+	})
+	return ret, cmd.RunWithStderr(repo.Ctx)
 }
 
 // getRepoRawDiffForFile returns an io.Reader for the diff results of file in given commit ID
 // and a "finish" function to wait for the git command and clean up resources after reading is done.
-func getRepoRawDiffForFile(ctx context.Context, repo *Repository, startCommit, endCommit string, diffType RawDiffType, file string) (io.Reader, func() gitcmd.RunStdError, error) {
+func getRepoRawDiffForFileCmd(_ context.Context, repo *Repository, startCommit, endCommit string, diffType RawDiffType, file string) (*gitcmd.Command, error) {
 	commit, err := repo.GetCommit(endCommit)
 	if err != nil {
-		return nil, nil, err
+		return nil, err
 	}
 	var files []string
 	if len(file) > 0 {
@@ -84,7 +77,7 @@ func getRepoRawDiffForFile(ctx context.Context, repo *Repository, startCommit, e
 		} else {
 			c, err := commit.Parent(0)
 			if err != nil {
-				return nil, nil, err
+				return nil, err
 			}
 			cmd.AddArguments("diff").
 				AddOptionFormat("--find-renames=%s", setting.Git.DiffRenameSimilarityThreshold).
@@ -99,25 +92,15 @@ func getRepoRawDiffForFile(ctx context.Context, repo *Repository, startCommit, e
 		} else {
 			c, err := commit.Parent(0)
 			if err != nil {
-				return nil, nil, err
+				return nil, err
 			}
 			query := fmt.Sprintf("%s...%s", endCommit, c.ID.String())
 			cmd.AddArguments("format-patch", "--no-signature", "--stdout").AddDynamicArguments(query).AddDashesAndList(files...)
 		}
 	default:
-		return nil, nil, util.NewInvalidArgumentErrorf("invalid diff type: %s", diffType)
-	}
-
-	stdoutReader, stdoutReaderClose := cmd.MakeStdoutPipe()
-	err = cmd.StartWithStderr(ctx)
-	if err != nil {
-		stdoutReaderClose()
-		return nil, nil, err
+		return nil, util.NewInvalidArgumentErrorf("invalid diff type: %s", diffType)
 	}
-	return stdoutReader, func() gitcmd.RunStdError {
-		stdoutReaderClose()
-		return cmd.WaitWithStderr()
-	}, nil
+	return cmd, nil
 }
 
 // ParseDiffHunkString parse the diff hunk content and return
@@ -254,7 +237,7 @@ func CutDiffAroundLine(originalDiff io.Reader, line int64, old bool, numbersOfLi
 		}
 	}
 	if err := scanner.Err(); err != nil {
-		return "", err
+		return "", fmt.Errorf("CutDiffAroundLine: scan: %w", err)
 	}
 
 	// No hunk found

modules/git/gitcmd/command.go

@@ -306,6 +306,10 @@ func (c *Command) MakeStdinPipe() (writer PipeWriter, closer func()) {
 // MakeStdoutPipe creates a reader for the command's stdout.
 // The returned closer function must be called by the caller to close the pipe.
 // After the pipe reader is closed, the unread data will be discarded.
+//
+// If the process (git command) still tries to write after the pipe is closed, the Wait error will be "signal: broken pipe".
+// WithPipelineFunc + Run won't return "broken pipe" error in this case if the callback returns no error.
+// But if you are calling Start / Wait family functions, you should either drain the pipe before close it, or handle the Wait error correctly.
 func (c *Command) MakeStdoutPipe() (reader PipeReader, closer func()) {
 	return c.makeStdoutStderr(&c.cmdStdout)
 }

modules/highlight/highlight.go

@@ -11,20 +11,16 @@ import (
 	gohtml "html"
 	"html/template"
 	"io"
-	"path"
 	"strings"
 	"sync"
 
-	"code.gitea.io/gitea/modules/analyze"
 	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/setting"
 	"code.gitea.io/gitea/modules/util"
 
 	"github.com/alecthomas/chroma/v2"
 	"github.com/alecthomas/chroma/v2/formatters/html"
-	"github.com/alecthomas/chroma/v2/lexers"
 	"github.com/alecthomas/chroma/v2/styles"
-	"github.com/go-enry/go-enry/v2"
 )
 
 // don't index files larger than this many bytes for performance purposes
@@ -84,85 +80,21 @@ func UnsafeSplitHighlightedLines(code template.HTML) (ret [][]byte) {
 	}
 }
 
-func getChromaLexerByLanguage(fileName, lang string) chroma.Lexer {
-	lang, _, _ = strings.Cut(lang, "?") // maybe, the value from gitattributes might contain `?` parameters?
-	ext := path.Ext(fileName)
-	// the "lang" might come from enry, it has different naming for some languages
-	switch lang {
-	case "F#":
-		lang = "FSharp"
-	case "Pascal":
-		lang = "ObjectPascal"
-	case "C":
-		if ext == ".C" || ext == ".H" {
-			lang = "C++"
-		}
-	}
-	if lang == "" && util.AsciiEqualFold(ext, ".sql") {
-		// there is a bug when using MySQL lexer: "--\nSELECT", the second line will be rendered as comment incorrectly
-		lang = "SQL"
-	}
-	// lexers.Get is slow if the language name can't be matched directly: it does extra "Match" call to iterate all lexers
-	return lexers.Get(lang)
-}
-
-// GetChromaLexerWithFallback returns a chroma lexer by given file name, language and code content. All parameters can be optional.
-// When code content is provided, it will be slow if no lexer is found by file name or language.
-// If no lexer is found, it will return the fallback lexer.
-func GetChromaLexerWithFallback(fileName, lang string, code []byte) (lexer chroma.Lexer) {
-	if lang != "" {
-		lexer = getChromaLexerByLanguage(fileName, lang)
-	}
-
-	if lexer == nil {
-		fileExt := path.Ext(fileName)
-		if val, ok := globalVars().highlightMapping[fileExt]; ok {
-			lexer = getChromaLexerByLanguage(fileName, val) // use mapped value to find lexer
-		}
-	}
-
-	if lexer == nil {
-		// when using "code" to detect, analyze.GetCodeLanguage is slower, it iterates many rules to detect language from content
-		// this is the old logic: use enry to detect language, and use chroma to render, but their naming is different for some languages
-		enryLanguage := analyze.GetCodeLanguage(fileName, code)
-		lexer = getChromaLexerByLanguage(fileName, enryLanguage)
-		if lexer == nil {
-			if enryLanguage != enry.OtherLanguage {
-				log.Warn("No chroma lexer found for enry detected language: %s (file: %s), need to fix the language mapping between enry and chroma.", enryLanguage, fileName)
-			}
-			lexer = lexers.Match(fileName) // lexers.Match will search by its basename and extname
-		}
-	}
-
-	return util.IfZero(lexer, lexers.Fallback)
-}
-
-func renderCode(fileName, language, code string, slowGuess bool) (output template.HTML, lexerName string) {
+// RenderCodeSlowGuess tries to get a lexer by file name and language first,
+// if not found, it will try to guess the lexer by code content, which is slow (more than several hundreds of milliseconds).
+func RenderCodeSlowGuess(fileName, language, code string) (output template.HTML, lexer chroma.Lexer, lexerDisplayName string) {
 	// diff view newline will be passed as empty, change to literal '\n' so it can be copied
 	// preserve literal newline in blame view
 	if code == "" || code == "\n" {
-		return "\n", ""
+		return "\n", nil, ""
 	}
 
 	if len(code) > sizeLimit {
-		return template.HTML(template.HTMLEscapeString(code)), ""
+		return template.HTML(template.HTMLEscapeString(code)), nil, ""
 	}
 
-	var codeForGuessLexer []byte
-	if slowGuess {
-		// it is slower to guess lexer by code content, so only do it when necessary
-		codeForGuessLexer = util.UnsafeStringToBytes(code)
-	}
-	lexer := GetChromaLexerWithFallback(fileName, language, codeForGuessLexer)
-	return RenderCodeByLexer(lexer, code), formatLexerName(lexer.Config().Name)
-}
-
-func RenderCodeFast(fileName, language, code string) (output template.HTML, lexerName string) {
-	return renderCode(fileName, language, code, false)
-}
-
-func RenderCodeSlowGuess(fileName, language, code string) (output template.HTML, lexerName string) {
-	return renderCode(fileName, language, code, true)
+	lexer = detectChromaLexerWithAnalyze(fileName, language, util.UnsafeStringToBytes(code)) // it is also slow
+	return RenderCodeByLexer(lexer, code), lexer, formatLexerName(lexer.Config().Name)
 }
 
 // RenderCodeByLexer returns a HTML version of code string with chroma syntax highlighting classes
@@ -204,7 +136,7 @@ func RenderFullFile(fileName, language string, code []byte) ([]template.HTML, st
 		html.PreventSurroundingPre(true),
 	)
 
-	lexer := GetChromaLexerWithFallback(fileName, language, code)
+	lexer := detectChromaLexerWithAnalyze(fileName, language, code)
 	lexerName := formatLexerName(lexer.Config().Name)
 
 	iterator, err := lexer.Tokenise(nil, string(code))

modules/highlight/highlight_test.go

@@ -205,36 +205,3 @@ func TestUnsafeSplitHighlightedLines(t *testing.T) {
 	assert.Equal(t, "<span>a</span>\n", string(ret[0]))
 	assert.Equal(t, "<span>b\n</span>", string(ret[1]))
 }
-
-func TestGetChromaLexer(t *testing.T) {
-	globalVars().highlightMapping[".my-html"] = "HTML"
-	t.Cleanup(func() { delete(globalVars().highlightMapping, ".my-html") })
-
-	cases := []struct {
-		fileName string
-		language string
-		content  string
-		expected string
-	}{
-		{"test.py", "", "", "Python"},
-
-		{"any-file", "javascript", "", "JavaScript"},
-		{"any-file", "", "/* vim: set filetype=python */", "Python"},
-		{"any-file", "", "", "fallback"},
-
-		{"test.fs", "", "", "Forth"},
-		{"test.fs", "F#", "", "FSharp"},
-		{"test.fs", "", "let x = 1", "FSharp"},
-
-		{"test.c", "", "", "C"},
-		{"test.C", "", "", "C++"},
-		{"OLD-CODE.PAS", "", "", "ObjectPascal"},
-		{"test.my-html", "", "", "HTML"},
-	}
-	for _, c := range cases {
-		lexer := GetChromaLexerWithFallback(c.fileName, c.language, []byte(c.content))
-		if assert.NotNil(t, lexer, "case: %+v", c) {
-			assert.Equal(t, c.expected, lexer.Config().Name, "case: %+v", c)
-		}
-	}
-}