Skip to content

Commit c18688b

Browse files
committed
fix: improve git diff errors
1 parent be6d820 commit c18688b

File tree

1 file changed

+52
-30
lines changed

1 file changed

+52
-30
lines changed

Diff for: revgrep.go

+52-30
Original file line numberDiff line numberDiff line change
@@ -334,10 +334,13 @@ func (c *Checker) linesChanged() map[string][]pos {
334334
return changes
335335
}
336336

337-
// GitPatch returns a patch from a git repository,
338-
// if no git repository was found and no errors occurred, nil is returned, else an error is returned revisionFrom and revisionTo defines the git diff parameters,
339-
// if left blank and there are unstaged changes or untracked files, only those will be returned else only check changes since HEAD~.
340-
// If revisionFrom is set but revisionTo is not, untracked files will be included, to exclude untracked files set revisionTo to HEAD~.
337+
// GitPatch returns a patch from a git repository.
338+
// If no git repository was found and no errors occurred, nil is returned,
339+
// else an error is returned revisionFrom and revisionTo defines the git diff parameters,
340+
// if left blank and there are unstaged changes or untracked files,
341+
// only those will be returned else only check changes since HEAD~.
342+
// If revisionFrom is set but revisionTo is not,
343+
// untracked files will be included, to exclude untracked files set revisionTo to HEAD~.
341344
// It's incorrect to specify revisionTo without a revisionFrom.
342345
func GitPatch(revisionFrom, revisionTo string) (io.Reader, []string, error) {
343346
// check if git repo exists
@@ -356,59 +359,59 @@ func GitPatch(revisionFrom, revisionTo string) (io.Reader, []string, error) {
356359
for _, file := range bytes.Split(ls, []byte{'\n'}) {
357360
if len(file) == 0 || bytes.HasSuffix(file, []byte{'/'}) {
358361
// ls-files was sometimes showing directories when they were ignored
359-
// I couldn't create a test case for this as I couldn't reproduce correctly
360-
// for the moment, just exclude files with trailing /
362+
// I couldn't create a test case for this as I couldn't reproduce correctly for the moment,
363+
// just exclude files with trailing /
361364
continue
362365
}
366+
363367
newFiles = append(newFiles, string(file))
364368
}
365369

366-
var patch bytes.Buffer
367370
if revisionFrom != "" {
368-
cmd := gitDiff(revisionFrom)
371+
args := []string{revisionFrom}
372+
369373
if revisionTo != "" {
370-
cmd.Args = append(cmd.Args, revisionTo)
374+
args = append(args, revisionTo)
371375
}
372-
cmd.Args = append(cmd.Args, "--")
373376

374-
cmd.Stdout = &patch
375-
if err := cmd.Run(); err != nil {
376-
return nil, nil, fmt.Errorf("error executing git diff %q %q: %w", revisionFrom, revisionTo, err)
377+
args = append(args, "--")
378+
379+
patch, errDiff := gitDiff(args...)
380+
if errDiff != nil {
381+
return nil, nil, errDiff
377382
}
378383

379384
if revisionTo == "" {
380-
return &patch, newFiles, nil
385+
return patch, newFiles, nil
381386
}
382387

383-
return &patch, nil, nil
388+
return patch, nil, nil
384389
}
385390

386391
// make a patch for unstaged changes
387-
cmd := gitDiff("--")
388-
cmd.Stdout = &patch
389-
if err := cmd.Run(); err != nil {
390-
return nil, nil, fmt.Errorf("error executing git diff: %w", err)
392+
patch, err := gitDiff("--")
393+
if err != nil {
394+
return nil, nil, err
391395
}
396+
392397
unstaged := patch.Len() > 0
393398

394-
// If there's unstaged changes OR untracked changes (or both), then this is
395-
// a suitable patch
399+
// If there's unstaged changes OR untracked changes (or both),
400+
// then this is a suitable patch
396401
if unstaged || newFiles != nil {
397-
return &patch, newFiles, nil
402+
return patch, newFiles, nil
398403
}
399404

400405
// check for changes in recent commit
401-
402-
cmd = gitDiff("HEAD~", "--")
403-
cmd.Stdout = &patch
404-
if err := cmd.Run(); err != nil {
405-
return nil, nil, fmt.Errorf("error executing git diff HEAD~: %w", err)
406+
patch, err = gitDiff("HEAD~", "--")
407+
if err != nil {
408+
return nil, nil, err
406409
}
407410

408-
return &patch, nil, nil
411+
return patch, nil, nil
409412
}
410413

411-
func gitDiff(extraArgs ...string) *exec.Cmd {
414+
func gitDiff(extraArgs ...string) (*bytes.Buffer, error) {
412415
cmd := exec.Command("git", "diff", "--color=never", "--no-ext-diff")
413416

414417
if isSupportedByGit(2, 41, 0) {
@@ -418,7 +421,26 @@ func gitDiff(extraArgs ...string) *exec.Cmd {
418421
cmd.Args = append(cmd.Args, "--relative")
419422
cmd.Args = append(cmd.Args, extraArgs...)
420423

421-
return cmd
424+
patch := new(bytes.Buffer)
425+
errBuff := new(bytes.Buffer)
426+
427+
cmd.Stdout = patch
428+
cmd.Stderr = errBuff
429+
430+
if err := cmd.Run(); err != nil {
431+
return nil, fmt.Errorf("error executing %q: %w: %w", strings.Join(cmd.Args, " "), err, readAsError(errBuff))
432+
}
433+
434+
return patch, nil
435+
}
436+
437+
func readAsError(buff io.Reader) error {
438+
output, err := io.ReadAll(buff)
439+
if err != nil {
440+
return fmt.Errorf("read stderr: %w", err)
441+
}
442+
443+
return errors.New(string(output))
422444
}
423445

424446
func isSupportedByGit(major, minor, patch int) bool {

0 commit comments

Comments
 (0)