From b7e8672927faaafdf767ed5a9dcaa9e823612f89 Mon Sep 17 00:00:00 2001
From: wxiaoguang <wxiaoguang@gmail.com>
Date: Sat, 23 Mar 2024 13:31:29 +0800
Subject: [PATCH] better error handling

---
 modules/git/command.go   |  4 ++--
 modules/git/git.go       |  8 ++++----
 modules/git/grep.go      | 27 +++++++++++----------------
 modules/git/grep_test.go |  3 +++
 4 files changed, 20 insertions(+), 22 deletions(-)

diff --git a/modules/git/command.go b/modules/git/command.go
index 371109730a23e..e0b7324a13b58 100644
--- a/modules/git/command.go
+++ b/modules/git/command.go
@@ -392,9 +392,9 @@ func (r *runStdError) Stderr() string {
 	return r.stderr
 }
 
-func (r *runStdError) IsExitCode(code int) bool {
+func IsErrorExitCode(err error, code int) bool {
 	var exitError *exec.ExitError
-	if errors.As(r.err, &exitError) {
+	if errors.As(err, &exitError) {
 		return exitError.ExitCode() == code
 	}
 	return false
diff --git a/modules/git/git.go b/modules/git/git.go
index f688ea748815c..e411269f7c50a 100644
--- a/modules/git/git.go
+++ b/modules/git/git.go
@@ -341,7 +341,7 @@ func checkGitVersionCompatibility(gitVer *version.Version) error {
 
 func configSet(key, value string) error {
 	stdout, _, err := NewCommand(DefaultContext, "config", "--global", "--get").AddDynamicArguments(key).RunStdString(nil)
-	if err != nil && !err.IsExitCode(1) {
+	if err != nil && !IsErrorExitCode(err, 1) {
 		return fmt.Errorf("failed to get git config %s, err: %w", key, err)
 	}
 
@@ -364,7 +364,7 @@ func configSetNonExist(key, value string) error {
 		// already exist
 		return nil
 	}
-	if err.IsExitCode(1) {
+	if IsErrorExitCode(err, 1) {
 		// not exist, set new config
 		_, _, err = NewCommand(DefaultContext, "config", "--global").AddDynamicArguments(key, value).RunStdString(nil)
 		if err != nil {
@@ -382,7 +382,7 @@ func configAddNonExist(key, value string) error {
 		// already exist
 		return nil
 	}
-	if err.IsExitCode(1) {
+	if IsErrorExitCode(err, 1) {
 		// not exist, add new config
 		_, _, err = NewCommand(DefaultContext, "config", "--global", "--add").AddDynamicArguments(key, value).RunStdString(nil)
 		if err != nil {
@@ -403,7 +403,7 @@ func configUnsetAll(key, value string) error {
 		}
 		return nil
 	}
-	if err.IsExitCode(1) {
+	if IsErrorExitCode(err, 1) {
 		// not exist
 		return nil
 	}
diff --git a/modules/git/grep.go b/modules/git/grep.go
index e00bc4837478c..e533995984e38 100644
--- a/modules/git/grep.go
+++ b/modules/git/grep.go
@@ -5,10 +5,10 @@ package git
 
 import (
 	"bufio"
+	"bytes"
 	"context"
 	"errors"
 	"fmt"
-	"io"
 	"os"
 	"strconv"
 	"strings"
@@ -33,15 +33,9 @@ func GrepSearch(ctx context.Context, repo *Repository, search string, opts GrepO
 	if err != nil {
 		return nil, fmt.Errorf("unable to create os pipe to grep: %w", err)
 	}
-	stderrReader, stderrWriter, err := os.Pipe()
-	if err != nil {
-		return nil, fmt.Errorf("unable to create os pipe to grep: %w", err)
-	}
 	defer func() {
 		_ = stdoutReader.Close()
 		_ = stdoutWriter.Close()
-		_ = stderrReader.Close()
-		_ = stderrWriter.Close()
 	}()
 
 	/*
@@ -53,28 +47,26 @@ func GrepSearch(ctx context.Context, repo *Repository, search string, opts GrepO
 	 HEAD:.changelog.yml
 	 2^@repo: go-gitea/gitea
 	*/
-	var stderr []byte
 	var results []*GrepResult
 	cmd := NewCommand(ctx, "grep", "--null", "--break", "--heading", "--fixed-strings", "--line-number", "--ignore-case", "--full-name")
 	cmd.AddOptionValues("--context", fmt.Sprint(opts.ContextLineNumber))
 	if opts.IsFuzzy {
 		words := strings.Fields(search)
 		for _, word := range words {
-			cmd.AddOptionValues("-e", word)
+			cmd.AddOptionValues("-e", strings.TrimLeft(word, "-"))
 		}
 	} else {
-		cmd.AddOptionValues("-e", search)
+		cmd.AddOptionValues("-e", strings.TrimLeft(search, "-"))
 	}
 	cmd.AddDynamicArguments(util.IfZero(opts.RefName, "HEAD"))
+	stderr := bytes.Buffer{}
 	err = cmd.Run(&RunOpts{
 		Dir:    repo.Path,
 		Stdout: stdoutWriter,
-		Stderr: stderrWriter,
+		Stderr: &stderr,
 		PipelineFunc: func(ctx context.Context, cancel context.CancelFunc) error {
 			_ = stdoutWriter.Close()
-			_ = stderrWriter.Close()
 			defer stdoutReader.Close()
-			defer stderrReader.Close()
 
 			isInBlock := false
 			scanner := bufio.NewScanner(stdoutReader)
@@ -106,12 +98,15 @@ func GrepSearch(ctx context.Context, repo *Repository, search string, opts GrepO
 					res.LineCodes = append(res.LineCodes, lineCode)
 				}
 			}
-			stderr, _ = io.ReadAll(stderrReader)
 			return scanner.Err()
 		},
 	})
-	if err != nil && !errors.Is(err, context.Canceled) && len(stderr) != 0 {
-		return nil, fmt.Errorf("unable to run git grep: %w, stderr: %s", err, string(stderr))
+	// git grep exits with 1 if no results are found
+	if IsErrorExitCode(err, 1) && stderr.Len() == 0 {
+		return nil, nil
+	}
+	if err != nil && !errors.Is(err, context.Canceled) {
+		return nil, fmt.Errorf("unable to run git grep: %w, stderr: %s", err, stderr.String())
 	}
 	return results, nil
 }
diff --git a/modules/git/grep_test.go b/modules/git/grep_test.go
index cfd82d4380c53..af56d491f4e0e 100644
--- a/modules/git/grep_test.go
+++ b/modules/git/grep_test.go
@@ -34,4 +34,7 @@ func TestGrepSearch(t *testing.T) {
 	res, err = GrepSearch(context.Background(), repo, "no-such-content", GrepOptions{})
 	assert.NoError(t, err)
 	assert.Len(t, res, 0)
+
+	res, err = GrepSearch(context.Background(), &Repository{Path: "no-such-git-repo"}, "no-such-content", GrepOptions{})
+	assert.Error(t, err)
 }