Skip to content

Commit 3943bfe

Browse files
authored
[refactor] Codebase cleanup: reduce duplication and improve consistency (#86)
* [refactor] Small fixes: dead code, constants, flag consistency - Export errWorktreeNotFound from operations package, remove cmd/ duplicate - Use flagForce constant in rename.go instead of hardcoded "force" - Remove dead return value from parseCount in git/branch.go - Merge identical hintDryRunMerged/hintDryRunStale into hintDryRunClean - Remove stub Config.Validate() that always returned nil - Replace package-level flag vars in list.go with local GetBool/GetString calls * [refactor] Extract parallel collection helper Add generic internal/parallel.Collect[T] to replace repeated WaitGroup+semaphore pattern across 6 call sites (status, list, log, mcp/tool_status, mcp/tool_list, mcp/tool_sync). * [refactor] Unify JSON output types between CLI and MCP Move shared JSON types (ListItem, StatusData, ExecData, etc.) to internal/output/types.go. Both cmd/ and internal/mcp/ now import from the same source, eliminating duplicate struct definitions. * [refactor] Extract post-create setup from duplicate and restore Add operations.PostCreateSetup() to encapsulate the copy-files, install-deps, run-hooks sequence shared by add, duplicate, and restore. Reduces duplication and ensures consistent error messages. * [fix] Add fetch error warning to MCP sync handler Surface git fetch errors as a warning field in the MCP sync result instead of silently discarding them, matching the CLI behavior. * [chore] Make MCP command help platform-agnostic Remove Claude Code-specific configuration example from mcp command long description. * [refactor] Reduce parameter counts and add maxparams lint rule Refactor three 8-parameter functions down to 5 parameters each by bundling related args into structs, then add a custom lint rule to prevent regression.
1 parent 6000ef0 commit 3943bfe

37 files changed

Lines changed: 753 additions & 646 deletions

.golangci.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ linters:
3939
custom:
4040
rimbalint:
4141
type: "module"
42-
description: "Rimba custom lint rules (nolocalstruct, nolateexport)"
42+
description: "Rimba custom lint rules (maxparams, nolocalstruct, nolateexport)"
4343
decorder:
4444
dec-order:
4545
- type

cmd/clean.go

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,10 @@ const (
1818
flagMerged = "merged"
1919
flagStale = "stale"
2020

21-
hintMerged = "Remove worktrees whose branches are already merged into main"
22-
hintDryRunPrune = "Preview what would be pruned without making changes"
23-
hintDryRunMerged = "Preview what would be removed without making changes"
24-
hintDryRunStale = "Preview what would be removed without making changes"
25-
hintForce = "Skip confirmation prompt"
21+
hintMerged = "Remove worktrees whose branches are already merged into main"
22+
hintDryRunPrune = "Preview what would be pruned without making changes"
23+
hintDryRunClean = "Preview what would be removed without making changes"
24+
hintForce = "Skip confirmation prompt"
2625
)
2726

2827
func init() {
@@ -98,7 +97,7 @@ func cleanMerged(cmd *cobra.Command, r git.Runner) error {
9897
}
9998

10099
hint.New(cmd, hintPainter(cmd)).
101-
Add(flagDryRun, hintDryRunMerged).
100+
Add(flagDryRun, hintDryRunClean).
102101
Add(flagForce, hintForce).
103102
Show()
104103

@@ -160,7 +159,7 @@ func cleanStale(cmd *cobra.Command, r git.Runner) error {
160159
}
161160

162161
hint.New(cmd, hintPainter(cmd)).
163-
Add(flagDryRun, hintDryRunStale).
162+
Add(flagDryRun, hintDryRunClean).
164163
Add(flagForce, hintForce).
165164
Add(flagStaleDays, "Customize the staleness threshold (default: 14 days)").
166165
Show()

cmd/deps.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"github.com/lugassawan/rimba/internal/config"
88
"github.com/lugassawan/rimba/internal/deps"
99
"github.com/lugassawan/rimba/internal/git"
10+
"github.com/lugassawan/rimba/internal/operations"
1011
"github.com/lugassawan/rimba/internal/output"
1112
"github.com/lugassawan/rimba/internal/resolver"
1213
"github.com/lugassawan/rimba/internal/spinner"
@@ -172,7 +173,7 @@ var depsInstallCmd = &cobra.Command{
172173
}
173174
}
174175
if !found {
175-
return fmt.Errorf(errWorktreeNotFound, task)
176+
return fmt.Errorf(operations.ErrWorktreeNotFoundFmt, task)
176177
}
177178
}
178179

cmd/duplicate.go

