Skip to content

Commit d6da949

Browse files
committed
Fix other minor inconsistencies
1 parent f71fb08 commit d6da949

14 files changed

Lines changed: 119 additions & 122 deletions

commands/command_add_description.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ func createAddDescriptionCommand() *cobra.Command {
3939
MultiSelect: true,
4040
}
4141
aiCommand := ai.GetAiCommandInteractive()
42-
slog.Info(fmt.Sprint("commands", aiCommand))
42+
slog.Info(fmt.Sprint("command: ", aiCommand))
4343
targetCommits := getTargetCommits(args, indicatorTypeString, selectPrsOptions)
4444
executeAddDescription(targetCommits)
4545
}
@@ -90,9 +90,12 @@ func parseDescription(aiOutput string) string {
9090
break
9191
}
9292
}
93+
beginLen := 0
9394
if start == -1 {
9495
slog.Warn("Missing markdown start (" + fmt.Sprint(begins) + ") in claude output")
9596
start = 0
97+
} else {
98+
beginLen = len(begins[beginIndex])
9699
}
97100
end := -1
98101
for _, nextEnd := range ends {
@@ -105,7 +108,7 @@ func parseDescription(aiOutput string) string {
105108
slog.Debug("Missing markdown end (" + fmt.Sprint(ends) + ") in claude output")
106109
end = len(aiOutput[start:])
107110
}
108-
return string(aiOutput[start+len(begins[beginIndex]) : start+end])
111+
return string(aiOutput[start+beginLen : start+end])
109112
}
110113

