brbzull0 commented on PR #13825:
URL: https://github.com/apache/trafficserver/pull/13825#issuecomment-6164162389
@GiulioSavini thanks, the empty-token fix and the new tests look right. I
checked them against a local build of base and this head.
On your questions:
1. Please fix the repeated option in this PR, since this PR introduces it:
base rejects every case below. The rule I suggested in the review, checking
only an earlier occurrence, is not enough. `-t --tags=dns` also loses `dns`
today, because the bare `-t` is filled from the variable first and the `=` form
is then added after it. The rule should be: the variable applies only when
every occurrence of the option is bare. If any occurrence carries a value, in
either spelling, a bare occurrence stays the usage error it is on base.
With `TS_DEBUG_TAGS` set, these should all exit 64 as on base: `-t dns
-t`, `--tags=dns -t`, `-t --tags=dns` and `-t -t dns`. The same goes for
`--run-root X --run-root` in traffic_ctl and traffic_layout, and in the library
for `-P a b -P` (arity 2) and `-p a -p` (`MORE_THAN_ONE_ARG_N`). `-t` and `-t
-t` take the variable. A line with no bare occurrence should not change at all,
including the existing last-one-wins for typed repeats (`-t a -t b` gives `b`).
Resolving the variable after all occurrences are parsed, before the
mutex, dependency and default checks, makes the result independent of order and
spelling. That works if the bare occurrences are recorded outside the option's
entry, so that a later occurrence cannot reset them, and if a bare occurrence
never resets a value another occurrence typed.
2. A partial set for an arity above 1 (`-P a`) should stay a usage error:
the variable fills an option only when it has no values at all. For
`MORE_THAN_ZERO_ARG_N`, please leave it out and say so in `ArgParser.en.rst`.
Zero values is already valid input for that arity, so there is no usage error
for the variable to replace, and that is the case #13588 is about. No option in
the tree combines that arity with a variable.
3. Please add the `traffic_ctl_cold_config.test.py` runs in this PR, since
they are part of #13588's acceptance criteria.
With 1 in place, the "A value typed on the command line always takes
precedence over the variable" sentence in `traffic_ctl.en.rst` holds. One more
sentence there and in `ArgParser.en.rst`, saying the variable is used only when
the option is given without any value, would state the rule outright.
--
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]
