From 95be717cd5a5d4956a5152210176e598cf49ec75 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 20 Mar 2019 16:22:15 -0400 Subject: 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 Signed-off-by: Junio C Hamano --- ref-filter.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) (limited to 'ref-filter.c') 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; } -- cgit v1.3