Skip to content

Commit 97636c3

Browse files
committed
Bug fixes for arg parsing and support for positional arguments
* Bug Fix - Missing Required Args * Bug Fix - Neo4j Duplicate Arguments * Test Case Corrections * Added Positional Arguments Support (in CLI config set args too) * Terminal text output update for results
1 parent 644e110 commit 97636c3

File tree

15 files changed

+91972
-758
lines changed

15 files changed

+91972
-758
lines changed

cmd/config/args/set.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ func (c *SetCmd) run(cmd *cobra.Command, args []string) error {
5656
return fmt.Errorf("server-name is required")
5757
}
5858

59-
normalizedArgs := config.NormalizeArgs(args[1:])
59+
normalizedArgs := config.ProcessAllArgs(args[1:])
6060
cfg, err := c.ctxLoader.Load(flags.RuntimeFile)
6161
if err != nil {
6262
return fmt.Errorf("failed to load execution context config: %w", err)

internal/config/args.go

Lines changed: 55 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,6 @@ import (
77
)
88

99
const (
10-
// OptionTerminator represents the character sequence generally understood to indicate the termination of options
11-
// to be parsed that are supplied to a command line interface.
12-
// e.g. in the command: kubectl exec random_pod -- ls
13-
// 'ls' is not parsed by kubectl and instead passed verbatim to the random_pod.
14-
OptionTerminator = "--"
15-
1610
// FlagPrefixLong represents the expected prefix for a long format flag (e.g. --flag).
1711
FlagPrefixLong = "--"
1812

@@ -32,7 +26,7 @@ const (
3226
// --flag=value -> preserved as-is
3327
// -xyz -> -x, -y, -z (expanded short flags)
3428
//
35-
// Parsing stops at OptionTerminator ("--"). Positional arguments and arguments after OptionTerminator are excluded.
29+
// Positional arguments are excluded.
3630
//
3731
// This function is intended for internal normalization of flag arguments only.
3832
func NormalizeArgs(rawArgs []string) []string {
@@ -42,10 +36,6 @@ func NormalizeArgs(rawArgs []string) []string {
4236
for i := 0; i < numArgs; i++ {
4337
arg := strings.TrimSpace(rawArgs[i])
4438

45-
if arg == OptionTerminator {
46-
break // stop parsing flags (exclude "--" and everything after).
47-
}
48-
4939
nextIndex := i + 1
5040
hasNext := nextIndex < numArgs
5141
isShortFlag := strings.HasPrefix(arg, FlagPrefixShort) && !strings.HasPrefix(arg, FlagPrefixLong)
@@ -152,6 +142,60 @@ func MergeArgs(a, b []string) []string {
152142
return result
153143
}
154144

145+
// ProcessAllArgs processes a slice of arguments, normalizing flags while preserving positional arguments.
146+
// It processes arguments sequentially, normalizing flag groups as it encounters them,
147+
// and returns them in their original relative order.
148+
//
149+
// Examples:
150+
// - ["--flag", "value", "pos1", "pos2"] -> ["--flag=value", "pos1", "pos2"]
151+
// - ["pos1", "--flag=value", "pos2"] -> ["pos1", "--flag=value", "pos2"]
152+
// - ["/path/to/dir", "--verbose"] -> ["/path/to/dir", "--verbose"]
153+
func ProcessAllArgs(rawArgs []string) []string {
154+
if len(rawArgs) == 0 {
155+
return []string{}
156+
}
157+
158+
var result []string
159+
160+
// Process arguments sequentially
161+
for i := 0; i < len(rawArgs); i++ {
162+
arg := strings.TrimSpace(rawArgs[i])
163+
164+
isFlag := strings.HasPrefix(arg, FlagPrefixShort) || strings.HasPrefix(arg, FlagPrefixLong)
165+
if isFlag {
166+
// Check for combined short flags (like -xyz) which should not consume next arg as value
167+
isCombinedShortFlag := strings.HasPrefix(arg, FlagPrefixShort) &&
168+
!strings.HasPrefix(arg, FlagPrefixLong) &&
169+
len(arg) > 2 &&
170+
!strings.Contains(arg, FlagValueSeparator)
171+
172+
// Collect this flag and potentially its value for normalization
173+
flagGroup := []string{arg}
174+
175+
// Check if next argument is a value for this flag (not embedded with =)
176+
// Don't consume next arg for combined short flags
177+
if !isCombinedShortFlag && !strings.Contains(arg, FlagValueSeparator) && i+1 < len(rawArgs) {
178+
nextArg := strings.TrimSpace(rawArgs[i+1])
179+
nextIsFlag := strings.HasPrefix(nextArg, FlagPrefixShort) || strings.HasPrefix(nextArg, FlagPrefixLong)
180+
if !nextIsFlag {
181+
// Include the value
182+
flagGroup = append(flagGroup, nextArg)
183+
i++ // Skip the next argument since we processed it
184+
}
185+
}
186+
187+
// Normalize this flag group and add to result
188+
normalizedFlags := NormalizeArgs(flagGroup)
189+
result = append(result, normalizedFlags...)
190+
} else {
191+
// Positional argument - add as-is
192+
result = append(result, arg)
193+
}
194+
}
195+
196+
return result
197+
}
198+
155199
// parseArgs converts a slice of argument strings into a map of argEntry
156200
func parseArgs(args []string) map[string]argEntry {
157201
result := make(map[string]argEntry, len(args))

internal/config/args_test.go

Lines changed: 85 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -40,11 +40,6 @@ func TestNormalizeArgs(t *testing.T) {
4040
input: []string{"-xzv"},
4141
expected: []string{"-x", "-z", "-v"},
4242
},
43-
{
44-
name: "terminator excludes all after",
45-
input: []string{"--foo", "bar", "--", "--ignored", "positional"},
46-
expected: []string{"--foo=bar"},
47-
},
4843
{
4944
name: "positional args are skipped",
5045
input: []string{"--foo", "bar", "main.go", "--baz", "qux"},
@@ -490,3 +485,88 @@ func TestArgEntry_String(t *testing.T) {
490485
})
491486
}
492487
}
488+
489+
func TestProcessAllArgs(t *testing.T) {
490+
t.Parallel()
491+
492+
tests := []struct {
493+
name string
494+
input []string
495+
expected []string
496+
}{
497+
{
498+
name: "empty input",
499+
input: []string{},
500+
expected: []string{},
501+
},
502+
{
503+
name: "only positional args",
504+
input: []string{"pos1", "pos2", "pos3"},
505+
expected: []string{"pos1", "pos2", "pos3"},
506+
},
507+
{
508+
name: "only flags with embedded values",
509+
input: []string{"--flag=value", "--other=val"},
510+
expected: []string{"--flag=value", "--other=val"},
511+
},
512+
{
513+
name: "only boolean flags",
514+
input: []string{"--verbose", "--debug"},
515+
expected: []string{"--verbose", "--debug"},
516+
},
517+
{
518+
name: "flags with separate values (regression test)",
519+
input: []string{"--config", "dev.toml", "--output", "result.txt"},
520+
expected: []string{"--config=dev.toml", "--output=result.txt"},
521+
},
522+
{
523+
name: "mixed flags and positional args maintaining order",
524+
input: []string{"pos1", "--flag=value", "pos2", "--verbose", "pos3"},
525+
expected: []string{"pos1", "--flag=value", "pos2", "--verbose=pos3"},
526+
},
527+
{
528+
name: "flag with separate value followed by positional",
529+
input: []string{"--config", "dev.toml", "positional"},
530+
expected: []string{"--config=dev.toml", "positional"},
531+
},
532+
{
533+
name: "positional followed by flag with separate value",
534+
input: []string{"positional", "--config", "dev.toml"},
535+
expected: []string{"positional", "--config=dev.toml"},
536+
},
537+
{
538+
name: "complex mix with various flag formats",
539+
input: []string{"pos1", "--flag=embedded", "--other", "separate", "--bool", "pos2"},
540+
expected: []string{"pos1", "--flag=embedded", "--other=separate", "--bool=pos2"},
541+
},
542+
{
543+
name: "short flags with separate values",
544+
input: []string{"-f", "value", "-x", "other"},
545+
expected: []string{"-f=value", "-x=other"},
546+
},
547+
{
548+
name: "combined short flags",
549+
input: []string{"-xyz", "pos1"},
550+
expected: []string{"-x", "-y", "-z", "pos1"},
551+
},
552+
{
553+
name: "flag followed by another flag (no value consumption)",
554+
input: []string{"--verbose", "--debug", "pos1"},
555+
expected: []string{"--verbose", "--debug=pos1"},
556+
},
557+
{
558+
name: "multiple consecutive flags with separate values",
559+
input: []string{"--first", "val1", "--second", "val2", "--third", "val3"},
560+
expected: []string{"--first=val1", "--second=val2", "--third=val3"},
561+
},
562+
}
563+
564+
for _, tc := range tests {
565+
t.Run(tc.name, func(t *testing.T) {
566+
t.Parallel()
567+
568+
actual := ProcessAllArgs(tc.input)
569+
require.Equal(t, tc.expected, actual, "ProcessAllArgs should preserve order and normalize flags correctly")
570+
})
571+
}
572+
}

internal/packages/argument.go

Lines changed: 56 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,9 @@ const (
1515

1616
// VariableTypeArgBool represents a command line argument that is a boolean flag (doesn't have a value).
1717
VariableTypeArgBool VariableType = "argument_bool"
18+
19+
// VariableTypePositionalArg represents a positional command line argument.
20+
VariableTypePositionalArg VariableType = "positional_argument"
1821
)
1922

2023
// EnvVarPlaceholderRegex is used to find environment variable placeholders like ${VAR_NAME}.
@@ -27,9 +30,11 @@ type Arguments map[string]ArgumentMetadata
2730

2831
// ArgumentMetadata represents metadata about an argument/variable
2932
type ArgumentMetadata struct {
33+
Name string `json:"name"`
3034
Description string `json:"description"`
3135
Required bool `json:"required"`
3236
VariableType VariableType `json:"type"`
37+
Position *int `json:"position,omitempty"` // Position in args array for positional arguments
3338
}
3439

3540
// FilterBy allows filtering of Arguments using predicates.
@@ -43,6 +48,46 @@ func (a Arguments) Names() []string {
4348
return slices.Collect(maps.Keys(a))
4449
}
4550

51+
// Ordered returns all arguments with positional arguments first (in position order),
52+
// followed by all other arguments in alphabetical order by name.
53+
func (a Arguments) Ordered() []ArgumentMetadata {
54+
var positional []ArgumentMetadata
55+
var others []ArgumentMetadata
56+
57+
for name, meta := range a {
58+
// Ensure name is set in the metadata
59+
meta.Name = name
60+
61+
if meta.VariableType == VariableTypePositionalArg && meta.Position != nil {
62+
positional = append(positional, meta)
63+
} else {
64+
others = append(others, meta)
65+
}
66+
}
67+
68+
// Sort positional by position
69+
slices.SortFunc(positional, func(a, b ArgumentMetadata) int {
70+
return *a.Position - *b.Position
71+
})
72+
73+
// Sort others alphabetically by name
74+
slices.SortFunc(others, func(a, b ArgumentMetadata) int {
75+
if a.Name < b.Name {
76+
return -1
77+
} else if a.Name > b.Name {
78+
return 1
79+
}
80+
return 0
81+
})
82+
83+
// Combine: positional first, then others
84+
result := make([]ArgumentMetadata, 0, len(positional)+len(others))
85+
result = append(result, positional...)
86+
result = append(result, others...)
87+
88+
return result
89+
}
90+
4691
// FilterArguments allows Arguments to be filtered using any number of predicates.
4792
// All predicates must be true in order for an argument to be included in the results.
4893
func FilterArguments(args Arguments, predicate ...func(name string, data ArgumentMetadata) bool) Arguments {
@@ -71,7 +116,7 @@ func EnvVar(_ string, data ArgumentMetadata) bool {
71116

72117
// Argument is a predicate that requires the argument is a command line argument.
73118
func Argument(s string, data ArgumentMetadata) bool {
74-
return ValueArgument(s, data) || BoolArgument(s, data)
119+
return ValueArgument(s, data) || BoolArgument(s, data) || PositionalArgument(s, data)
75120
}
76121

77122
// ValueArgument is a predicate that requires the argument is a command line argument which requires a value.
@@ -83,3 +128,13 @@ func ValueArgument(_ string, data ArgumentMetadata) bool {
83128
func BoolArgument(_ string, data ArgumentMetadata) bool {
84129
return data.VariableType == VariableTypeArgBool
85130
}
131+
132+
// PositionalArgument is a predicate that requires the argument is a positional command line argument.
133+
func PositionalArgument(_ string, data ArgumentMetadata) bool {
134+
return data.VariableType == VariableTypePositionalArg
135+
}
136+
137+
// NonPositionalArgument is a predicate that requires the argument is not a positional command line argument.
138+
func NonPositionalArgument(s string, data ArgumentMetadata) bool {
139+
return !PositionalArgument(s, data)
140+
}

internal/printer/package_printer.go

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -145,12 +145,25 @@ func (p *PackagePrinter) Item(w io.Writer, pkg packages.Package) error {
145145
}
146146
}
147147

148-
args := pkg.Arguments.FilterBy(packages.Argument).Names()
149-
if len(args) > 0 {
150-
if _, err := fmt.Fprintln(w, " 📋 Args configurable via command line..."); err != nil {
148+
// Positional arguments (in order)
149+
positionalArgs := pkg.Arguments.FilterBy(packages.PositionalArgument).Ordered()
150+
if len(positionalArgs) > 0 {
151+
if _, err := fmt.Fprintln(w, " 📍 Positional arguments:"); err != nil {
151152
return err
152153
}
153-
if _, err := fmt.Fprintf(w, " 🖥️ %s\n", strings.Join(args, ", ")); err != nil {
154+
155+
for _, arg := range positionalArgs {
156+
_, _ = fmt.Fprintf(w, "\t(%d) %s\n", *arg.Position, arg.Name)
157+
}
158+
}
159+
160+
// Command-line flags (excluding positional arguments)
161+
flagArgs := pkg.Arguments.FilterBy(packages.NonPositionalArgument).Names()
162+
if len(flagArgs) > 0 {
163+
if _, err := fmt.Fprintln(w, " 📋 Args configurable via command line flags..."); err != nil {
164+
return err
165+
}
166+
if _, err := fmt.Fprintf(w, " 🖥️ %s\n", strings.Join(flagArgs, ", ")); err != nil {
154167
return err
155168
}
156169
}

0 commit comments

Comments
 (0)