-
Notifications
You must be signed in to change notification settings - Fork 19
fix #3 : enhance user feedback and error handling #63
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.
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
Conversation
Replace log.Fatal calls with pterm error displays for colored output
Add specific instructions for setting environment variables when API keys are missing
Update spinner message to display which LLM (Gemini, OpenAI, Claude, Grok) is being used
Display actionable guidance for staging changes, checking status, and verifying repository location
fix#3 : enhance user feedback and error handling
WalkthroughReplaces log.Fatal usage with pterm error/info outputs and explicit os.Exit(1) across error paths, updates header text, adds guidance when no changes are detected, customizes spinner message with selected LLM, and introduces per-LLM API failure guidance. Core logic for config, repo, stats, changes, and message generation remains unchanged. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant CLI as CLI (createMsg)
participant Repo as Git Repo
participant LLM as LLM Provider
U->>CLI: Run commit message generator
CLI->>CLI: Load config (LLM, repo)
alt Config error
CLI-->>U: pterm.Error (check setup) + exit(1)
end
CLI->>Repo: Get current dir, ensure repo
alt Not a repo / dir error
CLI-->>U: pterm.Error + exit(1)
end
CLI->>Repo: Collect file stats and changes
alt No changes
CLI-->>U: pterm.Info (nothing to commit) + exit(0/return)
end
CLI->>CLI: Start spinner ("Generating via <LLM>")
CLI->>LLM: Generate commit message
alt LLM/API error
CLI-->>U: pterm.Error (per-LLM env vars/setup) + exit(1)
else Success
CLI-->>U: Show generated message
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
cmd/cli/createMsg.go (2)
77-80: Extract duplicate tip messages to a helper function.The same four tip lines are repeated in two locations (lines 77-80 and 93-96). While these handle slightly different scenarios (no files tracked vs. no changes detected), the guidance is identical.
Consider extracting to a helper function:
+func showNoChangesGuidance() { + pterm.Info.Println("Tips:") + pterm.Info.Println(" - Stage your changes with: git add .") + pterm.Info.Println(" - Check repository status with: git status") + pterm.Info.Println(" - Make sure you're in the correct Git repository") +}Then replace both occurrences:
if fileStats.TotalFiles == 0 { pterm.Warning.Println("No changes detected in the Git repository.") - pterm.Info.Println("Tips:") - pterm.Info.Println(" - Stage your changes with: git add .") - pterm.Info.Println(" - Check repository status with: git status") - pterm.Info.Println(" - Make sure you're in the correct Git repository") + showNoChangesGuidance() return }if len(changes) == 0 { pterm.Warning.Println("No changes detected in the Git repository.") - pterm.Info.Println("Tips:") - pterm.Info.Println(" - Stage your changes with: git add .") - pterm.Info.Println(" - Check repository status with: git status") - pterm.Info.Println(" - Make sure you're in the correct Git repository") + showNoChangesGuidance() return }Also applies to: 93-96
142-154: Align error handling switch with generation switch.The generation switch (lines 113-137) uses
defaultfor Grok, but this error handling switch has an explicit"Grok"case. IfcommitLLMcontains an unexpected value, the code will call the Grok API (viadefault) but display a generic error message (viadefaulthere).For consistency, add an explicit
"Grok"case to the generation switch:case "Ollama": url := os.Getenv("OLLAMA_URL") if url == "" { url = "http://localhost:11434/api/generate" } model := os.Getenv("OLLAMA_MODEL") if model == "" { model = "llama3:latest" } commitMsg, err = ollama.GenerateCommitMessage(config, changes, url, model) + case "Grok": + commitMsg, err = grok.GenerateCommitMessage(config, changes, apiKey) default: - commitMsg, err = grok.GenerateCommitMessage(config, changes, apiKey) + pterm.Error.Printf("Unknown LLM: %s\n", commitLLM) + os.Exit(1) }This ensures consistent behavior: unknown LLM values fail fast rather than silently defaulting to Grok.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
cmd/cli/createMsg.go(6 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
cmd/cli/createMsg.go (1)
internal/git/operations.go (2)
IsRepository(16-23)GetChanges(26-130)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Package Binaries
🔇 Additional comments (3)
cmd/cli/createMsg.go (3)
26-27: LGTM! Clear and actionable error message.The replacement of
log.Fatalwithpterm.Error.Printfand explicitos.Exit(1)provides better visual feedback to users with colored output and a clear setup instruction.
37-38: LGTM! Consistent error handling pattern.The uniform replacement of
log.Fatalwithpterm.Error.Printfand explicitos.Exit(1)across all error paths provides consistent, user-friendly error messaging with colored output.Also applies to: 43-44, 60-61, 87-88, 107-108
105-105: LGTM! Enhanced user visibility.Dynamically including the LLM name in the spinner message provides better transparency about which provider is generating the commit message.
DFanso
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved 🎊
Summary by CodeRabbit
New Features
Style