Skip to content

Commit fbfc2c9

Browse files
committed
fix(tool): capture stdout-only when resolving git top-level
Address review: CombinedOutput could merge git stderr warnings into the resolved path; use Output() so the top-level path stays clean.
1 parent 852fb94 commit fbfc2c9

2 files changed

Lines changed: 10 additions & 1 deletion

File tree

cmd/opencodereview/git.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,15 @@ func runGitCmd(repoDir string, args ...string) ([]byte, error) {
1212
return cmd.CombinedOutput()
1313
}
1414

15+
// runGitCmdStdout is like runGitCmd but returns stdout only. Use it when the
16+
// output is consumed as data (e.g. a resolved path) so git's stderr warnings
17+
// (permissions, deprecations, config notices) can't pollute the result.
18+
func runGitCmdStdout(repoDir string, args ...string) ([]byte, error) {
19+
fullArgs := append([]string{"-C", repoDir}, args...)
20+
cmd := exec.Command("git", fullArgs...)
21+
return cmd.Output()
22+
}
23+
1524
func getCommitMessage(repoDir, commit string) (string, error) {
1625
out, err := runGitCmd(repoDir, "log", "-1", "--format=%B", "--end-of-options", commit)
1726
if err != nil {

cmd/opencodereview/shared.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ func resolveWorkingDir(input string, requireGit bool) (string, bool, error) {
108108
// requireGit is true only for the review path; scan (requireGit=false) keeps
109109
// the CWD so its `git ls-files` walk stays scoped to the subdirectory.
110110
if isGit && requireGit {
111-
if top, topErr := runGitCmd(absPath, "rev-parse", "--show-toplevel"); topErr == nil {
111+
if top, topErr := runGitCmdStdout(absPath, "rev-parse", "--show-toplevel"); topErr == nil {
112112
if t := strings.TrimSpace(string(top)); t != "" {
113113
absPath = t
114114
}

0 commit comments

Comments
 (0)