On Wed, Jan 19 2022, René Scharfe wrote:
Am 11.01.22 um 02:57 schrieb Ævar Arnfjörð Bjarmason:
quoted
On Mon, Dec 20 2021, Josh Steadmon wrote:
quoted
Since we've added an argument to "--track", also add "--track=direct" as
another way to explicitly get the original "--track" behavior ("--track"
without an argument still works as well).
[...]
-'git branch' [--track | --no-track] [-f] <branchname> [<start-point>]
+'git branch' [--track [direct|inherit] | --no-track] [-f] <branchname> [<start-point>]
The usage info here is correct...
Actually it isn't, because optional arguments need the equal sign. I.e.
this works as expected:
git branch --track=direct branch
But this here interprets "direct" as a branch name (and branch as a
start point):
git branch --track direct branch
The usage string could start with:
'git branch' [--track | --track=direct | --track=inherit | --no-track]
... or the less repetitive:
'git branch' [--track[=(direct|inherit)] | --no-track]
Options with required arguments accept either whitespace or an equal
sign between option name and arg. With PARSE_OPT_OPTARG we cannot
accept whitespace because we cannot decide whether the next thing after
the option name is an argument or the next parameter.
Well spotted. Your downthread patch LGTM (with the small nit I noted
that having an optbug() check for this would be even better).
quoted
quoted
---track::
+--track [inherit|direct]::
...as is this...
Same here:
--track[=(direct|inherit)]
quoted
quoted
- OPT_SET_INT('t', "track", &track, N_("set up tracking mode (see git-pull(1))"),
- BRANCH_TRACK_EXPLICIT),
+ OPT_CALLBACK_F('t', "track", &track, "direct|inherit",
+ N_("set branch tracking configuration"),
+ PARSE_OPT_OPTARG | PARSE_OPT_LITERAL_ARGHELP,
+ parse_opt_tracking_mode),
....
quoted
struct option options[] = {
OPT_BOOL('d', "detach", &opts->force_detach, N_("detach HEAD at named commit")),
- OPT_SET_INT('t', "track", &opts->track, N_("set upstream info for new branch"),
- BRANCH_TRACK_EXPLICIT),
+ OPT_CALLBACK_F('t', "track", &opts->track, "direct|inherit",
... but these are not. I.e. we'll emit:
-t, --track[=direct|inherit]
set branch tracking configuration
I.e. implying that the valid uses are --track, --track=direct, and
--trackinherit.
Well spotted. It should be specified as "(direct|inherit)" (i.e. with
parens).
*nod*
quoted
It looks like the problem is (ab)use of PARSE_OPT_OPTARG, i.e. it was
never meant for an enumeration of possible values, but for something
like N_("mode"). It could be made to support that, but it would require
some light patching of the releant bits of parse-options.c.
Could you please elaborate that point? AFAIU PARSE_OPT_OPTARG just
requires arguments to be attached with equal signs and there is no
limitation regarding the number of possible argument values.
I'd skimmed the code & -h generation, but see on a second reading that I
was just wrong about this.
I.e. I thought it would always misformat alternate args, but as your
downthread patch shows where we'll now for "git am -h" emit e.g.:
--show-current-patch[=(diff|raw)]
The output is now correct (and was before, we were just giving the flag
rudendantly).
quoted
The PARSE_OPT_LITERAL_ARGHELP should also be dropped if it's fixed to
use a string like "mode".
That's true. And it's also enabled automatically if the argument help
string contains any of the following characters: ()<>[]|. So basically
it's never needed explicitly..