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>

4 files changed, 4 insertions, 8 deletions

diff --git a/builtin/branch.c b/builtin/branch.c
index 4c83055730..d4359b33ac 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -644,8 +644,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 		OPT_MERGED(&filter, N_("print only branches that are merged")),
 		OPT_NO_MERGED(&filter, N_("print only branches that are not merged")),
 		OPT_COLUMN(0, "column", &colopts, N_("list branches in columns")),
-		OPT_CALLBACK(0 , "sort", sorting_tail, N_("key"),
-			     N_("field name to sort on"), &parse_opt_ref_sorting),
+		OPT_REF_SORT(sorting_tail),
 		{
 			OPTION_CALLBACK, 0, "points-at", &filter.points_at, N_("object"),
 			N_("print only branches of the object"), 0, parse_opt_object_name
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index e931be9ce4..465153e853 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -37,8 +37,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
 		OPT_INTEGER( 0 , "count", &maxcount, N_("show only <n> matched refs")),
 		OPT_STRING(  0 , "format", &format.format, N_("format"), N_("format to use for the output")),
 		OPT__COLOR(&format.use_color, N_("respect format colors")),
-		OPT_CALLBACK(0 , "sort", sorting_tail, N_("key"),
-			    N_("field name to sort on"), &parse_opt_ref_sorting),
+		OPT_REF_SORT(sorting_tail),
 		OPT_CALLBACK(0, "points-at", &filter.points_at,
 			     N_("object"), N_("print only refs which points at the given object"),
 			     parse_opt_object_name),
diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c
index 1d7f1f5ce2..6ef519514b 100644
--- a/builtin/ls-remote.c
+++ b/builtin/ls-remote.c
@@ -67,8 +67,7 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 		OPT_BIT(0, "refs", &flags, N_("do not show peeled tags"), REF_NORMAL),
 		OPT_BOOL(0, "get-url", &get_url,
 			 N_("take url.<base>.insteadOf into account")),
-		OPT_CALLBACK(0 , "sort", sorting_tail, N_("key"),
-			     N_("field name to sort on"), &parse_opt_ref_sorting),
+		OPT_REF_SORT(sorting_tail),
 		OPT_SET_INT_F(0, "exit-code", &status,
 			      N_("exit with exit code 2 if no matching refs are found"),
 			      2, PARSE_OPT_NOCOMPLETE),
diff --git a/builtin/tag.c b/builtin/tag.c
index 02f6bd1279..ad97595fbf 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -412,8 +412,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 		OPT_WITHOUT(&filter.no_commit, N_("print only tags that don't contain the commit")),
 		OPT_MERGED(&filter, N_("print only tags that are merged")),
 		OPT_NO_MERGED(&filter, N_("print only tags that are not merged")),
-		OPT_CALLBACK(0 , "sort", sorting_tail, N_("key"),
-			     N_("field name to sort on"), &parse_opt_ref_sorting),
+		OPT_REF_SORT(sorting_tail),
 		{
 			OPTION_CALLBACK, 0, "points-at", &filter.points_at, N_("object"),
 			N_("print only tags of the object"), PARSE_OPT_LASTARG_DEFAULT,