Skip to content

Commit 81ce37c

Browse files
authored
[feat] Add progress indicators for long-running operations (#78)
* [feat] Add progress callback and capture output in deps package Define ProgressFunc type and thread it through Install(), InstallPreferSource(), and RunPostCreateHooks(). Replace cmd.Stdout/Stderr = os.Stdout/Stderr with bytes.Buffer capture to prevent spinner interference, surfacing output only in errors. * [feat] Wire progress indicators into all commands Update installDeps, installDepsPreferSource, and runHooks to accept ProgressFunc. Add progress closures in add, restore, duplicate, and deps install commands to show per-item [n/m] spinner updates. * [test] Add tests for progress callbacks and output capture Add TestInstallProgressCallback, TestInstallOutputCapture, TestRunPostCreateHooksProgressCallback, and TestRunPostCreateHooksOutputCapture. Update all existing calls with nil progress argument.
1 parent 8102669 commit 81ce37c

10 files changed

Lines changed: 187 additions & 59 deletions

File tree

cmd/add.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -99,15 +99,19 @@ var addCmd = &cobra.Command{
9999
var depsResults []deps.InstallResult
100100
if !skipDeps {
101101
s.Update("Installing dependencies...")
102-
depsResults = installDeps(r, cfg, wtPath, wtEntries)
102+
depsResults = installDeps(r, cfg, wtPath, wtEntries, func(cur, total int, name string) {
103+
s.Update(fmt.Sprintf("Installing dependencies... (%s) [%d/%d]", name, cur, total))
104+
})
103105
}
104106

105107
// Post-create hooks
106108
skipHooks, _ := cmd.Flags().GetBool(flagSkipHooks)
107109
var hookResults []deps.HookResult
108110
if !skipHooks && len(cfg.PostCreate) > 0 {
109111
s.Update("Running hooks...")
110-
hookResults = runHooks(wtPath, cfg.PostCreate)
112+
hookResults = runHooks(wtPath, cfg.PostCreate, func(cur, total int, name string) {
113+
s.Update(fmt.Sprintf("Running hooks... (%s) [%d/%d]", name, cur, total))
114+
})
111115
}
112116

113117
s.Stop()

cmd/deps.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,9 @@ var depsInstallCmd = &cobra.Command{
196196

197197
s.Start("Installing dependencies...")
198198
mgr := &deps.Manager{Runner: r}
199-
results := mgr.Install(wt.Path, modules)
199+
results := mgr.Install(wt.Path, modules, func(cur, total int, name string) {
200+
s.Update(fmt.Sprintf("Installing dependencies... (%s) [%d/%d]", name, cur, total))
201+
})
200202
s.Stop()
201203

202204
out := cmd.OutOrStdout()

cmd/deps_helpers.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import (
1212

1313
// installDeps detects modules and installs dependencies, returning the results.
1414
// existingEntries should contain the current worktree list to avoid a redundant git call.
15-
func installDeps(r git.Runner, cfg *config.Config, wtPath string, existingEntries []git.WorktreeEntry) []deps.InstallResult {
15+
func installDeps(r git.Runner, cfg *config.Config, wtPath string, existingEntries []git.WorktreeEntry, onProgress deps.ProgressFunc) []deps.InstallResult {
1616
var configModules []config.ModuleConfig
1717
if cfg.Deps != nil {
1818
configModules = cfg.Deps.Modules
@@ -26,12 +26,12 @@ func installDeps(r git.Runner, cfg *config.Config, wtPath string, existingEntrie
2626
}
2727

2828
mgr := &deps.Manager{Runner: r}
29-
return mgr.Install(wtPath, modules)
29+
return mgr.Install(wtPath, modules, onProgress)
3030
}
3131

3232
// installDepsPreferSource is like installDeps but prefers cloning from sourceWT.
3333
// existingEntries should contain the current worktree list to avoid a redundant git call.
34-
func installDepsPreferSource(r git.Runner, cfg *config.Config, wtPath, sourceWT string, existingEntries []git.WorktreeEntry) []deps.InstallResult {
34+
func installDepsPreferSource(r git.Runner, cfg *config.Config, wtPath, sourceWT string, existingEntries []git.WorktreeEntry, onProgress deps.ProgressFunc) []deps.InstallResult {
3535
var configModules []config.ModuleConfig
3636
if cfg.Deps != nil {
3737
configModules = cfg.Deps.Modules
@@ -45,7 +45,7 @@ func installDepsPreferSource(r git.Runner, cfg *config.Config, wtPath, sourceWT
4545
}
4646

4747
mgr := &deps.Manager{Runner: r}
48-
return mgr.InstallPreferSource(wtPath, sourceWT, modules)
48+
return mgr.InstallPreferSource(wtPath, sourceWT, modules, onProgress)
4949
}
5050

5151
// worktreePathsExcluding returns paths from entries, excluding the given path.
@@ -78,8 +78,8 @@ func printInstallResults(out io.Writer, results []deps.InstallResult) {
7878
}
7979

8080
// runHooks executes post-create hooks and returns the results.
81-
func runHooks(wtPath string, hooks []string) []deps.HookResult {
82-
return deps.RunPostCreateHooks(wtPath, hooks)
81+
func runHooks(wtPath string, hooks []string, onProgress deps.ProgressFunc) []deps.HookResult {
82+
return deps.RunPostCreateHooks(wtPath, hooks, onProgress)
8383
}
8484

8585
// printHookResultsList prints pre-computed hook results.

cmd/deps_helpers_test.go

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ func TestInstallDeps(t *testing.T) {
151151
{Path: newWT, Branch: "feature/test"},
152152
}
153153

154-
results := installDeps(r, cfg, newWT, entries)
154+
results := installDeps(r, cfg, newWT, entries, nil)
155155
if len(results) == 0 {
156156
t.Fatal("expected at least 1 result from installDeps")
157157
}
@@ -179,7 +179,7 @@ func TestInstallDepsNoLockfiles(t *testing.T) {
179179
Deps: &config.DepsConfig{AutoDetect: boolPtr(true)},
180180
}
181181

182-
results := installDeps(r, cfg, newWT, nil)
182+
results := installDeps(r, cfg, newWT, nil, nil)
183183
if results != nil {
184184
t.Errorf("expected nil results for no lockfiles, got %v", results)
185185
}
@@ -219,7 +219,7 @@ func TestInstallDepsPreferSource(t *testing.T) {
219219
{Path: newWT, Branch: "feature/copy"},
220220
}
221221

222-
results := installDepsPreferSource(r, cfg, newWT, sourceWT, entries)
222+
results := installDepsPreferSource(r, cfg, newWT, sourceWT, entries, nil)
223223
if len(results) == 0 {
224224
t.Fatal("expected at least 1 result")
225225
}
@@ -244,7 +244,7 @@ func TestInstallDepsPreferSourceNoLockfiles(t *testing.T) {
244244
Deps: &config.DepsConfig{AutoDetect: boolPtr(true)},
245245
}
246246

247-
results := installDepsPreferSource(r, cfg, newWT, sourceWT, nil)
247+
results := installDepsPreferSource(r, cfg, newWT, sourceWT, nil, nil)
248248
if results != nil {
249249
t.Errorf("expected nil results for no lockfiles, got %v", results)
250250
}
@@ -260,7 +260,7 @@ func TestInstallDepsNilDepsConfig(t *testing.T) {
260260

261261
cfg := &config.Config{} // Deps is nil
262262

263-
results := installDeps(r, cfg, newWT, nil)
263+
results := installDeps(r, cfg, newWT, nil, nil)
264264
if results != nil {
265265
t.Errorf("expected nil results for nil Deps config, got %v", results)
266266
}
@@ -277,7 +277,7 @@ func TestInstallDepsPreferSourceNilDepsConfig(t *testing.T) {
277277

278278
cfg := &config.Config{} // Deps is nil
279279

280-
results := installDepsPreferSource(r, cfg, newWT, sourceWT, nil)
280+
results := installDepsPreferSource(r, cfg, newWT, sourceWT, nil, nil)
281281
if results != nil {
282282
t.Errorf("expected nil results for nil Deps config, got %v", results)
283283
}
@@ -286,7 +286,7 @@ func TestInstallDepsPreferSourceNilDepsConfig(t *testing.T) {
286286
func TestRunHooks(t *testing.T) {
287287
dir := t.TempDir()
288288

289-
results := runHooks(dir, []string{"touch marker.txt"})
289+
results := runHooks(dir, []string{"touch marker.txt"}, nil)
290290
if len(results) != 1 {
291291
t.Fatalf("expected 1 result, got %d", len(results))
292292
}
@@ -302,7 +302,7 @@ func TestRunHooks(t *testing.T) {
302302
func TestRunHooksFailure(t *testing.T) {
303303
dir := t.TempDir()
304304

305-
results := runHooks(dir, []string{"false"})
305+
results := runHooks(dir, []string{"false"}, nil)
306306
if len(results) != 1 {
307307
t.Fatalf("expected 1 result, got %d", len(results))
308308
}

cmd/duplicate.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -128,15 +128,19 @@ var duplicateCmd = &cobra.Command{
128128
var depsResults []deps.InstallResult
129129
if !skipDeps {
130130
s.Update("Installing dependencies...")
131-
depsResults = installDepsPreferSource(r, cfg, wtPath, wt.Path, wtEntries)
131+
depsResults = installDepsPreferSource(r, cfg, wtPath, wt.Path, wtEntries, func(cur, total int, name string) {
132+
s.Update(fmt.Sprintf("Installing dependencies... (%s) [%d/%d]", name, cur, total))
133+
})
132134
}
133135

134136
// Post-create hooks
135137
skipHooks, _ := cmd.Flags().GetBool(flagSkipHooks)
136138
var hookResults []deps.HookResult
137139
if !skipHooks && len(cfg.PostCreate) > 0 {
138140
s.Update("Running hooks...")
139-
hookResults = runHooks(wtPath, cfg.PostCreate)
141+
hookResults = runHooks(wtPath, cfg.PostCreate, func(cur, total int, name string) {
142+
s.Update(fmt.Sprintf("Running hooks... (%s) [%d/%d]", name, cur, total))
143+
})
140144
}
141145

142146
s.Stop()

cmd/restore.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,15 +78,19 @@ var restoreCmd = &cobra.Command{
7878
var depsResults []deps.InstallResult
7979
if !skipDeps {
8080
s.Update("Installing dependencies...")
81-
depsResults = installDeps(r, cfg, wtPath, wtEntries)
81+
depsResults = installDeps(r, cfg, wtPath, wtEntries, func(cur, total int, name string) {
82+
s.Update(fmt.Sprintf("Installing dependencies... (%s) [%d/%d]", name, cur, total))
83+
})
8284
}
8385

8486
// Post-create hooks
8587
skipHooks, _ := cmd.Flags().GetBool(flagSkipHooks)
8688
var hookResults []deps.HookResult
8789
if !skipHooks && len(cfg.PostCreate) > 0 {
8890
s.Update("Running hooks...")
89-
hookResults = runHooks(wtPath, cfg.PostCreate)
91+
hookResults = runHooks(wtPath, cfg.PostCreate, func(cur, total int, name string) {
92+
s.Update(fmt.Sprintf("Running hooks... (%s) [%d/%d]", name, cur, total))
93+
})
9094
}
9195

9296
s.Stop()

internal/deps/hooks.go

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
package deps
22

33
import (
4+
"bytes"
45
"fmt"
5-
"os"
66
"os/exec"
7+
"strings"
78
)
89

910
// HookResult holds the outcome of a post-create hook execution.
@@ -14,17 +15,23 @@ type HookResult struct {
1415

1516
// RunPostCreateHooks executes shell commands in the worktree directory.
1617
// It collects errors but does not stop on failure — all hooks run regardless.
17-
func RunPostCreateHooks(worktreeDir string, hooks []string) []HookResult {
18+
func RunPostCreateHooks(worktreeDir string, hooks []string, onProgress ProgressFunc) []HookResult {
1819
results := make([]HookResult, 0, len(hooks))
19-
for _, hook := range hooks {
20-
cmd := exec.Command("sh", "-c", hook)
20+
for i, hook := range hooks {
21+
if onProgress != nil {
22+
onProgress(i+1, len(hooks), hook)
23+
}
24+
25+
cmd := exec.Command("sh", "-c", hook) //nolint:gosec // hook commands come from user config
2126
cmd.Dir = worktreeDir
22-
cmd.Stdout = os.Stdout
23-
cmd.Stderr = os.Stderr
27+
28+
var buf bytes.Buffer
29+
cmd.Stdout = &buf
30+
cmd.Stderr = &buf
2431

2532
err := cmd.Run()
2633
if err != nil {
27-
err = fmt.Errorf("hook %q: %w", hook, err)
34+
err = fmt.Errorf("hook %q: %w\n%s", hook, err, strings.TrimSpace(buf.String()))
2835
}
2936
results = append(results, HookResult{Command: hook, Error: err})
3037
}

internal/deps/hooks_test.go

Lines changed: 45 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,14 @@ package deps
33
import (
44
"os"
55
"path/filepath"
6+
"strings"
67
"testing"
78
)
89

910
func TestRunPostCreateHooksSuccess(t *testing.T) {
1011
dir := t.TempDir()
1112

12-
results := RunPostCreateHooks(dir, []string{"touch marker.txt"})
13+
results := RunPostCreateHooks(dir, []string{"touch marker.txt"}, nil)
1314

1415
if len(results) != 1 {
1516
t.Fatalf(fmtExpectedOneResult, len(results))
@@ -31,7 +32,7 @@ func TestRunPostCreateHooksPartialFailure(t *testing.T) {
3132
"touch good.txt",
3233
"false", // always fails
3334
"touch also-good.txt",
34-
})
35+
}, nil)
3536

3637
if len(results) != 3 {
3738
t.Fatalf("expected 3 results, got %d", len(results))
@@ -59,7 +60,7 @@ func TestRunPostCreateHooksPartialFailure(t *testing.T) {
5960
func TestRunPostCreateHooksEmpty(t *testing.T) {
6061
dir := t.TempDir()
6162

62-
results := RunPostCreateHooks(dir, nil)
63+
results := RunPostCreateHooks(dir, nil, nil)
6364

6465
if len(results) != 0 {
6566
t.Errorf("expected 0 results, got %d", len(results))
@@ -72,7 +73,7 @@ func TestRunPostCreateHooksShellFeatures(t *testing.T) {
7273
// Test shell features: pipes and quoting
7374
results := RunPostCreateHooks(dir, []string{
7475
"echo 'hello world' > output.txt",
75-
})
76+
}, nil)
7677

7778
if results[0].Error != nil {
7879
t.Fatalf(fmtExpectedNoError, results[0].Error)
@@ -86,3 +87,43 @@ func TestRunPostCreateHooksShellFeatures(t *testing.T) {
8687
t.Errorf("expected 'hello world\\n', got %q", string(data))
8788
}
8889
}
90+
91+
func TestRunPostCreateHooksProgressCallback(t *testing.T) {
92+
dir := t.TempDir()
93+
94+
var calls []progressCall
95+
onProgress := func(current, total int, name string) {
96+
calls = append(calls, progressCall{current, total, name})
97+
}
98+
99+
hooks := []string{"touch a.txt", "touch b.txt"}
100+
RunPostCreateHooks(dir, hooks, onProgress)
101+
102+
if len(calls) != 2 {
103+
t.Fatalf("expected 2 progress calls, got %d", len(calls))
104+
}
105+
if calls[0].current != 1 || calls[0].total != 2 || calls[0].name != "touch a.txt" {
106+
t.Errorf("calls[0] = %+v, want {1 2 touch a.txt}", calls[0])
107+
}
108+
if calls[1].current != 2 || calls[1].total != 2 || calls[1].name != "touch b.txt" {
109+
t.Errorf("calls[1] = %+v, want {2 2 touch b.txt}", calls[1])
110+
}
111+
}
112+
113+
func TestRunPostCreateHooksOutputCapture(t *testing.T) {
114+
dir := t.TempDir()
115+
116+
results := RunPostCreateHooks(dir, []string{
117+
"echo hook-output-captured && exit 1",
118+
}, nil)
119+
120+
if len(results) != 1 {
121+
t.Fatalf(fmtExpectedOneResult, len(results))
122+
}
123+
if results[0].Error == nil {
124+
t.Fatal("expected error from failing hook")
125+
}
126+
if !strings.Contains(results[0].Error.Error(), "hook-output-captured") {
127+
t.Errorf("error should contain captured output, got %q", results[0].Error.Error())
128+
}
129+
}

0 commit comments

Comments
 (0)