Skip to content

Commit 4425984

Browse files
committed
argparse: allow positional args to be optional
Add 'optional' property to 'argparse.positionalArg'. This leads to the following behavior if an arg isn't specified on the command line: - If 'required' is true, exit with the appropriate usage message. - If 'required' is false, stop processing positional arguments. The arg values are never set, leaving them with the default values with which they were originally specified. For now, mark all existing positional arguments as required to avoid changes to behavior. Signed-off-by: Victoria Dye <[email protected]>
1 parent d6aab70 commit 4425984

File tree

6 files changed

+30
-16
lines changed

6 files changed

+30
-16
lines changed

cmd/git-bundle-server/delete.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ data.`
3434

3535
func (d *deleteCmd) Run(ctx context.Context, args []string) error {
3636
parser := argparse.NewArgParser(d.logger, "git-bundle-server delete <route>")
37-
route := parser.PositionalString("route", "the route to delete")
37+
route := parser.PositionalString("route", "the route to delete", true)
3838
parser.Parse(ctx, args)
3939

4040
repoProvider := utils.GetDependency[core.RepositoryProvider](ctx, d.container)

cmd/git-bundle-server/init.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,9 @@ should be hosted at '<route>'.`
3636

3737
func (i *initCmd) Run(ctx context.Context, args []string) error {
3838
parser := argparse.NewArgParser(i.logger, "git-bundle-server init <url> <route>")
39-
url := parser.PositionalString("url", "the URL of a repository to clone")
39+
url := parser.PositionalString("url", "the URL of a repository to clone", true)
4040
// TODO: allow parsing <route> out of <url>
41-
route := parser.PositionalString("route", "the route to host the specified repo")
41+
route := parser.PositionalString("route", "the route to host the specified repo", true)
4242
parser.Parse(ctx, args)
4343

4444
repoProvider := utils.GetDependency[core.RepositoryProvider](ctx, i.container)

cmd/git-bundle-server/start.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ specified '<route>'.`
3434

3535
func (s *startCmd) Run(ctx context.Context, args []string) error {
3636
parser := argparse.NewArgParser(s.logger, "git-bundle-server start <route>")
37-
route := parser.PositionalString("route", "the route for which bundles should be generated")
37+
route := parser.PositionalString("route", "the route for which bundles should be generated", true)
3838
parser.Parse(ctx, args)
3939

4040
repoProvider := utils.GetDependency[core.RepositoryProvider](ctx, s.container)

cmd/git-bundle-server/stop.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ specified '<route>'.`
3333

3434
func (s *stopCmd) Run(ctx context.Context, args []string) error {
3535
parser := argparse.NewArgParser(s.logger, "git-bundle-server stop <route>")
36-
route := parser.PositionalString("route", "the route for which bundles should stop being generated")
36+
route := parser.PositionalString("route", "the route for which bundles should stop being generated", true)
3737
parser.Parse(ctx, args)
3838

3939
repoProvider := utils.GetDependency[core.RepositoryProvider](ctx, s.container)

cmd/git-bundle-server/update.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ bundles, and update the bundle list.`
3636

3737
func (u *updateCmd) Run(ctx context.Context, args []string) error {
3838
parser := argparse.NewArgParser(u.logger, "git-bundle-server update <route>")
39-
route := parser.PositionalString("route", "the route to update")
39+
route := parser.PositionalString("route", "the route to update", true)
4040
parser.Parse(ctx, args)
4141

4242
repoProvider := utils.GetDependency[core.RepositoryProvider](ctx, u.container)

internal/argparse/argparse.go

Lines changed: 24 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ const usageExitCode int = 2
1515
type positionalArg struct {
1616
name string
1717
description string
18+
required bool
1819
value interface{}
1920
}
2021

@@ -93,31 +94,33 @@ func (a *argParser) Subcommand(subcommand Subcommand) {
9394
a.subcommands[subcommand.Name()] = subcommand
9495
}
9596

96-
func (a *argParser) PositionalStringVar(name string, description string, arg *string) {
97+
func (a *argParser) PositionalStringVar(name string, description string, arg *string, required bool) {
9798
a.positionalArgs = append(a.positionalArgs, &positionalArg{
9899
name: name,
99100
description: description,
101+
required: required,
100102
value: arg,
101103
})
102104
}
103105

104-
func (a *argParser) PositionalString(name string, description string) *string {
106+
func (a *argParser) PositionalString(name string, description string, required bool) *string {
105107
arg := new(string)
106-
a.PositionalStringVar(name, description, arg)
108+
a.PositionalStringVar(name, description, arg, required)
107109
return arg
108110
}
109111

110-
func (a *argParser) PositionalListVar(name string, description string, arg *[]string) {
112+
func (a *argParser) PositionalListVar(name string, description string, arg *[]string, required bool) {
111113
a.positionalArgs = append(a.positionalArgs, &positionalArg{
112114
name: name,
113115
description: description,
116+
required: required,
114117
value: arg,
115118
})
116119
}
117120

118-
func (a *argParser) PositionalList(name string, description string) *[]string {
121+
func (a *argParser) PositionalList(name string, description string, required bool) *[]string {
119122
arg := &[]string{}
120-
a.PositionalListVar(name, description, arg)
123+
a.PositionalListVar(name, description, arg, required)
121124
return arg
122125
}
123126

@@ -131,7 +134,14 @@ func (a *argParser) Parse(ctx context.Context, args []string) {
131134
if len(a.subcommands) > 0 && len(a.positionalArgs) > 0 {
132135
panic("cannot mix subcommands and positional args")
133136
}
137+
138+
mustBeOptional := false
134139
for i, positionalArg := range a.positionalArgs {
140+
mustBeOptional = mustBeOptional || !positionalArg.required
141+
if mustBeOptional && positionalArg.required {
142+
panic("cannot have required args after optional args")
143+
}
144+
135145
if i < len(a.positionalArgs)-1 {
136146
// Only the last positional arg can be a list
137147
_, isList := positionalArg.value.(*[]string)
@@ -165,13 +175,17 @@ func (a *argParser) Parse(ctx context.Context, args []string) {
165175
} else {
166176
// Handle positional args
167177
for _, arg := range a.positionalArgs {
168-
// First, try single string case
169-
sPtr, isStr := arg.value.(*string)
170-
if isStr {
171-
if a.NArg() == 0 {
178+
if a.NArg() == 0 {
179+
if arg.required {
172180
a.Usage(ctx, "No value specified for required argument '%s'", arg.name)
181+
} else {
182+
break
173183
}
184+
}
174185

186+
// First, try single string case
187+
sPtr, isStr := arg.value.(*string)
188+
if isStr {
175189
*sPtr = a.Arg(0)
176190
a.argOffset++
177191
continue

0 commit comments

Comments
 (0)