Fix cross-platform help command and improve diff messages#36
Open
yankeeDamn wants to merge 3 commits intoHexmosTech:mainfrom
Open
Fix cross-platform help command and improve diff messages#36yankeeDamn wants to merge 3 commits intoHexmosTech:mainfrom
yankeeDamn wants to merge 3 commits intoHexmosTech:mainfrom
Conversation
Issue 1: Override cli.HelpPrinter in cmd/app.go to use a direct fmt.Fprint-based renderer, avoiding any OS-specific pager or man-page lookup that can fail on Windows/PowerShell. Issue 2: Replace vague "no diff content collected" with diff-source-aware messages. For staged diffs (the default), users now see: "No staged changes found. Please stage your files using `git add <file>` before running a review." Agent-Logs-Url: https://github.com/yankeeDamn/git-lrc/sessions/b7e07e01-34b2-4026-b457-3ebd4b059163 Co-authored-by: yankeeDamn <74879019+yankeeDamn@users.noreply.github.com>
Agent-Logs-Url: https://github.com/yankeeDamn/git-lrc/sessions/b7e07e01-34b2-4026-b457-3ebd4b059163 Co-authored-by: yankeeDamn <74879019+yankeeDamn@users.noreply.github.com>
…ror-message Fix cross-platform --help and improve empty diff error messages
There was a problem hiding this comment.
Pull request overview
This PR improves CLI user experience when a review is requested but no diff content is available, and makes --help rendering consistent across operating systems by overriding the default urfave/cli help printer.
Changes:
- Replace generic “no diff content” errors with diff-source-specific guidance via
noDiffMessage. - Use the new
noDiffMessagein both vouch and main diff collection flows. - Override
cli.HelpPrinterto delegate tocli.HelpPrinterCustomfor consistent cross-platform help output.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| internal/appcore/review_runtime.go | Adds noDiffMessage and uses it when collected diff content is empty. |
| cmd/app.go | Overrides urfave/cli help printer to ensure consistent help output across OSes. |
| } | ||
| if len(diffContent) == 0 { | ||
| return fmt.Errorf("no diff content to vouch for") | ||
| return fmt.Errorf("%s", noDiffMessage(opts.DiffSource)) |
There was a problem hiding this comment.
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)) |
|
|
||
| if len(diffContent) == 0 { | ||
| return fmt.Errorf("no diff content collected") | ||
| return fmt.Errorf("%s", noDiffMessage(opts.DiffSource)) |
There was a problem hiding this comment.
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)) |
Comment on lines
+11
to
+17
| // 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. |
There was a problem hiding this comment.
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. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request improves the user experience when no diff content is found during a review and enhances cross-platform help output consistency. The most important changes include providing more beginner-friendly, context-aware error messages when no diff is available, and ensuring help output works identically across operating systems.
User experience improvements for empty diffs:
noDiffMessagefunction inreview_runtime.goto generate tailored, beginner-friendly error messages depending on the diff source (e.g., staged, working, commit, range, file). This replaces generic error messages when no diff content is found.runReviewWithOptionsto use the newnoDiffMessagefunction for both vouch and main diff collection paths, ensuring users receive clear instructions on what to do next. [1] [2]Cross-platform CLI help consistency:
HelpPrinterincmd/app.goto useHelpPrinterCustom, ensuring that--helpoutput is rendered directly to the terminal without invoking any external pagers or shell commands. This guarantees identical behavior on Windows, macOS, and Linux.