Skip to content

Commit e7fdf6d

Browse files
skarimskarim
authored
Merge pull request #5 from github/skarim/fix-initial-branch-prefix
fix: initial branch missing prefix
2 parents 90eb999 + 0b9293a commit e7fdf6d

2 files changed

Lines changed: 146 additions & 44 deletions

File tree

‎cmd/init.go‎

Lines changed: 57 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -101,13 +101,62 @@ func runInit(cfg *config.Config, opts *initOptions) error {
101101

102102
var branches []string
103103

104+
// --adopt takes existing branches as-is; --prefix and --numbered don't apply.
105+
if opts.adopt && (opts.prefix != "" || opts.numbered) {
106+
cfg.Errorf("--adopt cannot be combined with --prefix or --numbered")
107+
return ErrInvalidArgs
108+
}
109+
104110
// Validate --numbered requires a prefix (either from flag or interactive input,
105111
// but for non-interactive paths we can check early).
106112
if opts.numbered && opts.prefix == "" && !cfg.IsInteractive() {
107113
cfg.Errorf("--numbered requires --prefix")
108114
return ErrInvalidArgs
109115
}
110116

117+
// Prompt for prefix interactively if not provided via flag and we're
118+
// in interactive mode (not adopt, not explicit branches).
119+
if opts.prefix == "" && !opts.adopt && len(opts.branches) == 0 && cfg.IsInteractive() {
120+
p := prompter.New(cfg.In, cfg.Out, cfg.Err)
121+
if opts.numbered {
122+
// --numbered requires a prefix; prompt specifically for one
123+
prefixInput, err := p.Input("Enter a branch prefix (required for --numbered)", "")
124+
if err != nil {
125+
if isInterruptError(err) {
126+
printInterrupt(cfg)
127+
return ErrSilent
128+
}
129+
cfg.Errorf("failed to read prefix: %s", err)
130+
return ErrSilent
131+
}
132+
opts.prefix = strings.TrimSpace(prefixInput)
133+
if opts.prefix == "" {
134+
cfg.Errorf("--numbered requires a prefix")
135+
return ErrInvalidArgs
136+
}
137+
} else {
138+
prefixInput, err := p.Input("Set a branch prefix? (leave blank to skip)", "")
139+
if err != nil {
140+
if isInterruptError(err) {
141+
printInterrupt(cfg)
142+
return ErrSilent
143+
}
144+
cfg.Errorf("failed to read prefix: %s", err)
145+
return ErrSilent
146+
}
147+
opts.prefix = strings.TrimSpace(prefixInput)
148+
}
149+
}
150+
151+
// Validate prefix, after it has been determined (from flag or prompt),
152+
// before any branch creation.
153+
if opts.prefix != "" {
154+
if err := git.ValidateRefName(opts.prefix); err != nil {
155+
cfg.Errorf("invalid prefix %q: must be a valid git ref component", opts.prefix)
156+
return ErrInvalidArgs
157+
}
158+
}
159+
111160
if opts.adopt {
112161
// Adopt mode: validate all specified branches exist
113162
if len(opts.branches) == 0 {
@@ -145,8 +194,12 @@ func runInit(cfg *config.Config, opts *initOptions) error {
145194
}
146195
}
147196
} else if len(opts.branches) > 0 {
148-
// Explicit branch names provided — create them
197+
// Explicit branch names provided — apply prefix and create them
198+
prefixed := make([]string, 0, len(opts.branches))
149199
for _, b := range opts.branches {
200+
if opts.prefix != "" {
201+
b = opts.prefix + "/" + b
202+
}
150203
if err := sf.ValidateNoDuplicateBranch(b); err != nil {
151204
cfg.Errorf("branch %q already exists in a stack", b)
152205
return ErrInvalidArgs
@@ -157,49 +210,17 @@ func runInit(cfg *config.Config, opts *initOptions) error {
157210
return ErrSilent
158211
}
159212
}
213+
prefixed = append(prefixed, b)
160214
}
161-
branches = opts.branches
215+
branches = prefixed
162216
} else {
163-
// Interactive mode
217+
// Interactive mode — prefix was already prompted for above
164218
if !cfg.IsInteractive() {
165219
cfg.Errorf("interactive input required; provide branch names or use --adopt")
166220
return ErrInvalidArgs
167221
}
168222
p := prompter.New(cfg.In, cfg.Out, cfg.Err)
169223

170-
// Step 1: Ask for prefix
171-
if opts.prefix == "" {
172-
if opts.numbered {
173-
// --numbered requires a prefix; prompt specifically for one
174-
prefixInput, err := p.Input("Enter a branch prefix (required for --numbered)", "")
175-
if err != nil {
176-
if isInterruptError(err) {
177-
printInterrupt(cfg)
178-
return ErrSilent
179-
}
180-
cfg.Errorf("failed to read prefix: %s", err)
181-
return ErrSilent
182-
}
183-
opts.prefix = strings.TrimSpace(prefixInput)
184-
if opts.prefix == "" {
185-
cfg.Errorf("--numbered requires a prefix")
186-
return ErrInvalidArgs
187-
}
188-
} else {
189-
prefixInput, err := p.Input("Set a branch prefix? (leave blank to skip)", "")
190-
if err != nil {
191-
if isInterruptError(err) {
192-
printInterrupt(cfg)
193-
return ErrSilent
194-
}
195-
cfg.Errorf("failed to read prefix: %s", err)
196-
return ErrSilent
197-
}
198-
opts.prefix = strings.TrimSpace(prefixInput)
199-
}
200-
}
201-
202-
// Step 2: Ask for branch name (unless --numbered auto-generates it)
203224
if opts.numbered {
204225
// Auto-generate numbered branch name
205226
branchName := branch.NextNumberedName(opts.prefix, nil)
@@ -278,14 +299,6 @@ func runInit(cfg *config.Config, opts *initOptions) error {
278299
}
279300
}
280301

281-
// Validate prefix (from flag or interactive input)
282-
if opts.prefix != "" {
283-
if err := git.ValidateRefName(opts.prefix); err != nil {
284-
cfg.Errorf("invalid prefix %q: must be a valid git ref component", opts.prefix)
285-
return ErrInvalidArgs
286-
}
287-
}
288-
289302
// Build stack
290303
trunkSHA, _ := git.RevParse(trunk)
291304
branchRefs := make([]stack.BranchRef, len(branches))

‎cmd/init_test.go‎

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package cmd
22

33
import (
4+
"fmt"
45
"io"
56
"os"
67
"testing"
@@ -110,6 +111,94 @@ func TestInit_PrefixStoredInStack(t *testing.T) {
110111
assert.Equal(t, "feat", sf.Stacks[0].Prefix)
111112
}
112113

114+
func TestInit_PrefixAppliedToExplicitBranches(t *testing.T) {
115+
gitDir := t.TempDir()
116+
var created []string
117+
restore := git.SetOps(&git.MockOps{
118+
GitDirFn: func() (string, error) { return gitDir, nil },
119+
DefaultBranchFn: func() (string, error) { return "main", nil },
120+
CurrentBranchFn: func() (string, error) { return "main", nil },
121+
CreateBranchFn: func(name, base string) error {
122+
created = append(created, name)
123+
return nil
124+
},
125+
})
126+
defer restore()
127+
128+
cfg, outR, errR := config.NewTestConfig()
129+
err := runInit(cfg, &initOptions{branches: []string{"b1", "b2"}, prefix: "feat"})
130+
output := collectOutput(cfg, outR, errR)
131+
132+
require.NoError(t, err, "runInit should succeed")
133+
require.NotContains(t, output, "\u2717", "unexpected error")
134+
assert.Equal(t, []string{"feat/b1", "feat/b2"}, created, "branches should be created with prefix")
135+
136+
sf, err := stack.Load(gitDir)
137+
require.NoError(t, err, "loading stack")
138+
names := sf.Stacks[0].BranchNames()
139+
assert.Equal(t, []string{"feat/b1", "feat/b2"}, names, "stack should store prefixed branch names")
140+
}
141+
142+
func TestInit_InvalidPrefixRejectedBeforeBranchCreation(t *testing.T) {
143+
gitDir := t.TempDir()
144+
var created []string
145+
restore := git.SetOps(&git.MockOps{
146+
GitDirFn: func() (string, error) { return gitDir, nil },
147+
DefaultBranchFn: func() (string, error) { return "main", nil },
148+
CurrentBranchFn: func() (string, error) { return "main", nil },
149+
ValidateRefNameFn: func(name string) error {
150+
return fmt.Errorf("invalid ref name: %s", name)
151+
},
152+
CreateBranchFn: func(name, base string) error {
153+
created = append(created, name)
154+
return nil
155+
},
156+
})
157+
defer restore()
158+
159+
cfg, outR, errR := config.NewTestConfig()
160+
err := runInit(cfg, &initOptions{branches: []string{"mybranch"}, prefix: "bad..prefix"})
161+
output := collectOutput(cfg, outR, errR)
162+
163+
assert.ErrorIs(t, err, ErrInvalidArgs, "should reject invalid prefix")
164+
assert.Contains(t, output, "invalid prefix")
165+
assert.Empty(t, created, "no branches should be created when prefix is invalid")
166+
}
167+
168+
func TestInit_AdoptRejectsPrefix(t *testing.T) {
169+
gitDir := t.TempDir()
170+
restore := git.SetOps(&git.MockOps{
171+
GitDirFn: func() (string, error) { return gitDir, nil },
172+
DefaultBranchFn: func() (string, error) { return "main", nil },
173+
CurrentBranchFn: func() (string, error) { return "main", nil },
174+
})
175+
defer restore()
176+
177+
cfg, outR, errR := config.NewTestConfig()
178+
err := runInit(cfg, &initOptions{adopt: true, branches: []string{"b1"}, prefix: "feat"})
179+
output := collectOutput(cfg, outR, errR)
180+
181+
assert.ErrorIs(t, err, ErrInvalidArgs)
182+
assert.Contains(t, output, "--adopt cannot be combined with --prefix or --numbered")
183+
}
184+
185+
func TestInit_AdoptRejectsNumbered(t *testing.T) {
186+
gitDir := t.TempDir()
187+
restore := git.SetOps(&git.MockOps{
188+
GitDirFn: func() (string, error) { return gitDir, nil },
189+
DefaultBranchFn: func() (string, error) { return "main", nil },
190+
CurrentBranchFn: func() (string, error) { return "main", nil },
191+
})
192+
defer restore()
193+
194+
cfg, outR, errR := config.NewTestConfig()
195+
err := runInit(cfg, &initOptions{adopt: true, branches: []string{"b1"}, numbered: true})
196+
output := collectOutput(cfg, outR, errR)
197+
198+
assert.ErrorIs(t, err, ErrInvalidArgs)
199+
assert.Contains(t, output, "--adopt cannot be combined with --prefix or --numbered")
200+
}
201+
113202
func TestInit_RerereAlreadyEnabled(t *testing.T) {
114203
gitDir := t.TempDir()
115204
enableRerereCalled := false

0 commit comments

Comments
 (0)