All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: git@vger.kernel.org
Subject: [PATCH] show-branch: reject --[no-](topo|date)-order
Date: Wed, 19 Jul 2023 09:32:36 -0700	[thread overview]
Message-ID: <xmqqh6pzc15n.fsf@gitster.g> (raw)
In-Reply-To: <xmqq7cqwcet8.fsf@gitster.g> (Junio C. Hamano's message of "Wed, 19 Jul 2023 04:37:39 -0700")

"git show-branch --no-topo-order" behaved exactly the same way as
"git show-branch --topo-order" did, which was nonsense.  This was
because we choose between topo- and date- by setting a variable to
either REV_SORT_IN_GRAPH_ORDER or REV_SORT_BY_COMMIT_DATE with
OPT_SET_INT() and REV_SORT_IN_GRAPH_ORDER happens to be 0.  The
OPT_SET_INTO() macro assigns 0 to the target variable in respose to
the negated form of its option.

"--no-date-order" by luck behaves identically to "--topo-order"
exactly for the same reason, and it sort-of makes sense right now,
but the "sort-of makes sense" will quickly break down once we add a
third way to sort.  Not-A may be B when there are only two choices
between A and B, but once your choices become among A, B, and C,
not-A does not mean B.

Just mark these two ordering options to reject negation, and add a
test, which was missing.  "git show-branch --no-reflog" is also
unnegatable, so throw in a test for that while we are at it.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 * Comes on top of the "--no-sparse should mean dense" patch.

 builtin/show-branch.c  | 14 +++++++-------
 t/t3202-show-branch.sh |  9 +++++++++
 2 files changed, 16 insertions(+), 7 deletions(-)

diff --git c/builtin/show-branch.c w/builtin/show-branch.c
index 67f3ef0639..b574afb7c3 100644
--- c/builtin/show-branch.c
+++ w/builtin/show-branch.c
@@ -667,17 +667,17 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)
 			 N_("show possible merge bases")),
 		OPT_BOOL(0, "independent", &independent,
 			    N_("show refs unreachable from any other ref")),
-		OPT_SET_INT(0, "topo-order", &sort_order,
-			    N_("show commits in topological order"),
-			    REV_SORT_IN_GRAPH_ORDER),
+		OPT_SET_INT_F(0, "topo-order", &sort_order,
+			      N_("show commits in topological order"),
+			      REV_SORT_IN_GRAPH_ORDER, PARSE_OPT_NONEG),
 		OPT_BOOL(0, "topics", &topics,
 			 N_("show only commits not on the first branch")),
 		OPT_SET_INT(0, "sparse", &sparse,
 			    N_("show merges reachable from only one tip"), 1),
-		OPT_SET_INT(0, "date-order", &sort_order,
-			    N_("topologically sort, maintaining date order "
-			       "where possible"),
-			    REV_SORT_BY_COMMIT_DATE),
+		OPT_SET_INT_F(0, "date-order", &sort_order,
+			      N_("topologically sort, maintaining date order "
+				 "where possible"),
+			      REV_SORT_BY_COMMIT_DATE, PARSE_OPT_NONEG),
 		OPT_CALLBACK_F('g', "reflog", &reflog_base, N_("<n>[,<base>]"),
 			    N_("show <n> most recent ref-log entries starting at "
 			       "base"),
diff --git c/t/t3202-show-branch.sh w/t/t3202-show-branch.sh
index ca7c44f0f7..b17f388f56 100755
--- c/t/t3202-show-branch.sh
+++ w/t/t3202-show-branch.sh
@@ -213,6 +213,15 @@ done <<\EOF
 --reflog --current
 EOF
 
+# unnegatable options
+for opt in topo-order date-order reflog
+do
+	test_expect_success "show-branch --no-$opt (should fail)" '
+		test_must_fail git show-branch --no-$opt 2>err &&
+		grep "unknown option .no-$opt." err
+	'
+done
+
 test_expect_success 'error descriptions on non-existent branch' '
 	cat >expect <<-EOF &&
 	error: No branch named '\''non-existent'\'.'



      reply	other threads:[~2023-07-19 16:32 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-07-19 11:37 [PATCH] show-branch: --no-sparse should give dense output Junio C Hamano
2023-07-19 16:32 ` Junio C Hamano [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqh6pzc15n.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.