CLI: Align WSLC argument parser with Docker flag and value semantics

[David Bennett (dkbennett)]

Summary of the Pull Request

Purpose: a valid Docker command line, mapped to the equivalent wslc command, should produce a valid wslc command. This PR brings the wslc CLI argument parser in line with Docker's command-line semantics so that users coming from Docker are not surprised by parse failures or different results.

Boolean flags now accept Docker-style adjoined values: --flag, --flag=true, --flag=1, --flag=t, and their false counterparts --flag=false, --flag=0, --flag=f (all case-insensitive), on both long names and alias forms including chains (-ab=false). A space-separated token after a flag is never consumed as the flag's value (--flag value leaves value as a positional), matching Docker. Adjoined values may be double-quoted (--flag="true"), consistent with value arguments. Repeated flags and single-value arguments are last-wins, and unlimited value arguments accumulate, all matching Docker.

Flags now store their explicit parsed value (true or false) rather than being encoded purely by presence. A new ArgMap::GetFlag<ArgType::X>(defaultValue = false) shorthand folds the presence check and the stored value into a single call, and all flag on/off tests in the CLI now use it. This closes a forward-looking Docker-parity gap: a flag whose behavior is on by default can be represented and disabled with --flag=false via GetFlag<...>(true), which pure presence-encoding could not express. Contains() is unchanged and still used for "was it specified" checks (value arguments and mutual-exclusion validation).