Lines changed: 22 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,6 @@ import (
66
"path/filepath"
77

88
"github.com/lugassawan/rimba/internal/config"
9-
"github.com/lugassawan/rimba/internal/deps"
10-
"github.com/lugassawan/rimba/internal/fileutil"
119
"github.com/lugassawan/rimba/internal/git"
1210
"github.com/lugassawan/rimba/internal/hint"
1311
"github.com/lugassawan/rimba/internal/operations"
@@ -89,11 +87,6 @@ var duplicateCmd = &cobra.Command{
8987
wtDir := filepath.Join(repoRoot, cfg.WorktreeDir)
9088
wtPath := resolver.WorktreePath(wtDir, newBranch)
9189

92-
wtEntries, err := git.ListWorktrees(r)
93-
if err != nil {
94-
return err
95-
}
96-
9790
// Validate
9891
if git.BranchExists(r, newBranch) {
9992
return fmt.Errorf("branch %q already exists", newBranch)
@@ -117,36 +110,28 @@ var duplicateCmd = &cobra.Command{
117110
return err
118111
}
119112

120-
// Copy files
121-
s.Update("Copying files...")
122-
copied, err := fileutil.CopyEntries(repoRoot, wtPath, cfg.CopyFiles)
123-
if err != nil {
124-
return fmt.Errorf("worktree created but failed to copy files: %w\nTo retry, manually copy files to: %s\nTo remove the worktree: rimba remove %s", err, wtPath, newTask)
125-
}
126-
127-
// Dependencies — prefer cloning from source worktree
113+
// Post-create setup: copy files, deps, hooks
128114
skipDeps, _ := cmd.Flags().GetBool(flagSkipDeps)
115+
skipHooks, _ := cmd.Flags().GetBool(flagSkipHooks)
129116
var configModules []config.ModuleConfig
130117
if cfg.Deps != nil {
131118
configModules = cfg.Deps.Modules
132119
}
133120

134-
var depsResults []deps.InstallResult
135-
if !skipDeps {
136-
s.Update("Installing dependencies...")
137-
depsResults = operations.InstallDepsPreferSource(r, wtPath, wt.Path, cfg.IsAutoDetectDeps(), configModules, wtEntries, func(cur, total int, name string) {
138-
s.Update(fmt.Sprintf("Installing dependencies... (%s) [%d/%d]", name, cur, total))
139-
})
140-
}
141-
142-
// Post-create hooks
143-
skipHooks, _ := cmd.Flags().GetBool(flagSkipHooks)
144-
var hookResults []deps.HookResult
145-
if !skipHooks && len(cfg.PostCreate) > 0 {
146-
s.Update("Running hooks...")
147-
hookResults = operations.RunPostCreateHooks(wtPath, cfg.PostCreate, func(cur, total int, name string) {
148-
s.Update(fmt.Sprintf("Running hooks... (%s) [%d/%d]", name, cur, total))
149-
})
121+
pcResult, err := operations.PostCreateSetup(r, operations.PostCreateParams{
122+
RepoRoot: repoRoot,
123+
WtPath: wtPath,
124+
Task: newTask,
125+
CopyFiles: cfg.CopyFiles,
126+
SkipDeps: skipDeps,
127+
AutoDetect: cfg.IsAutoDetectDeps(),
128+
ConfigModules: configModules,
129+
SkipHooks: skipHooks,
130+
PostCreate: cfg.PostCreate,
131+
SourcePath: wt.Path,
132+
}, func(msg string) { s.Update(msg) })
133+
if err != nil {
134+
return err
150135
}
151136

152137
s.Stop()
@@ -155,15 +140,15 @@ var duplicateCmd = &cobra.Command{
155140
fmt.Fprintf(out, "Duplicated worktree %q as %q\n", task, newTask)
156141
fmt.Fprintf(out, " Branch: %s\n", newBranch)
157142
fmt.Fprintf(out, " Path: %s\n", wtPath)
158-
if len(copied) > 0 {
159-
fmt.Fprintf(out, " Copied: %v\n", copied)
143+
if len(pcResult.Copied) > 0 {
144+
fmt.Fprintf(out, " Copied: %v\n", pcResult.Copied)
160145
}
161-
if skipped := fileutil.SkippedEntries(cfg.CopyFiles, copied); len(skipped) > 0 {
162-
fmt.Fprintf(out, " Skipped (not found): %v\n", skipped)
146+
if len(pcResult.Skipped) > 0 {
147+
fmt.Fprintf(out, " Skipped (not found): %v\n", pcResult.Skipped)
163148
}
164149

165-
printInstallResults(out, depsResults)
166-
printHookResultsList(out, hookResults)
150+
printInstallResults(out, pcResult.DepsResults)
151+
printHookResultsList(out, pcResult.HookResults)
167152

168153
return nil
169154
},

cmd/exec.go

Lines changed: 3 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -19,23 +19,6 @@ import (
1919
"github.com/spf13/cobra"
2020
)
2121

22-
type execJSONData struct {
23-
Command string `json:"command"`
24-
Results []execJSONResult `json:"results"`
25-
Success bool `json:"success"`
26-
}
27-
28-
type execJSONResult struct {
29-
Task string `json:"task"`
30-
Branch string `json:"branch"`
31-
Path string `json:"path"`
32-
ExitCode int `json:"exit_code"`
33-
Stdout string `json:"stdout"`
34-
Stderr string `json:"stderr"`
35-
Error string `json:"error,omitempty"`
36-
Cancelled bool `json:"cancelled,omitempty"`
37-
}
38-
3922
const (
4023
flagFailFast = "fail-fast"
4124
flagConcurrency = "concurrency"
@@ -157,9 +140,9 @@ var execCmd = &cobra.Command{
157140
s.Stop()
158141

159142
if isJSON(cmd) {
160-
jsonResults := make([]execJSONResult, len(results))
143+
jsonResults := make([]output.ExecResult, len(results))
161144
for i, r := range results {
162-
jr := execJSONResult{
145+
jr := output.ExecResult{
163146
Task: r.Target.Task,
164147
Branch: r.Target.Branch,
165148
Path: r.Target.Path,
@@ -173,7 +156,7 @@ var execCmd = &cobra.Command{
173156
}
174157
jsonResults[i] = jr
175158
}
176-
data := execJSONData{
159+
data := output.ExecData{
177160
Command: args[0],
178161
Results: jsonResults,
179162
Success: !hasFailure(results),

cmd/exec_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -164,9 +164,9 @@ func TestPrintExecResultsJSON(t *testing.T) {
164164
}
165165

166166
// Simulate the JSON output path from exec command
167-
jsonResults := make([]execJSONResult, len(results))
167+
jsonResults := make([]output.ExecResult, len(results))
168168
for i, r := range results {
169-
jr := execJSONResult{
169+
jr := output.ExecResult{
170170
Task: r.Target.Task,
171171
Branch: r.Target.Branch,
172172
Path: r.Target.Path,
@@ -179,7 +179,7 @@ func TestPrintExecResultsJSON(t *testing.T) {
179179
}
180180
jsonResults[i] = jr
181181
}
182-
data := execJSONData{
182+
data := output.ExecData{
183183
Command: "echo hello",
184184
Results: jsonResults,
185185
Success: !hasFailure(results),

cmd/list.go

Lines changed: 21 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -5,34 +5,19 @@ import (
55
"os"
66
"path/filepath"
77
"strings"
8-
"sync"
98

109
"github.com/lugassawan/rimba/internal/config"
1110
"github.com/lugassawan/rimba/internal/git"
1211
"github.com/lugassawan/rimba/internal/hint"
1312
"github.com/lugassawan/rimba/internal/operations"
1413
"github.com/lugassawan/rimba/internal/output"
14+
"github.com/lugassawan/rimba/internal/parallel"
1515
"github.com/lugassawan/rimba/internal/resolver"
1616
"github.com/lugassawan/rimba/internal/spinner"
1717
"github.com/lugassawan/rimba/internal/termcolor"
1818
"github.com/spf13/cobra"
1919
)
2020

21-
type listJSONItem struct {
22-
Task string `json:"task"`
23-
Type string `json:"type"`
24-
Branch string `json:"branch"`
25-
Path string `json:"path"`
26-
IsCurrent bool `json:"is_current"`
27-
Status resolver.WorktreeStatus `json:"status"`
28-
}
29-
30-
type listArchivedJSONItem struct {
31-
Task string `json:"task"`
32-
Type string `json:"type"`
33-
Branch string `json:"branch"`
34-
}
35-
3621
const (
3722
flagType = "type"
3823
flagDirty = "dirty"
@@ -51,20 +36,13 @@ type candidate struct {
5136
isCurrent bool
5237
}
5338

54-
var (
55-
listType string
56-
listDirty bool
57-
listBehind bool
58-
listArchived bool
59-
)
60-
6139
func init() {
6240
rootCmd.AddCommand(listCmd)
6341

64-
listCmd.Flags().StringVar(&listType, flagType, "", "filter by prefix type (e.g. feature, bugfix)")
65-
listCmd.Flags().BoolVar(&listDirty, flagDirty, false, "show only dirty worktrees")
66-
listCmd.Flags().BoolVar(&listBehind, flagBehind, false, "show only worktrees behind upstream")
67-
listCmd.Flags().BoolVar(&listArchived, flagArchived, false, "show archived branches (not in any active worktree)")
42+
listCmd.Flags().String(flagType, "", "filter by prefix type (e.g. feature, bugfix)")
43+
listCmd.Flags().Bool(flagDirty, false, "show only dirty worktrees")
44+
listCmd.Flags().Bool(flagBehind, false, "show only worktrees behind upstream")
45+
listCmd.Flags().Bool(flagArchived, false, "show archived branches (not in any active worktree)")
6846

6947
listCmd.MarkFlagsMutuallyExclusive(flagArchived, flagType)
7048
listCmd.MarkFlagsMutuallyExclusive(flagArchived, flagDirty)
@@ -87,6 +65,11 @@ var listCmd = &cobra.Command{
8765
Short: "List all worktrees",
8866
Long: "Lists all git worktrees with their branch, path, and status (dirty, ahead/behind).",
8967
RunE: func(cmd *cobra.Command, args []string) error {
68+
listType, _ := cmd.Flags().GetString(flagType)
69+
listDirty, _ := cmd.Flags().GetBool(flagDirty)
70+
listBehind, _ := cmd.Flags().GetBool(flagBehind)
71+
listArchived, _ := cmd.Flags().GetBool(flagArchived)
72+
9073
if listArchived {
9174
r := newRunner()
9275
mainBranch, err := resolveMainBranch(r)
@@ -120,7 +103,7 @@ var listCmd = &cobra.Command{
120103

121104
if len(entries) == 0 {
122105
if isJSON(cmd) {
123-
return output.WriteJSON(cmd.OutOrStdout(), version, "list", make([]listJSONItem, 0))
106+
return output.WriteJSON(cmd.OutOrStdout(), version, "list", make([]output.ListItem, 0))
124107
}
125108
fmt.Fprintln(cmd.OutOrStdout(), "No worktrees found.")
126109
return nil
@@ -172,31 +155,19 @@ var listCmd = &cobra.Command{
172155
candidates = append(candidates, candidate{entry: e, displayPath: displayPath, isCurrent: isCurrent})
173156
}
174157

175-
rows := make([]resolver.WorktreeDetail, len(candidates))
176-
var wg sync.WaitGroup
177-
sem := make(chan struct{}, 8)
178-
179-
for i, c := range candidates {
180-
s.Update(fmt.Sprintf("Loading worktrees... (%d/%d)", i+1, len(candidates)))
181-
wg.Add(1)
182-
go func(idx int, c candidate) {
183-
defer wg.Done()
184-
sem <- struct{}{}
185-
defer func() { <-sem }()
186-
187-
status := operations.CollectWorktreeStatus(r, c.entry.Path)
188-
rows[idx] = resolver.NewWorktreeDetail(c.entry.Branch, prefixes, c.displayPath, status, c.isCurrent)
189-
}(i, c)
190-
}
191-
wg.Wait()
158+
rows := parallel.Collect(len(candidates), 8, func(i int) resolver.WorktreeDetail {
159+
c := candidates[i]
160+
status := operations.CollectWorktreeStatus(r, c.entry.Path)
161+
return resolver.NewWorktreeDetail(c.entry.Branch, prefixes, c.displayPath, status, c.isCurrent)
162+
})
192163

193164
rows = operations.FilterDetailsByStatus(rows, listDirty, listBehind)
194165

195166
s.Stop()
196167

197168
if len(rows) == 0 {
198169
if isJSON(cmd) {
199-
return output.WriteJSON(cmd.OutOrStdout(), version, "list", make([]listJSONItem, 0))
170+
return output.WriteJSON(cmd.OutOrStdout(), version, "list", make([]output.ListItem, 0))
200171
}
201172
fmt.Fprintln(cmd.OutOrStdout(), "No worktrees match the given filters.")
202173
return nil
@@ -205,9 +176,9 @@ var listCmd = &cobra.Command{
205176
resolver.SortDetailsByTask(rows)
206177

207178
if isJSON(cmd) {
208-
items := make([]listJSONItem, len(rows))
179+
items := make([]output.ListItem, len(rows))
209180
for i, r := range rows {
210-
items[i] = listJSONItem{
181+
items[i] = output.ListItem{
211182
Task: r.Task,
212183
Type: r.Type,
213184
Branch: r.Branch,
@@ -264,11 +235,11 @@ func listArchivedBranches(cmd *cobra.Command, r git.Runner, mainBranch string) e
264235
prefixes := resolver.AllPrefixes()
265236

266237
if isJSON(cmd) {
267-
items := make([]listArchivedJSONItem, 0, len(archived))
238+
items := make([]output.ListArchivedItem, 0, len(archived))
268239
for _, b := range archived {
269240
task, matchedPrefix := resolver.TaskFromBranch(b, prefixes)
270241
typeName := strings.TrimSuffix(matchedPrefix, "/")
271-
items = append(items, listArchivedJSONItem{Task: task, Type: typeName, Branch: b})
242+
items = append(items, output.ListArchivedItem{Task: task, Type: typeName, Branch: b})
272243
}
273244
return output.WriteJSON(cmd.OutOrStdout(), version, "list", items)
274245
}

0 commit comments

Comments
 (0)