GiulioSavini commented on PR #13825: URL: https://github.com/apache/trafficserver/pull/13825#issuecomment-6142762657
Thanks for the detailed review, @brbzull0. Pushed the blocking fix: an empty token at `args[j]` is now refused immediately (`N argument(s) expected by <name>`) instead of being treated the same as `j >= args.size()`. That also fixes the root cause you pointed out — `j` never advancing past the empty token, which left it in `args` for the enclosing command to read as a positional. Added both your repro cases (`-t "" rec` and `-t -- "" rec`) as `REQUIRE_THROWS`. Also added the three missing-coverage cases you listed: a command's own declared variable (`init` with `TS_ARGPARSER_TEST_FIXED`), that `-m` alone never activates `cold`/`tags` from their variables, and that a typed value for `-c`/`-p` wins outright (checked via `.value()`/`size()`, not just the earlier size-only checks). Left open, since they're design calls rather than bugs in this PR: - **Repeated-option precedence** (`--tags dns --tags` letting the bare second occurrence overwrite the typed one with the variable): agree this should probably be a hard error like it was before, rather than silently displacing a typed value. Want me to add that in this PR, or should it be a separate follow-up? - **`MORE_THAN_ZERO_ARG_N` not reading the variable**, and the **mixed fixed-arity fill** (`-P a` → `['a', 'ENV']`): these are genuinely open questions about the intended contract rather than something I should decide unilaterally. Happy to implement whichever reading you prefer, or to write up the exclusion in `ArgParser.en.rst` if `MORE_THAN_ZERO_ARG_N` is meant to stay out. - **`traffic_ctl_cold_config.test.py` e2e coverage**: haven't added this yet. Can follow up with the two extra `ts.Env` runs you described if that's wanted in this same PR rather than as a fast-follow. `test_tscore` (116/116) and `cmake --build build-dev --target format` both clean on top of the new commit. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
