Skip to content

Conversation

@oprudkyi
Copy link
Contributor

@oprudkyi oprudkyi commented Oct 25, 2025

What type of PR is this?

Fix to allow running commands in parallel (probably partial)

  • bug
  • feature

What this PR does / why we need it:

  • extra test go test -run 'TestCommand_ParallelRun' -count 100000 -failfast -race
  • avoid global HelpFlag reuse in different commands by cloning flag
  • avoid global VersionFlag reuse in different commands
  • remove globals used for splicing slices and maps
  • defaultSliceFlagSeparator, disableSliceFlagSeparator, defaultMapFlagKeyValueSeparator converted to consts
  • command - added new MapFlagKeyValueSeparator field
  • command fields - SliceFlagSeparator, DisableSliceFlagSeparator, MapFlagKeyValueSeparator are propagated down to FlagBase and then to (MapBase|SliceBase)

Which issue(s) this PR fixes:

Fixes #2176

Special notes for your reviewer:

I just lurked around, so please recheck carefully

Testing

  • extra test go test -run 'TestCommand_ParallelRun' -count 100000 -failfast -race

Release Notes

Reduced dependency on internal global vars, allows to run commands in parallel 

`go test -run 'TestCommand_ParallelRun' -count 100000 -failfast -race`
- defaultSliceFlagSeparator, disableSliceFlagSeparator, defaultMapFlagKeyValueSeparator
converted to consts
- command - new MapFlagKeyValueSeparator field
@oprudkyi oprudkyi requested a review from a team as a code owner October 25, 2025 14:29

func (cmd *Command) set(fName string, f Flag, val string) error {
cmd.setFlags[f] = struct{}{}
cmd.setMultiValueParsingConfig(f)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

strictly speaking this call should be moved into PreParse phase.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @dearchap
I tried to avoid breaking things, and there are two set methods set and Set

Initially I thought to call setMultiValueParsingConfig from inside parseFlags(args Args) (Args, error) { or run(... but found that Set can be called without Run , at least in tests , so I moved it where setMultiValueParsingConfig will be called for sure

@dearchap dearchap merged commit 9dad0d4 into urfave:main Oct 28, 2025
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

nil pointer dereference due to HelpFlag when running in parallel

2 participants