Skip to content

Fix cross-platform help command and improve diff messages #36

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
yankeeDamn wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Fix cross-platform help command and improve diff messages #36

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
61376d2
fix: cross-platform --help and beginner-friendly empty diff messages
Copilot Apr 19, 2026
f48d880
fix: add clarifying comment on HelpPrinter override rationale
Copilot Apr 19, 2026
b6b15cb
Merge pull request #1 from yankeeDamn/copilot/fix-help-command-and-er…
yankeeDamn Apr 19, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions cmd/app.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,24 @@ package cmd

import (
"fmt"
"io"

"github.com/urfave/cli/v2"
)

func init() {
// Override the default HelpPrinter to write directly to the provided
// io.Writer using fmt.Fprint. This avoids any OS-specific pager or
// man-page lookup and ensures --help works identically on Windows,
// macOS, and Linux.
// HelpPrinterCustom renders the help template directly into w via
// text/template.Execute — no external pager, man-page, or shell command
// is invoked, which is what makes this safe on Windows/PowerShell.
Comment on lines +11 to +17
Copy link

Copilot AI Apr 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment says the override writes help output using fmt.Fprint, but the implementation delegates to cli.HelpPrinterCustom (which already writes to io.Writer). Update the comment to match what the code actually does to avoid misleading future readers.

Suggested change
// Override the default HelpPrinter to write directly to the provided
// io.Writer using fmt.Fprint. This avoids any OS-specific pager or
// man-page lookup and ensures --help works identically on Windows,
// macOS, and Linux.
// HelpPrinterCustom renders the help template directly into w via
// text/template.Execute — no external pager, man-page, or shell command
// is invoked, which is what makes this safe on Windows/PowerShell.
// Override the default HelpPrinter to delegate to HelpPrinterCustom,
// which writes help output to the provided io.Writer without invoking
// any OS-specific pager, man-page lookup, or shell command. This keeps
// --help behavior consistent across Windows, macOS, and Linux and makes
// it safe on Windows/PowerShell.

Copilot uses AI. Check for mistakes.
cli.HelpPrinter = func(w io.Writer, templ string, data interface{}) {
cli.HelpPrinterCustom(w, templ, data, nil)
}
}

// Handlers contains injected command actions so CLI wiring can live outside main.
type Handlers struct {
RunReviewSimple cli.ActionFunc
Expand Down
23 changes: 21 additions & 2 deletions internal/appcore/review_runtime.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ func runReviewWithOptions(opts reviewopts.Options) error {
return fmt.Errorf("failed to collect diff for vouch: %w", diffErr)
}
if len(diffContent) == 0 {
return fmt.Errorf("no diff content to vouch for")
return fmt.Errorf("%s", noDiffMessage(opts.DiffSource))
Copy link

Copilot AI Apr 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fmt.Errorf("%s", noDiffMessage(...)) is redundant and can be replaced with errors.New(noDiffMessage(...)) (or fmt.Errorf("%s", ...) -> errors.New(...)) to avoid formatting-only error creation and keep this idiomatic.

Suggested change
return fmt.Errorf("%s", noDiffMessage(opts.DiffSource))
return errors.New(noDiffMessage(opts.DiffSource))

Copilot uses AI. Check for mistakes.
}
parsedFiles, parseErr := parseDiffToFiles(diffContent)
if parseErr != nil {
Expand Down Expand Up @@ -271,7 +271,7 @@ func runReviewWithOptions(opts reviewopts.Options) error {
}

if len(diffContent) == 0 {
return fmt.Errorf("no diff content collected")
return fmt.Errorf("%s", noDiffMessage(opts.DiffSource))
Copy link

Copilot AI Apr 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above: fmt.Errorf("%s", noDiffMessage(...)) can be simplified to errors.New(noDiffMessage(...)) to avoid a formatting-only fmt.Errorf call.

Suggested change
return fmt.Errorf("%s", noDiffMessage(opts.DiffSource))
return errors.New(noDiffMessage(opts.DiffSource))

Copilot uses AI. Check for mistakes.
}

var fakeBaseFiles []reviewmodel.DiffReviewFileResult
Expand Down Expand Up @@ -1368,6 +1368,25 @@ func collectDiffWithOptions(opts reviewopts.Options) ([]byte, error) {
}
}

// noDiffMessage returns a beginner-friendly error string tailored to the
// diff source that produced no content.
func noDiffMessage(diffSource string) string {
switch diffSource {
case "staged":
return "No staged changes found. Please stage your files using `git add <file>` before running a review."
case "working":
return "No working-tree changes found. Make some edits before running a review."
case "commit":
return "The specified commit(s) produced an empty diff. Verify the commit reference and try again."
case "range":
return "The specified range produced an empty diff. Verify the range and try again."
case "file":
return "The provided diff file is empty. Provide a non-empty diff file."
default:
return "No diff content collected."
}
}

// runCommitAndMaybePush commits the staged changes and optionally pushes with safety checks.
func runCommitAndMaybePush(message string, push bool, verbose bool) error {
msg := strings.TrimSpace(message)
Expand Down
Loading