Merge pull request #81 from entireio/fix/positional-source-url-args · Entire
Merge pull request #81 from entireio/fix/positional-source-url-args
691407d→main·
Soph·1mo ago·6 files·+85 added/-28 removed
Consume positional source/target left-to-right across commands
Changes
6
cmd/git-sync
Aargs_test.go+50
Mbootstrap.go+2/-5
Mconvert_sha256.go+4/-13
Mflags.go+25
Mprobe.go+2/-5
Msyncplan.go+2/-5
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
package main
import "testing"
// Positional endpoints must be consumed left-to-right, skipping whatever was
// already given via a flag — so `--source-url URL <target>` fills the target
// slot rather than being dropped (the reported bug across sync/bootstrap/probe).
func TestResolvePositionalEndpoints(t *testing.T) {
const src = "https://example.invalid/source.git"
const tgt = "https://example.invalid/target.git"
cases := []struct {
name string
source, target string
args []string
wantSource, wantTarget string
wantErr bool
}{
{name: "both positional", args: []string{src, tgt}, wantSource: src, wantTarget: tgt},
{name: "source flag plus positional target", source: src, args: []string{tgt}, wantSource: src, wantTarget: tgt},
{name: "target flag plus positional source", target: tgt, args: []string{src}, wantSource: src, wantTarget: tgt},
{name: "both flags, no positionals", source: src, target: tgt, wantSource: src, wantTarget: tgt},
{name: "source flag only, no positionals", source: src, wantSource: src},
// Over-specified: source set by flag, then two positionals — the old
// fixed-index code silently ignored one; we reject instead.
{name: "source flag plus two positionals", source: src, args: []string{"a", "b"}, wantErr: true},
{name: "both flags plus a positional", source: src, target: tgt, args: []string{"extra"}, wantErr: true},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
source, target := tc.source, tc.target
err := resolvePositionalEndpoints(&source, &target, tc.args)
if tc.wantErr {
if err == nil {
t.Fatalf("expected error for over-specified args, got nil")
}
return
}
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if source != tc.wantSource {
t.Errorf("source = %q, want %q", source, tc.wantSource)
}
if target != tc.wantTarget {
t.Errorf("target = %q, want %q", target, tc.wantTarget)
}
})
}
}
Acmd/git-sync/args_test.go+50
29 unmodified lines
30
31
32
33
34
35
36
37
33
34
35
36
37
29 unmodified lines
RunE: func(cmd *cobra.Command, args []string) error {
req.Protocol = gitsync.ProtocolMode(protocolVal)
if req.Source.URL == "" && len(args) > 0 {
req.Source.URL = args[0]
}
if req.Target.URL == "" && len(args) > 1 {
req.Target.URL = args[1]
if err := resolvePositionalEndpoints(&req.Source.URL, &req.Target.URL, args); err != nil {
return err
}
if branches != "" {
Mcmd/git-sync/bootstrap.go+2/-5
109 unmodified lines
110
111
112
113
114
115
116
117
118
113
114
115
120
121
122
123
124
125
126
116
117
118
119
120
109 unmodified lines
return cmd
}
// resolveConvertSHA256Args consumes positional args left-to-right,
// skipping fields the user already supplied via flags. Without that
// rule, `--source-url <url> <dir>` would look like one positional and
// land in SourceURL — leaving TargetDir empty even though the user
// gave both. The two-flags-no-positionals and zero-flags-two-positionals
// shapes also work, as do the symmetric --target-dir + positional URL.
// resolveConvertSHA256Args fills the source URL and target dir from flags
// and/or positional args (see resolvePositionalEndpoints) and requires both.
func resolveConvertSHA256Args(req *sha256convert.Request, args []string) error {
positional := args
if req.SourceURL == "" && len(positional) > 0 {
req.SourceURL = positional[0]
positional = positional[1:]
}
if req.TargetDir == "" && len(positional) > 0 {
req.TargetDir = positional[0]
if err := resolvePositionalEndpoints(&req.SourceURL, &req.TargetDir, args); err != nil {
return err
}
if req.SourceURL == "" || req.TargetDir == "" {
return errors.New("convert-sha256 requires a source URL and a target directory")
Mcmd/git-sync/convert_sha256.go+4/-13
141 unmodified lines
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
141 unmodified lines
return value == "1" || value == "true" || value == "yes" || value == "on"
// resolvePositionalEndpoints fills the source and target from positional args
// left-to-right, skipping whichever was already supplied via a flag. Consuming
// by fixed index instead (args[0]→source, args[1]→target) breaks the mixed
// form `--source-url URL <target>`: the lone positional lands in args[0] and
// the target slot stays empty.
//
// A positional left over after both slots are filled means the user
// over-specified an endpoint (e.g. `--source-url URL a b`); reject it rather
// than silently pick one. Callers still validate that both ended up set.
func resolvePositionalEndpoints(source, target *string, args []string) error {
positional := args
if *source == "" && len(positional) > 0 {
*source = positional[0]
positional = positional[1:]
}
if *target == "" && len(positional) > 0 {
*target = positional[0]
positional = positional[1:]
}
if len(positional) > 0 {
return fmt.Errorf("unexpected argument %q: source and target are already set via flags or earlier positionals", positional[0])
}
return nil
}
func splitCSV(value string) []string {
parts := strings.Split(value, ",")
out := make([]string, 0, len(parts))
Mcmd/git-sync/flags.go+25
28 unmodified lines
29
30
31
32
33
34
35
36
32
33
34
35
36
28 unmodified lines
RunE: func(cmd *cobra.Command, args []string) error {
req.Protocol = gitsync.ProtocolMode(protocolVal)
if req.Source.URL == "" && len(args) > 0 {
req.Source.URL = args[0]
}
if targetURL == "" && len(args) > 1 {
targetURL = args[1]
if err := resolvePositionalEndpoints(&req.Source.URL, &targetURL, args); err != nil {
return err
}
if req.Source.URL == "" {
return errors.New("probe requires a source repository URL")
Mcmd/git-sync/probe.go+2/-5
47 unmodified lines
48 49 50 51 52 53 54 55 51 52 53 54 55
47 unmodified lines
req.Policy.Mode = gitsync.OperationMode(modeValue) req.Policy.Protocol = gitsync.ProtocolMode(protocolVal)
if branches != "" {