Supporting changes: ParseBool gains an opt-in AllowExtendedForms parameter (default preserves today's behavior for every other caller; the CLI opts in to also accept t/f); the count limit is modeled as enum class Limit { Single, Unlimited } instead of an int-plus-sentinel; and a latent bug where unlimited positionals never rendered the help "..." indicator is fixed.

PR Checklist

• Closes: Link to issue #xxx
• Communication: I've discussed this with core contributors already. If work hasn't been agreed, this work might be rejected
• Tests: Added/updated if needed and all pass
• Localization: All end user facing strings can be localized
• Dev docs: Added/updated if needed
• Documentation updated: If checked, please file a pull request on our docs repo and link it here: #xxx

Detailed Description of the Pull Request / Additional comments

Docker-style boolean flag values (ArgumentParser.cpp/.h)

• ApplyFlagValue strips one pair of surrounding double quotes, then parses the token as a Docker-style boolean. Invalid tokens (for example --flag=maybe) raise an argument error instead of being silently ignored.
• StripSurroundingQuotes is shared by the flag and value adjoined-value paths so both treat ="value" identically.
• The alias, alias-chain, and named-flag paths all route through ApplyFlagValue, so -q=false, -qi=false, and --flag=false behave the same.

Flag value storage + GetFlag shorthand (ArgumentTypes.h, ArgumentParser.cpp)

• SetFlag(type, value) clears any prior entry (including an env-preloaded overridable default), then stores the explicit value. Clearing first keeps a flag at a single entry and gives last-wins for repeats (--flag --flag=false ends up false).
• ArgMap::GetFlag<E>(defaultValue = false) returns the stored value when present, or defaultValue when absent, with a static_assert that E is a Kind::Flag. Pass true for a default-on flag.

Consumer migration (43 sites)

• Every behavioral flag on/off test moved from Contains(ArgType::Flag) to GetFlag<ArgType::Flag>().
• Value/positional Contains() checks are untouched.

ParseBool extended forms (src/shared/inc/stringshared.h)

• New AllowExtendedForms parameter, defaulting to false so configfile, CommandLine.h, and OptionParser are unaffected. When true, t/f (case-insensitive) are accepted in addition to true/false/1/0. The CLI flag path opts in.

Count limit model + latent fix

• enum class Limit { Single, Unlimited } replaces the int count plus NO_LIMIT sentinel across Argument, the parser, and all commands. Single value args are last-wins; unlimited args accumulate.
• Command.cpp help rendering used Limit() > 1 (always false under the old model), so unlimited positionals never showed "..."; it now uses IsUnlimited().

Validation Steps Performed

• WSLCCLIParserUnitTests.cpp: value-aware assertions via GetFlag; Flag_FalseForms_StoreSingleFalseEntry (present with false); Flag_FalseOverridesPreloadedDefault; and Flag_GetFlagDefaultTrue_DefaultOnFlag demonstrating the default-on pattern. ParserTestCases.h covers true/false/1/0/t/f, quoted, alias, and reject cases. SimpleTests.cpp covers ParseBool default vs extended modes.

[Copilot]

Pull request overview

Updates the WSLC CLI argument parsing and consumption model to match Docker’s flag/value semantics, reducing surprising parse failures and enabling explicit boolean flag values.

Changes:

• Implement Docker-style boolean flag value parsing (including =true/false/1/0/t/f, case-insensitive) and ensure space-separated tokens after flags remain positional.
• Change single-value arguments (including flags) to “last-wins” behavior and model argument multiplicity via Limit::{Single,Unlimited}.
• Migrate flag consumers to ArgMap::GetFlag<...>(defaultValue) and extend shared ParseBool with an opt-in extended-forms parameter; update/expand parser unit tests.

Reviewed changes

Copilot reviewed 44 out of 44 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
test/windows/wslc/WSLCCLIParserUnitTests.cpp	Updates expectations for last-wins semantics; adds targeted tests for stored flag values and Docker-style boolean forms.
test/windows/wslc/ParserTestCases.h	Extends parser test matrix for last-wins single-value args and boolean =value flag parsing rules.
test/windows/SimpleTests.cpp	Adds coverage for ParseBool(..., AllowExtendedForms=true) including t/f forms.
src/windows/wslc/tasks/VolumeTasks.cpp	Switches flag checks from Contains() to GetFlag() for Docker-style explicit false handling.
src/windows/wslc/tasks/SessionTasks.cpp	Switches verbose flag usage to GetFlag().
src/windows/wslc/tasks/NetworkTasks.cpp	Switches internal/quiet/force/all flag usage to GetFlag().
src/windows/wslc/tasks/ImageTasks.cpp	Switches multiple flags (verbose/quiet/no-trunc/etc.) to GetFlag() to honor explicit false.
src/windows/wslc/tasks/ContainerTasks.cpp	Switches numerous flags to GetFlag() to align runtime behavior with stored boolean values.
src/windows/wslc/core/Command.cpp	Adjusts help rendering for unlimited args and switches help checks to GetFlag(); removes now-obsolete “too many” validation.
src/windows/wslc/core/CLIExecutionContext.cpp	Switches global --no-color handling to GetFlag().
src/windows/wslc/commands/*.cpp	Replaces NO_LIMIT with Limit::Unlimited across command argument definitions; updates certain mutual-exclusion checks to use GetFlag() for flags.
src/windows/wslc/arguments/ArgumentTypes.h	Introduces Limit enum and ArgMap::GetFlag helper.
src/windows/wslc/arguments/ArgumentParser.h / .cpp	Implements stored flag values, shared quote stripping, last-wins single-value behavior, and Docker-style adjoined boolean parsing.
src/windows/wslc/arguments/Argument.h / .cpp	Replaces integer count limit with Limit and adds IsSingle()/IsUnlimited() helpers.
src/shared/inc/stringshared.h	Extends ParseBool with an opt-in AllowExtendedForms parameter supporting t/f.
localization/strings/en-US/Resources.resw	Removes the unused “too many arguments” string after semantics change.

[Copilot]

Pull request overview

Copilot reviewed 44 out of 44 changed files in this pull request and generated no new comments.

[ggarzia-MSFT]

lgtm

[Blue (OneBlue)]

LGTM, minor suggestion

[Blue (OneBlue)]

nit: If an override happens, I think outputting a warning would be a good idea

[David Bennett (dkbennett)]

I agree that it may indicate a user mistake and a warning makes sense, however Docker does not do this and overwrites are silent. The reason for this is partially that it is the POSIX/GNU convention and Docker inherited it from dependent packages. The functional reason is that the idea is defaults layer and later things can override. This behavior would also allow us to have a feature for configurable defaults that are later overridden by the command line, but some users may do this today via scripting.

Given that this behavior has been in docker for a decade and some consumers may rely on it, I am inclined to not put in a warning on overrides as that would violate the convention and might pose problems for users and scripts and future features that may rely on this override behavior.

I think it is worth discussing this behavior and whether we want to intentionally diverge, but that can be a follow-up PR.