{"thread":{"id":"60010","subject":"[PATCH] show-branch: --no-sparse should give dense output","startedAt":"2023-07-19T11:39:00Z","lastAt":"2023-07-19T16:32:45Z","messageCount":2,"participants":["Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"479632","messageId":"xmqq7cqwcet8.fsf@gitster.g","threadId":"60010","inReplyTo":null,"subject":"[PATCH] show-branch: --no-sparse should give dense output","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-07-19T11:37:39Z","receivedAt":"2023-07-19T11:39:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"git show-branch --no-sparse\" behaved exactly the same way as \"git\nshow-branch --sparse\", which did not make any sense.  This was\nbecause it used a variable \"dense\" initialized to 1 by default to\ngive \"non sparse\" behaviour, and OPT_SET_INT() to set the varilable\nto 0 in response to the \"--sparse\" option.  Unfortunately,\nOPT_SET_INT() sets 0 to the given variable when the option is\nnegated.\n\nFlip the polarity of the variable \"dense\" by renaming it to \"sparse\"\nand initializing it to 0, and have OPT_SET_INT() set the variable to\n1 when \"--sparse\" is given.  This way, \"--no-sparse\" would set 0 to\nthe variable and would give us the \"dense\" behaviour.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n builtin/show-branch.c  |  8 ++++----\n t/t3202-show-branch.sh | 16 ++++++++++++++++\n 2 files changed, 20 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/show-branch.c b/builtin/show-branch.c\nindex 7ef4a642c1..67f3ef0639 100644\n--- a/builtin/show-branch.c\n+++ b/builtin/show-branch.c\n@@ -645,7 +645,7 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)\n \tint with_current_branch = 0;\n \tint head_at = -1;\n \tint topics = 0;\n-\tint dense = 1;\n+\tint sparse = 0;\n \tconst char *reflog_base = NULL;\n \tstruct option builtin_show_branch_options[] = {\n \t\tOPT_BOOL('a', \"all\", &all_heads,\n@@ -672,8 +672,8 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)\n \t\t\t    REV_SORT_IN_GRAPH_ORDER),\n \t\tOPT_BOOL(0, \"topics\", &topics,\n \t\t\t N_(\"show only commits not on the first branch\")),\n-\t\tOPT_SET_INT(0, \"sparse\", &dense,\n-\t\t\t    N_(\"show merges reachable from only one tip\"), 0),\n+\t\tOPT_SET_INT(0, \"sparse\", &sparse,\n+\t\t\t    N_(\"show merges reachable from only one tip\"), 1),\n \t\tOPT_SET_INT(0, \"date-order\", &sort_order,\n \t\t\t    N_(\"topologically sort, maintaining date order \"\n \t\t\t       \"where possible\"),\n@@ -936,7 +936,7 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)\n \t\t\t    !is_merge_point &&\n \t\t\t    (this_flag & (1u << REV_SHIFT)))\n \t\t\t\tcontinue;\n-\t\t\tif (dense && is_merge &&\n+\t\t\tif (!sparse && is_merge &&\n \t\t\t    omit_in_dense(commit, rev, num_rev))\n \t\t\t\tcontinue;\n \t\t\tfor (i = 0; i < num_rev; i++) {\ndiff --git a/t/t3202-show-branch.sh b/t/t3202-show-branch.sh\nindex be20ebe1d5..ca7c44f0f7 100755\n--- a/t/t3202-show-branch.sh\n+++ b/t/t3202-show-branch.sh\n@@ -119,6 +119,22 @@ test_expect_success 'show branch --remotes' '\n \ttest_must_be_empty actual.out\n '\n \n+test_expect_success 'show-branch --sparse' '\n+\ttest_when_finished \"git checkout branch10 && git branch -D branchA\" &&\n+\tgit checkout -b branchA branch10 &&\n+\tgit merge -s ours -m \"merge 1 and 10 to make A\" branch1 &&\n+\tgit commit --allow-empty -m \"another\" &&\n+\n+\tgit show-branch --sparse >out &&\n+\tgrep \"merge 1 and 10 to make A\" out &&\n+\n+\tgit show-branch >out &&\n+\t! grep \"merge 1 and 10 to make A\" out &&\n+\n+\tgit show-branch --no-sparse >out &&\n+\t! grep \"merge 1 and 10 to make A\" out\n+'\n+\n test_expect_success 'setup show branch --list' '\n \tsed \"s/^> //\" >expect <<-\\EOF\n \t>   [branch1] branch1\n-- \n2.41.0-376-gcba07a324d\n\n"},{"id":"479643","messageId":"xmqqh6pzc15n.fsf@gitster.g","threadId":"60010","inReplyTo":"xmqq7cqwcet8.fsf@gitster.g","subject":"[PATCH] show-branch: reject --[no-](topo|date)-order","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-07-19T16:32:36Z","receivedAt":"2023-07-19T16:32:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"git show-branch --no-topo-order\" behaved exactly the same way as\n\"git show-branch --topo-order\" did, which was nonsense.  This was\nbecause we choose between topo- and date- by setting a variable to\neither REV_SORT_IN_GRAPH_ORDER or REV_SORT_BY_COMMIT_DATE with\nOPT_SET_INT() and REV_SORT_IN_GRAPH_ORDER happens to be 0.  The\nOPT_SET_INTO() macro assigns 0 to the target variable in respose to\nthe negated form of its option.\n\n\"--no-date-order\" by luck behaves identically to \"--topo-order\"\nexactly for the same reason, and it sort-of makes sense right now,\nbut the \"sort-of makes sense\" will quickly break down once we add a\nthird way to sort.  Not-A may be B when there are only two choices\nbetween A and B, but once your choices become among A, B, and C,\nnot-A does not mean B.\n\nJust mark these two ordering options to reject negation, and add a\ntest, which was missing.  \"git show-branch --no-reflog\" is also\nunnegatable, so throw in a test for that while we are at it.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n * Comes on top of the \"--no-sparse should mean dense\" patch.\n\n builtin/show-branch.c  | 14 +++++++-------\n t/t3202-show-branch.sh |  9 +++++++++\n 2 files changed, 16 insertions(+), 7 deletions(-)\n\ndiff --git c/builtin/show-branch.c w/builtin/show-branch.c\nindex 67f3ef0639..b574afb7c3 100644\n--- c/builtin/show-branch.c\n+++ w/builtin/show-branch.c\n@@ -667,17 +667,17 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)\n \t\t\t N_(\"show possible merge bases\")),\n \t\tOPT_BOOL(0, \"independent\", &independent,\n \t\t\t    N_(\"show refs unreachable from any other ref\")),\n-\t\tOPT_SET_INT(0, \"topo-order\", &sort_order,\n-\t\t\t    N_(\"show commits in topological order\"),\n-\t\t\t    REV_SORT_IN_GRAPH_ORDER),\n+\t\tOPT_SET_INT_F(0, \"topo-order\", &sort_order,\n+\t\t\t      N_(\"show commits in topological order\"),\n+\t\t\t      REV_SORT_IN_GRAPH_ORDER, PARSE_OPT_NONEG),\n \t\tOPT_BOOL(0, \"topics\", &topics,\n \t\t\t N_(\"show only commits not on the first branch\")),\n \t\tOPT_SET_INT(0, \"sparse\", &sparse,\n \t\t\t    N_(\"show merges reachable from only one tip\"), 1),\n-\t\tOPT_SET_INT(0, \"date-order\", &sort_order,\n-\t\t\t    N_(\"topologically sort, maintaining date order \"\n-\t\t\t       \"where possible\"),\n-\t\t\t    REV_SORT_BY_COMMIT_DATE),\n+\t\tOPT_SET_INT_F(0, \"date-order\", &sort_order,\n+\t\t\t      N_(\"topologically sort, maintaining date order \"\n+\t\t\t\t \"where possible\"),\n+\t\t\t      REV_SORT_BY_COMMIT_DATE, PARSE_OPT_NONEG),\n \t\tOPT_CALLBACK_F('g', \"reflog\", &reflog_base, N_(\"<n>[,<base>]\"),\n \t\t\t    N_(\"show <n> most recent ref-log entries starting at \"\n \t\t\t       \"base\"),\ndiff --git c/t/t3202-show-branch.sh w/t/t3202-show-branch.sh\nindex ca7c44f0f7..b17f388f56 100755\n--- c/t/t3202-show-branch.sh\n+++ w/t/t3202-show-branch.sh\n@@ -213,6 +213,15 @@ done <<\\EOF\n --reflog --current\n EOF\n \n+# unnegatable options\n+for opt in topo-order date-order reflog\n+do\n+\ttest_expect_success \"show-branch --no-$opt (should fail)\" '\n+\t\ttest_must_fail git show-branch --no-$opt 2>err &&\n+\t\tgrep \"unknown option .no-$opt.\" err\n+\t'\n+done\n+\n test_expect_success 'error descriptions on non-existent branch' '\n \tcat >expect <<-EOF &&\n \terror: No branch named '\\''non-existent'\\'.'\n\n\n"}]}