111114
func getNewPrBody(pr gitutil.PrInfo, description string) string {

commands/command_migrate.go

Lines changed: 42 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ func executeMigrate() {
4646
// Step 1: Find all local branches where the current user has made commits
4747
slog.Debug("Step 1: Finding user branches...")
4848
userBranches := findUserBranches()
49-
slog.Debug(fmt.Sprintf("Step 1 complete: Found %d user branches", len(userBranches)))
49+
slog.Debug(fmt.Sprint("Step 1 complete: Found ", len(userBranches), " user branches"))
5050

5151
if len(userBranches) == 0 {
5252
util.Fprintln(appConfig.Io.Out, "No branches found with your commits")
@@ -56,18 +56,18 @@ func executeMigrate() {
5656
// Step 2: Display branches to user for selection
5757
slog.Debug("Step 2: Selecting branches to migrate...")
5858
selectedBranches := selectBranchesToMigrate(userBranches)
59-
slog.Debug(fmt.Sprintf("Step 2 complete: Selected %d branches", len(selectedBranches)))
59+
slog.Debug(fmt.Sprint("Step 2 complete: Selected ", len(selectedBranches), " branches"))
6060

6161
if len(selectedBranches) == 0 {
6262
slog.Debug("No branches selected for migration")
6363
appConfig.Exit(0)
6464
}
6565

66-
slog.Debug(fmt.Sprintf("Selected %d branches for migration: %v", len(selectedBranches), selectedBranches))
66+
slog.Debug(fmt.Sprint("Selected ", len(selectedBranches), " branches for migration: ", selectedBranches))
6767

6868
// Step 3: Find the most recent commit from origin/main
6969
mostRecentMainCommit := findMostRecentMainCommit(selectedBranches)
70-
slog.Debug(fmt.Sprintf("Most recent origin/main commit: %s", mostRecentMainCommit))
70+
slog.Debug(fmt.Sprint("Most recent origin/main commit: ", mostRecentMainCommit))
7171

7272
// Step 4: Process each selected branch
7373
var results []migrationResult
@@ -78,7 +78,7 @@ func executeMigrate() {
7878

7979
// Switch to main branch
8080
mainBranch := gitutil.GetLocalMainBranchOrDie()
81-
slog.Info(fmt.Sprintf("Switching to branch %s", mainBranch))
81+
slog.Info(fmt.Sprint("Switching to branch ", mainBranch))
8282
util.ExecuteOrDie(util.ExecuteOptions{}, "git", "checkout", mainBranch)
8383

8484
// Step 5: Report summary of migrated branches
@@ -87,16 +87,31 @@ func executeMigrate() {
8787

8888
// processBranch handles the migration of a single branch
8989
func processBranch(branch string, baseCommit string) migrationResult {
90-
slog.Info(fmt.Sprintf("Processing branch: %s", branch))
90+
slog.Info(fmt.Sprint("Processing branch: ", branch))
9191

9292
// Step 4f: Check if branch has a PR - skip if so
9393
mergedPR := gitutil.GetMergedPR(branch)
9494
if mergedPR != nil {
95-
slog.Warn(fmt.Sprintf("Branch %s has merged PR #%s: %s - skipping migration", branch, mergedPR.Number, mergedPR.Title))
95+
slog.Warn(fmt.Sprint("Branch ", branch, " has merged PR #", mergedPR.Number, ": ", mergedPR.Title, " - skipping migration"))
9696
return migrationResult{
9797
branchName: branch,
9898
success: false,
99-
reason: fmt.Sprintf("already merged (PR #%s)", mergedPR.Number),
99+
reason: fmt.Sprint("already merged (PR #", mergedPR.Number, ")"),
100+
numCommits: 0,
101+
}
102+
}
103+
104+
// Step 4c: Check if branch has an unmerged PR before modifying git state
105+
pr := gitutil.GetUnmergedPR(branch)
106+
if pr != nil {
107+
// Step 4d: Handle branch with unmerged PR
108+
// Branches with unmerged PRs are skipped because migrating them would create a new branch
109+
// name (from the template) that doesn't match the existing PR's branch.
110+
slog.Warn(fmt.Sprint("Branch ", branch, " has an unmerged PR #", pr.Number, ": ", pr.Title, " - skipping migration"))
111+
return migrationResult{
112+
branchName: branch,
113+
success: false,
114+
reason: fmt.Sprint("Unmerged PR (#", pr.Number, ")"),
100115
numCommits: 0,
101116
}
102117
}
@@ -107,13 +122,13 @@ func processBranch(branch string, baseCommit string) migrationResult {
107122

108123
// Get commits ahead of base
109124
commitsAhead := getCommitsAhead(baseCommit, "HEAD")
110-
slog.Debug(fmt.Sprintf("Found %d commits ahead of main for branch %s", len(commitsAhead), branch))
125+
slog.Debug(fmt.Sprint("Found ", len(commitsAhead), " commits ahead of main for branch ", branch))
111126
for _, commit := range commitsAhead {
112-
slog.Debug(fmt.Sprintf(" - %s", commit))
127+
slog.Debug(fmt.Sprint(" - ", commit))
113128
}
114129

115130
if len(commitsAhead) == 0 {
116-
slog.Info(fmt.Sprintf("Branch %s has no commits ahead of main - skipping", branch))
131+
slog.Info(fmt.Sprint("Branch ", branch, " has no commits ahead of main - skipping"))
117132
return migrationResult{
118133
branchName: branch,
119134
success: false,
@@ -122,23 +137,8 @@ func processBranch(branch string, baseCommit string) migrationResult {
122137
}
123138
}
124139

125-
// Step 4c: Check if branch has an unmerged PR
126-
pr := gitutil.GetUnmergedPR(branch)
127-
if pr != nil {
128-
// Step 4d: Handle branch with unmerged PR
129-
// Branches with unmerged PRs are skipped because migrating them would create a new branch
130-
// name (from the template) that doesn't match the existing PR's branch.
131-
slog.Warn(fmt.Sprintf("Branch %s has an unmerged PR #%s: %s - skipping migration", branch, pr.Number, pr.Title))
132-
return migrationResult{
133-
branchName: branch,
134-
success: false,
135-
reason: fmt.Sprintf("Unmerged PR (#%s)", pr.Number),
136-
numCommits: 0,
137-
}
138-
} else {
139-
// Step 4e: Handle branch without PR
140-
return handleBranchWithoutPR(branch, baseCommit, commitsAhead)
141-
}
140+
// Step 4e: Handle branch without PR
141+
return handleBranchWithoutPR(branch, baseCommit, commitsAhead)
142142
}
143143

144144
// rebaseBranch rebases the current branch onto baseCommit, handling errors appropriately
@@ -148,7 +148,7 @@ func rebaseBranch(branch string, baseCommit string) {
148148
gitutil.RebaseAndSkipAllEmptyOrDie(
149149
util.ExecuteOptions{Io: util.StdIo{Out: appConfig.Io.Out, In: nil, Err: appConfig.Io.Err}},
150150
baseCommit)
151-
slog.Info(fmt.Sprintf("Successfully rebased branch: %s", branch))
151+
slog.Info(fmt.Sprint("Successfully rebased branch: ", branch))
152152
}
153153

154154
// itemWithTimestamp associates a name with a timestamp for sorting purposes
@@ -202,7 +202,7 @@ func findUserBranches() []string {
202202
result[i] = b.name
203203
}
204204

205-
slog.Info(fmt.Sprintf("Found %d branches with your commits", len(result)))
205+
slog.Info(fmt.Sprint("Found ", len(result), " branches with your commits"))
206206
return result
207207
}
208208

@@ -229,7 +229,7 @@ func filterBranchesWithUserCommits(allBranches []string, mainBranch string, user
229229
continue
230230
}
231231

232-
slog.Debug(fmt.Sprintf("Found branch %s with user commits (timestamp: %s)", branch, strings.TrimSpace(logOutput)))
232+
slog.Debug(fmt.Sprint("Found branch ", branch, " with user commits (timestamp: ", strings.TrimSpace(logOutput), ")"))
233233
branchesWithTimestamps = append(branchesWithTimestamps, itemWithTimestamp{
234234
name: branch,
235235
timestamp: timestamp,
@@ -285,7 +285,7 @@ func computeDisabledBranches(branches []string) map[int]bool {
285285
slog.Info("Fetching commits from main for branch filtering...")
286286
mainBranch := gitutil.GetLocalMainBranchOrDie()
287287
mainCommits := templates.GetNewCommits(mainBranch, "")
288-
slog.Info(fmt.Sprintf("Found %d commits on main for branch filtering", len(mainCommits)))
288+
slog.Info(fmt.Sprint("Found ", len(mainCommits), " commits on main for branch filtering"))
289289

290290
branchesOnMain := make(map[string]bool)
291291
for _, commit := range mainCommits {
@@ -299,7 +299,7 @@ func computeDisabledBranches(branches []string) map[int]bool {
299299
for i, branch := range branches {
300300
if branchesOnMain[branch] {
301301
disabledBranches[i] = true
302-
slog.Info(fmt.Sprintf("Branch %s already exists on main - will be disabled", branch))
302+
slog.Info(fmt.Sprint("Branch ", branch, " already exists on main - will be disabled"))
303303
}
304304
}
305305
return disabledBranches
@@ -318,7 +318,7 @@ func findMostRecentMainCommit(branches []string) string {
318318
// For each branch, find its merge-base with origin/main
319319
for _, branch := range allBranches {
320320
mergeBase := gitutil.FirstOriginMainCommit(branch)
321-
slog.Debug(fmt.Sprintf("Merge-base for %s with origin/main: %s", branch, mergeBase))
321+
slog.Debug(fmt.Sprint("Merge-base for ", branch, " with origin/main: ", mergeBase))
322322

323323
// Get the timestamp of this commit
324324
timestampStr := util.ExecuteOrDie(util.ExecuteOptions{}, "git", "log", "-1", "--format=%ct", mergeBase)
@@ -372,19 +372,19 @@ func promptForInput(prompt string) string {
372372
return strings.TrimSpace(scanner.Text())
373373
}
374374
if err := scanner.Err(); err != nil {
375-
panic(fmt.Sprintf("Failed to read input: %s", err.Error()))
375+
panic(fmt.Sprint("Failed to read input: ", err.Error()))
376376
}
377377
return ""
378378
}
379379

380380
// handleBranchWithoutPR handles step 4e: migration of a branch that doesn't have a PR.
381381
func handleBranchWithoutPR(branch string, baseCommit string, commitsAhead []string) migrationResult {
382-
slog.Info(fmt.Sprintf("Branch %s does not have an unmerged PR", branch))
382+
slog.Info(fmt.Sprint("Branch ", branch, " does not have an unmerged PR"))
383383

384384
// Prompt user for the eventual PR name
385385
prName := promptForInput("What should the PR be named when it is eventually created")
386386
if prName == "" {
387-
slog.Info(fmt.Sprintf("No PR name provided, skipping branch %s", branch))
387+
slog.Info(fmt.Sprint("No PR name provided, skipping branch ", branch))
388388
return migrationResult{
389389
branchName: branch,
390390
success: false,
@@ -402,7 +402,7 @@ func handleBranchWithoutPR(branch string, baseCommit string, commitsAhead []stri
402402
// Rename the first commit to match the eventual PR title
403403
firstCommit := commitsAhead[0]
404404
renameCommitOnBranch(branch, firstCommit, prName)
405-
slog.Info(fmt.Sprintf("Renamed first commit to: %s", prName))
405+
slog.Info(fmt.Sprint("Renamed first commit to: ", prName))
406406

407407
// Rename remaining commits with prefix if provided
408408
if commitPrefix != "" && len(commitsAhead) > 1 {
@@ -411,16 +411,16 @@ func handleBranchWithoutPR(branch string, baseCommit string, commitsAhead []stri
411411

412412
// Get the final list of commits in chronological order (oldest first)
413413
finalCommits := getCommitsAhead(baseCommit, branch)
414-
slog.Info(fmt.Sprintf("Final commits to cherry-pick (in order): %d commits", len(finalCommits)))
414+
slog.Info(fmt.Sprint("Final commits to cherry-pick (in order): ", len(finalCommits), " commits"))
415415
for i, commit := range finalCommits {
416-
slog.Debug(fmt.Sprintf(" %d. %s: %s", i+1, commit, getCommitSubject(commit)))
416+
slog.Debug(fmt.Sprint(" ", i+1, ". ", commit, ": ", getCommitSubject(commit)))
417417
}
418418

419419
// Cherry-pick each commit onto local main branch IN ORDER (oldest to newest)
420420
mainBranch := gitutil.GetLocalMainBranchOrDie()
421421
util.ExecuteOrDie(util.ExecuteOptions{}, "git", "checkout", mainBranch)
422422

423-
slog.Info(fmt.Sprintf("Cherry-picking %d commits to %s (oldest to newest)", len(finalCommits), mainBranch))
423+
slog.Info(fmt.Sprint("Cherry-picking ", len(finalCommits), " commits to ", mainBranch, " (oldest to newest)"))
424424
gitutil.CherryPickAndSkipAllEmpty("", finalCommits)
425425

426426
return migrationResult{
@@ -479,7 +479,7 @@ func prefixRemainingCommits(branch string, baseCommit string, commitPrefix strin
479479
oldSubject := getCommitSubject(commit)
480480
newSubject := commitPrefix + " " + oldSubject
481481
renameCommitOnBranch(branch, commit, newSubject)
482-
slog.Info(fmt.Sprintf("Renamed commit to: %s", newSubject))
482+
slog.Info(fmt.Sprint("Renamed commit to: ", newSubject))
483483

484484
// Get updated commit list after each rename since hashes change
485485
currentCommits = getCommitsAhead(baseCommit, branch)

commands/command_migrate_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -426,7 +426,7 @@ func TestGetMergedPR_WithMergedPR(t *testing.T) {
426426
testutil.AddCommit("feature 1 commit", "feature1.txt")
427427

428428
// Mock the gh pr list response to simulate a merged PR
429-
testExecutor.SetResponse("456|Implement feature|MERGED", nil,
429+
testExecutor.SetResponse("456"+gitutil.GhDelim+"Implement feature"+gitutil.GhDelim+"MERGED", nil,
430430
"gh", "pr", "list", "--head", "feature-1", "--state", "merged",
431431
"--json", "number,title,state", "--jq", util.MatchAnyRemainingArgs)
432432

@@ -478,7 +478,7 @@ func TestSdMigrate_SkipsBranchWithMergedPR(t *testing.T) {
478478
util.ExecuteOrDie(util.ExecuteOptions{}, "git", "checkout", gitutil.GetLocalMainBranchOrDie())
479479

480480
// Mock the gh pr list response to simulate a merged PR
481-
testExecutor.SetResponse("789|Already merged feature|MERGED", nil,
481+
testExecutor.SetResponse("789"+gitutil.GhDelim+"Already merged feature"+gitutil.GhDelim+"MERGED", nil,
482482
"gh", "pr", "list", "--head", "feature-1", "--state", "merged",
483483
"--json", "number,title,state", "--jq", util.MatchAnyRemainingArgs)
484484

commands/command_rebase_main.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -65,9 +65,9 @@ func rebaseMain() {
6565

6666
var confirmedClosedCommits []templates.GitLog
6767
if len(closedCommits) > 0 {
68-
slog.Info(fmt.Sprintf("Found %d commits from closed (not merged) PRs:", len(closedCommits)))
68+
slog.Info(fmt.Sprint("Found ", len(closedCommits), " commits from closed (not merged) PRs:"))
6969
for _, closedCommit := range closedCommits {
70-
slog.Info(fmt.Sprintf(" - %s: %s", closedCommit.Branch, closedCommit.Subject))
70+
slog.Info(fmt.Sprint(" - ", closedCommit.Branch, ": ", closedCommit.Subject))
7171
}
7272
if interactive.Confirm("Do you want to drop these closed PR commits and delete their local branches?", false) {
7373
confirmedClosedCommits = closedCommits
@@ -165,7 +165,7 @@ func getBranchesByPRState(mergedState bool) []string {
165165
for _, branchRawLine := range branchesRawLines {
166166
fields := strings.Fields(branchRawLine)
167167
if len(fields) != 2 {
168-
break
168+
continue
169169
}
170170
// Validate refs from GitHub API before passing to git commands.
171171
util.RequireGitRef(fields[0])

gitutil/gh_checks.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,7 @@ func getMinChecks() int {
112112
return numChecks
113113
})
114114
if len(allNumChecks) == 0 {
115+
cache.minChecks = DefaultMinChecks
115116
return
116117
}
117118

gitutil/gh_pr_info.go

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -22,37 +22,39 @@ type PrInfo struct {
2222
// Returns the PR information if found, nil otherwise.
2323
func GetMergedPR(branchName string) *PrInfo {
2424
// Check for merged PRs with this branch as the head
25-
slog.Debug(fmt.Sprintf("Checking for merged PR with head branch: %s", branchName))
25+
slog.Debug(fmt.Sprint("Checking for merged PR with head branch: ", branchName))
2626
output := util.ExecuteOrDieTrimmed(
2727
util.ExecuteOptions{Retries: GhRetries},
2828
"gh", "pr", "list",
2929
"--head", branchName,
3030
"--state", "merged",
3131
"--json", "number,title,state",
32-
"--jq", ".[0] | \"\\(.number)|\\(.title)|\\(.state)\"",
32+
"--jq", ".[0] | \"\\(.number)"+GhDelim+"\\(.title)"+GhDelim+"\\(.state)\"",
3333
)
3434

35-
slog.Debug(fmt.Sprintf("gh pr list output for branch %s: '%s'", branchName, output))
35+
slog.Debug(fmt.Sprint("gh pr list output for branch ", branchName, ": '", output, "'"))
3636

37-
if output == "" || output == "null" || output == "||" || output == "null|null|null" {
38-
slog.Debug(fmt.Sprintf("No merged PR found for branch %s", branchName))
37+
nullOutput := GhDelim + GhDelim
38+
nullNullOutput := "null" + GhDelim + "null" + GhDelim + "null"
39+
if output == "" || output == "null" || output == nullOutput || output == nullNullOutput {
40+
slog.Debug(fmt.Sprint("No merged PR found for branch ", branchName))
3941
return nil
4042
}
4143

42-
// Parse the output: "number|title|state"
43-
parts := strings.SplitN(output, "|", 3)
44+
// Parse the output: "number<delim>title<delim>state"
45+
parts := strings.SplitN(output, GhDelim, 4)
4446
if len(parts) != 3 {
45-
slog.Warn(fmt.Sprintf("Unexpected PR output format for branch %s: %s", branchName, output))
47+
slog.Warn(fmt.Sprint("Unexpected PR output format for branch ", branchName, ": ", output))
4648
return nil
4749
}
4850

4951
// Check if any part is empty or "null" (which means the PR data was incomplete/missing)
5052
if parts[0] == "" || parts[0] == "null" || parts[1] == "" || parts[1] == "null" || parts[2] == "" || parts[2] == "null" {
51-
slog.Debug(fmt.Sprintf("No merged PR found for branch %s (empty or null PR data)", branchName))
53+
slog.Debug(fmt.Sprint("No merged PR found for branch ", branchName, " (empty or null PR data)"))
5254
return nil
5355
}
5456

55-
slog.Debug(fmt.Sprintf("Found merged PR for branch %s: #%s - %s (%s)", branchName, parts[0], parts[1], parts[2]))
57+
slog.Debug(fmt.Sprint("Found merged PR for branch ", branchName, ": #", parts[0], " - ", parts[1], " (", parts[2], ")"))
5658

5759
return &PrInfo{
5860
Number: parts[0],

0 commit comments

Comments
 (0)