On Tue, Jan 18 2022, Josh Steadmon wrote:
quoted hunk ↗ jump to hunk
As Ævar pointed out in [1], the use of PARSE_OPT_LITERAL_ARGHELP with a
list of allowed parameters is not recommended. Both git-branch and
git-checkout were changed in d311566 (branch: add flags and config to
inherit tracking, 2021-12-20) to use this discouraged combination for
their --track flags.
Fix this by removing PARSE_OPT_LITERAL_ARGHELP, and changing the arghelp
to simply be "mode". Users may discover allowed values in the manual
pages.
[1]: https://lore.kernel.org/git/220111.86a6g3yqf9.gmgdl@evledraar.gmail.com/ (local)
Signed-off-by: Josh Steadmon <redacted>
---
builtin/branch.c | 4 ++--
builtin/checkout.c | 6 +++---
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index 2251e6a54f..0c8d8a8827 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -638,9 +638,9 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
OPT__VERBOSE(&filter.verbose,
N_("show hash and subject, give twice for upstream branch")),
OPT__QUIET(&quiet, N_("suppress informational messages")),
- OPT_CALLBACK_F('t', "track", &track, "direct|inherit",
+ OPT_CALLBACK_F('t', "track", &track, N_("mode"),
N_("set branch tracking configuration"),
- PARSE_OPT_OPTARG | PARSE_OPT_LITERAL_ARGHELP,
+ PARSE_OPT_OPTARG,
parse_opt_tracking_mode),
OPT_SET_INT_F(0, "set-upstream", &track, N_("do not use"),
BRANCH_TRACK_OVERRIDE, PARSE_OPT_HIDDEN),diff --git a/builtin/checkout.c b/builtin/checkout.c
index 94814c37b4..6a5dd2a2a2 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -1549,9 +1549,9 @@ static struct option *add_common_switch_branch_options(
{
struct option options[] = {
OPT_BOOL('d', "detach", &opts->force_detach, N_("detach HEAD at named commit")),
- OPT_CALLBACK_F('t', "track", &opts->track, "direct|inherit",
- N_("set up tracking mode (see git-pull(1))"),
- PARSE_OPT_OPTARG | PARSE_OPT_LITERAL_ARGHELP,
+ OPT_CALLBACK_F('t', "track", &opts->track, N_("mode"),
+ N_("set branch tracking configuration"),
+ PARSE_OPT_OPTARG,
parse_opt_tracking_mode),
OPT__FORCE(&opts->force, N_("force checkout (throw away local modifications)"),
PARSE_OPT_NOCOMPLETE),
base-commit: b56bd95bbc8f716cb8cbb5fdc18b9b0f00323c6a
Additional comment: I think this change is correct, as noted in my
just-sent
https://lore.kernel.org/git/220120.864k5ymx55.gmgdl@evledraar.gmail.com/ (local);
it would be nice but not necessary to follow-up with an optbug() test as
noted in
https://lore.kernel.org/git/220119.867davokff.gmgdl@evledraar.gmail.com/ (local)
though.
But to not merely repeat myself, I also saw that we're emitting the
wrong output from usage_argh() in cases of !PARSE_OPT_NOARG. I.e. we
need this fix too:
diff --git a/parse-options.c b/parse-options.c
index a8283037be9..2be1eabd84e 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -915,8 +915,10 @@ static int usage_argh(const struct option *opts, FILE *outfile)
s = literal ? "[=%s]" : "[=<%s>]";
else
s = literal ? "[%s]" : "[<%s>]";
- else
+ else if (opts->flags & PARSE_OPT_NOARG)
s = literal ? " %s" : " <%s>";
+ else
+ s = literal ? "[=]%s" : "[=]<%s>";
return utf8_fprintf(outfile, s, opts->argh ? _(opts->argh) : _("..."));
}
With that we'll now emit:
$ ./git add -h 2>&1|grep chmod
--chmod[=](+|-)x override the executable bit of the listed files
Which is correct, as we accept both of:
git add --chmod +x
git add --chmod=+x
But not:
$ git add --chmod
error: parse-options.c:58: option `chmod' requires a value
But the usage output stated that the "=" was mandatory before.