Address PR review on trail checkout

feea8fe→main·

matthiaswenz·4w ago·2 files·+14 added/-6 removed

• Label the empty-branch error via describeTrailRef so a numbered trail with no title no longer renders as trail "" has no branch.
• Print an explicit cancellation line when the user declines fetching a remote-only branch, so the preceding "Checking out …" doesn't read as a successful switch.
• Copy the table-test input into a local before taking its address in the parallel subtest.

Co-Authored-By: Claude Opus 4.8 noreply@anthropic.com

Sessions

5aba9c537ceaView transcript

Changes

2

• cmd/entire/cli

• Mtrail_checkout_cmd_test.go+5/-2

• Mtrail_cmd.go+9/-4

92 unmodified lines

93
94
95
96
96
97
98
99
100
98
101
102
103
104

92 unmodified lines

for _, tc := range cases {
        t.Run(tc.name, func(t *testing.T) {
            t.Parallel()
            got := describeTrailRef(&tc.in)
            // Copy the input into a local so the parallel subtest never takes the
            // address of the shared range variable.
            in := tc.in
            got := describeTrailRef(&in)
            if got != tc.want {
                t.Fatalf("describeTrailRef(%#v) = %q, want %q", tc.in, got, tc.want)
                t.Fatalf("describeTrailRef(%#v) = %q, want %q", in, got, tc.want)
            }
        })
    }

Mcmd/entire/cli/trail_checkout_cmd_test.go+5/-2

1009 unmodified lines

1010
1011
1012
1013
1013
1014
1015
1016
5 unmodified lines

1022
1023
1024
1025
1026
1025
1026
1028
1027
1028
1029
1030
1031
1032
1033
1034
1035
1036
1037
1038

1009 unmodified lines

branch := strings.TrimSpace(found.Branch)
        if branch == "" {
            return fmt.Errorf("trail %q has no branch to check out", found.Title)
            return fmt.Errorf("%s has no branch to check out", describeTrailRef(found))
        }

currentBranch, _ := GetCurrentBranch(ctx) //nolint:errcheck // best-effort; a detached HEAD just means "not already on the branch"
5 unmodified lines

fmt.Fprintf(w, "Checking out %s\n", describeTrailRef(found))
        // switchToBranchForResume handles local vs. remote-only branches, the
        // uncommitted-changes guard, and the fetch prompt; reuse it rather than
        // re-deriving that logic here. proceed=false (user declined the
        // fetch) is a clean stop, not an error.
        // re-deriving that logic here.
        proceed, err := switchToBranchForResume(ctx, w, errW, branch, force)
        if err != nil || !proceed {
            if err != nil {
                return err
            }
            if !proceed {
                // The user declined to fetch a remote-only branch — a clean stop, not
                // an error. Say so explicitly so the preceding "Checking out …" line
                // doesn't read as a successful switch.
                fmt.Fprintf(w, "Checkout of branch %s cancelled.\n", branch)
            }
            return nil
        })
}

Mcmd/entire/cli/trail_cmd.go+9/-4