Refactor: Extract internal modules for better organization #10
Conversation
- Moved Git operations, stats, and display functions to - src/internal/git, src/internal/stats, and src/internal/display. - Added src/internal/utils for utilities. Updated README.md with - CodeRabbit badge to enhance documentation.
WalkthroughAdds internal packages for display, git operations, stats, and utilities; refactors main to use these APIs and environment-based API key handling; introduces spinner-based progress and a README badge update. New exported types/functions provide file statistics gathering, git change aggregation, and styled terminal rendering. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Main as main.go
participant Git as internal/git
participant Stats as internal/stats
participant Disp as internal/display
participant Utils as internal/utils
User->>Main: run CLI
Main->>Git: IsRepository(path)
Git-->>Main: bool
alt not a repo
Main-->>User: exit with message
else repo
Main->>Stats: GetFileStatistics(config)
Stats->>Git: run git diff/ls-files/numstat
Stats->>Utils: normalize/filter results
Stats-->>Main: FileStatistics
Main->>Disp: ShowFileStatistics(stats)
Main->>Git: GetChanges(config)
Git->>Utils: check files (IsTextFile/IsSmallFile)
Git-->>Main: aggregated changes string
Main->>Disp: ShowCommitMessage(message)
Main->>Disp: ShowChangesPreview(stats)
Main-->>User: rendered output
end
note right of Main: spinner shown during operations
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧬 Code graph analysis (1)src/internal/git/operations.go (2)
🔇 Additional comments (3)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/main.go (2)
114-117: Simplify error handling on commit message generation failure.The spinner failure message may not be visible before
log.Fatalfterminates the program. Consider removing the spinner failure call or usingpterm.Errorinstead oflog.Fatalfto ensure the spinner message is displayed.Apply this diff:
if err != nil { - spinnerGenerating.Fail("Failed to generate commit message") - log.Fatalf("Error: %v", err) + spinnerGenerating.Fail(fmt.Sprintf("Failed to generate commit message: %v", err)) + os.Exit(1) }
22-23: Replace “google” with “gemini” in COMMIT_LLM checks
Update src/main.go (lines 22 and 106) to check forCOMMIT_LLM == "gemini"and readGEMINI_API_KEY(not"google"/GOOGLE_API_KEY) so it aligns with README.
🧹 Nitpick comments (5)
src/internal/utils/utils.go (3)
10-16: Consider using filepath.ToSlash for idiomatic path normalization.The current implementation manually replaces backslashes, but Go's
filepath.ToSlash()provides the same functionality in a more idiomatic way.Apply this diff to use the standard library function:
-// NormalizePath handles both forward and backslashes func NormalizePath(path string) string { - // Replace backslashes with forward slashes - normalized := strings.ReplaceAll(path, "\\", "/") + // Use filepath.ToSlash for cross-platform path normalization + normalized := filepath.ToSlash(path) // Remove any trailing slash normalized = strings.TrimSuffix(normalized, "/") return normalized }
19-35: Consider expanding the text file extension list.The current list covers common cases, but consider adding extensions like
.sql,.swift,.kt,.scala,.r,.groovyfor broader language support.This is a low-priority enhancement that can be deferred.
50-58: Consider filtering whitespace-only strings.The function currently only filters empty strings (
""), but git output might contain whitespace-only entries. Consider usingstrings.TrimSpace(s) != ""for more robust filtering.Apply this diff if whitespace-only strings should be filtered:
func FilterEmpty(slice []string) []string { filtered := []string{} for _, s := range slice { - if s != "" { + if strings.TrimSpace(s) != "" { filtered = append(filtered, s) } } return filtered }src/internal/git/operations.go (1)
25-117: Consider limiting the size of the returned change summary.For repositories with large changesets, the returned string could become very large and cause memory issues. Consider implementing a size limit or truncation strategy.
Example approach:
const maxChangeSize = 1024 * 1024 // 1MB limit func GetChanges(config *types.RepoConfig) (string, error) { var changes strings.Builder // ... existing logic ... result := changes.String() if len(result) > maxChangeSize { return result[:maxChangeSize] + "\n\n... (truncated due to size)", nil } return result, nil }src/internal/display/display.go (1)
26-93: Consider extracting the repetitive bullet list logic.The code follows the same pattern for staged, unstaged, and untracked files (create header item, add files up to limit, add "more" indicator). This repetition could be reduced with a helper function.
Example refactor:
func addFileSection(bulletItems *[]pterm.BulletListItem, files []string, label string, color pterm.Color, icon string, limit int) { if len(files) == 0 { return } style := pterm.NewStyle(color) *bulletItems = append(*bulletItems, pterm.BulletListItem{ Level: 0, Text: style.Sprint(fmt.Sprintf("%s %s: %d", icon, label, len(files))), TextStyle: style, BulletStyle: style, }) for i, file := range files { if i < limit { *bulletItems = append(*bulletItems, pterm.BulletListItem{ Level: 1, Text: file, }) } } if len(files) > limit { *bulletItems = append(*bulletItems, pterm.BulletListItem{ Level: 1, Text: pterm.Gray(fmt.Sprintf("... and %d more", len(files)-limit)), }) } } func ShowFileStatistics(stats *FileStatistics) { pterm.DefaultSection.Println("📊 Changes Summary") bulletItems := []pterm.BulletListItem{} addFileSection(&bulletItems, stats.StagedFiles, "Staged files", pterm.FgGreen, "✅", 5) addFileSection(&bulletItems, stats.UnstagedFiles, "Unstaged files", pterm.FgYellow, "⚠️", 3) addFileSection(&bulletItems, stats.UntrackedFiles, "Untracked files", pterm.FgCyan, "📝", 3) pterm.DefaultBulletList.WithItems(bulletItems).Render() }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
README.md(1 hunks)src/internal/display/display.go(1 hunks)src/internal/git/operations.go(1 hunks)src/internal/stats/statistics.go(1 hunks)src/internal/utils/utils.go(1 hunks)src/main.go(6 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
src/internal/stats/statistics.go (3)
src/types/types.go (1)
RepoConfig(10-13)src/internal/display/display.go (1)
FileStatistics(10-17)src/internal/utils/utils.go (1)
FilterEmpty(50-58)
src/internal/git/operations.go (2)
src/types/types.go (1)
RepoConfig(10-13)src/internal/utils/utils.go (2)
IsTextFile(19-35)IsSmallFile(38-47)
src/main.go (3)
src/internal/git/operations.go (2)
IsRepository(15-22)GetChanges(25-117)src/internal/stats/statistics.go (1)
GetFileStatistics(14-74)src/internal/display/display.go (3)
ShowFileStatistics(20-96)ShowCommitMessage(99-115)ShowChangesPreview(118-133)
⏰ 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: Build Go Binary (windows-latest)
🔇 Additional comments (4)
src/internal/git/operations.go (1)
15-22: LGTM!The implementation correctly checks if a directory is a git repository using the standard git command. Error handling is appropriate for a boolean check function.
src/internal/display/display.go (2)
99-115: LGTM!The commit message display implementation follows pterm best practices with appropriate styling and clear visual hierarchy.
Based on learnings.
118-133: LGTM!The changes preview implementation appropriately handles both cases (with and without line statistics) and provides clear visual feedback.
src/internal/stats/statistics.go (1)
56-68: Numstat parsing with strings.Fields is safe
strings.Fields correctly splits on tabs to yield the added/deleted counts; filenames with spaces aren’t used here.
| stagedCmd := exec.Command("git", "-C", config.Path, "diff", "--name-only", "--cached") | ||
| stagedOutput, err := stagedCmd.Output() | ||
| if err == nil && len(stagedOutput) > 0 { | ||
| stats.StagedFiles = strings.Split(strings.TrimSpace(string(stagedOutput)), "\n") | ||
| } | ||
|
|
||
| // Get unstaged files | ||
| unstagedCmd := exec.Command("git", "-C", config.Path, "diff", "--name-only") | ||
| unstagedOutput, err := unstagedCmd.Output() | ||
| if err == nil && len(unstagedOutput) > 0 { | ||
| stats.UnstagedFiles = strings.Split(strings.TrimSpace(string(unstagedOutput)), "\n") | ||
| } | ||
|
|
||
| // Get untracked files | ||
| untrackedCmd := exec.Command("git", "-C", config.Path, "ls-files", "--others", "--exclude-standard") | ||
| untrackedOutput, err := untrackedCmd.Output() | ||
| if err == nil && len(untrackedOutput) > 0 { | ||
| stats.UntrackedFiles = strings.Split(strings.TrimSpace(string(untrackedOutput)), "\n") | ||
| } |
There was a problem hiding this comment.
Propagate git command errors to the caller.
The function silently ignores all errors from git commands. If git is not installed, the repository is corrupted, or there's a permission issue, the function returns empty statistics without any indication of failure. This makes debugging difficult and could mask critical issues.
Consider propagating the first encountered error:
// Get staged files
stagedCmd := exec.Command("git", "-C", config.Path, "diff", "--name-only", "--cached")
stagedOutput, err := stagedCmd.Output()
- if err == nil && len(stagedOutput) > 0 {
+ if err != nil {
+ return nil, fmt.Errorf("failed to get staged files: %w", err)
+ }
+ if len(stagedOutput) > 0 {
stats.StagedFiles = strings.Split(strings.TrimSpace(string(stagedOutput)), "\n")
}
// Get unstaged files
unstagedCmd := exec.Command("git", "-C", config.Path, "diff", "--name-only")
unstagedOutput, err := unstagedCmd.Output()
- if err == nil && len(unstagedOutput) > 0 {
+ if err != nil {
+ return nil, fmt.Errorf("failed to get unstaged files: %w", err)
+ }
+ if len(unstagedOutput) > 0 {
stats.UnstagedFiles = strings.Split(strings.TrimSpace(string(unstagedOutput)), "\n")
}
// Get untracked files
untrackedCmd := exec.Command("git", "-C", config.Path, "ls-files", "--others", "--exclude-standard")
untrackedOutput, err := untrackedCmd.Output()
- if err == nil && len(untrackedOutput) > 0 {
+ if err != nil {
+ return nil, fmt.Errorf("failed to get untracked files: %w", err)
+ }
+ if len(untragedOutput) > 0 {
stats.UntrackedFiles = strings.Split(strings.TrimSpace(string(untrackedOutput)), "\n")
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/internal/stats/statistics.go around lines 22 to 40, the code currently
ignores errors from the git exec.Command.Output() calls; update the function to
propagate the first encountered error back to the caller instead of silently
continuing: after each Output() call check if err != nil and immediately return
the error (or wrap it with context), otherwise process the output as currently
done (trim/split and assign to stats fields only when non-empty); ensure the
function signature is updated to return an error if it doesn't already and
include contextual messages (e.g., which git subcommand failed) when returning
the error.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Fix #113 : add token count, cost estimation, and processing time to dry run mode
Description
This PR significantly enhances the commit message generator with a comprehensive UI overhaul, automatic clipboard functionality, and major code refactoring. The changes provide users with clear, colorful feedback about their Git repository state, automatically copy generated messages to clipboard, and establish a clean, maintainable codebase through modular architecture.
Type of Change
Changes Made
Code Improvements & Refactoring
Architecture Improvements
main.go(481 lines) into organized modulesinternal/directory following Go best practicesData Structures
Refactored Functions (with proper encapsulation)
Display Package (
internal/display):ShowFileStatistics(): Renders file stats with pterm stylingShowCommitMessage(): Displays commit message in styled panelShowChangesPreview(): Shows line change statistics tableGit Package (
internal/git):IsRepository(): Validates Git repositoryGetChanges(): Retrieves all Git changes (staged, unstaged, untracked)Stats Package (
internal/stats):GetFileStatistics(): Collects comprehensive file statistics from GitUtils Package (
internal/utils):NormalizePath(): Cross-platform path normalizationIsTextFile(): Text file detection by extensionIsSmallFile(): Size-based file filteringFilterEmpty(): String slice cleanup utilityBenefits of Refactoring
Dependency Updates
github.com/atotto/clipboard v0.1.4for clipboard functionalitygo.modandgo.sumwith new dependencies and reordered entriesCommits Included
This PR includes the following commits:
refactor: reorganize code into modular package structure
Testing
Test Scenarios Verified
Checklist
Additional Notes
Technical Details
internal/packagesBreaking Changes
None. This is a pure enhancement that maintains full backward compatibility. No API changes or functionality modifications.
For Hacktoberfest Participants
Thank you for your contribution! 🎉
Summary by CodeRabbit
New Features
Refactor
Documentation