diff options
author | Jeff King <peff@peff.net> | 2019-03-20 23:22:15 +0300 |
---|---|---|
committer | Junio C Hamano <gitster@pobox.com> | 2019-03-21 06:03:35 +0300 |
commit | 95be717cd5a5d4956a5152210176e598cf49ec75 (patch) | |
tree | 31818e4c171801707212006b7856a0d6a089de76 /ref-filter.c | |
parent | 9a1180fc304ad9831641e5788e9c8d3dfc10ccdd (diff) |
parse_opt_ref_sorting: always use with NONEG flag
The "--sort" parameter of for-each-ref, etc, does not handle negation,
and instead returns an error to the parse-options code. But neither
piece of code prints anything for the user, which may leave them
confused:
$ git for-each-ref --no-sort
$ echo $?
129
As the comment in the callback function notes, this probably should
clear the list, which would make it consistent with other list-like
options (i.e., anything that uses OPT_STRING_LIST currently).
Unfortunately that's a bit tricky due to the way the ref-filter code
works. But in the meantime, let's at least make the error a little less
confusing:
- switch to using PARSE_OPT_NONEG in the option definition, which will
cause the options code to produce a useful message
- since this was cut-and-pasted to four different spots, let's define
a single OPT_REF_SORT() macro that we can use everywhere
- the callback can use BUG_ON_OPT_NEG() to make sure the correct flags
are used (incidentally, this also satisfies -Wunused-parameters,
since we're now looking at "unset")
- expand the comment into a NEEDSWORK to make it clear that the
direction is right, but the details need to be worked out
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Diffstat (limited to 'ref-filter.c')
-rw-r--r-- | ref-filter.c | 9 |
1 files changed, 7 insertions, 2 deletions
diff --git a/ref-filter.c b/ref-filter.c index 3aca105307..8d11a94cbd 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -2337,8 +2337,13 @@ void parse_ref_sorting(struct ref_sorting **sorting_tail, const char *arg) int parse_opt_ref_sorting(const struct option *opt, const char *arg, int unset) { - if (!arg) /* should --no-sort void the list ? */ - return -1; + /* + * NEEDSWORK: We should probably clear the list in this case, but we've + * already munged the global used_atoms list, which would need to be + * undone. + */ + BUG_ON_OPT_NEG(unset); + parse_ref_sorting(opt->value, arg); return 0; } |