git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 2/8] tag: Refactor the options handling code to be less bizarro

From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date
Mar 18, 2017, 10:32 UTC
Message-ID
<20170318103256.27141-3-avarab@gmail.com>
In-Reply-To
<20170318103256.27141-1-avarab@gmail.com>

Refactor the options handling code so that "cmdmode = 'l'" isn't set on the likes of "tag -a". This change introduces no functional changes, but makes subsequent patches easier to reason about, as both "annotate = 1" "cmdmode = 'l'" won't be unexpectedly set if "tag" is merely invoked as "tag -a", "tag -s" etc.

Before this the cmdmode variable would be set to 'l' indicating that "tag" was operating in list mode, this was then used a couple of lines later by e.g. the codepath for "tag -a" where the command would only bail out with usage_with_options() if the likes of "-a" were set, *and* some cmdmode (i.e. 'l' in this case) had been set.

This behavior was an emergent property of other earlier changes, starting with commit 6fa8342b12 (tag: Check that options are only allowed in the appropriate mode, 2008-11-05), to its present form in commit e6b722db09 ("tag: use OPT_CMDMODE", 2013-07-30).

Change the test suite to more exhaustively assert that already existing behavior related to this option parsing is kept.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 builtin/tag.c  |  4 ++--
 t/t7004-tag.sh | 13 +++++++++++++
 2 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/builtin/tag.c b/builtin/tag.c
index ad29be6923..0bba3fd070 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -454,10 +454,10 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 	}
 	create_tag_object = (opt.sign || annotate || msg.given || msgfile);
 
-	if (argc == 0 && !cmdmode)
+	if (argc == 0 && !cmdmode && !create_tag_object)
 		cmdmode = 'l';
 
-	if ((create_tag_object || force) && (cmdmode != 0))
+	if ((create_tag_object || force) && (cmdmode || (!cmdmode && !argc)))
 		usage_with_options(git_tag_usage, options);
 
 	finalize_colopts(&colopts, -1);
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 876ccfc830..74fc82a3c0 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -1455,6 +1455,19 @@ test_expect_success 'checking that initial commit is in all tags' "
 
 test_expect_success 'mixing incompatibles modes and options is forbidden' '
 	test_must_fail git tag -a &&
+	test_must_fail git tag -a -l &&
+	test_must_fail git tag -s &&
+	test_must_fail git tag -s -l &&
+	test_must_fail git tag -m &&
+	test_must_fail git tag -m -l &&
+	test_must_fail git tag -m "hlagh" &&
+	test_must_fail git tag -m "hlagh" -l &&
+	test_must_fail git tag -F &&
+	test_must_fail git tag -F -l &&
+	test_must_fail git tag -f &&
+	test_must_fail git tag -f -l &&
+	test_must_fail git tag -a -s -m -F &&
+	test_must_fail git tag -a -s -m -F -l &&
 	test_must_fail git tag -l -v &&
 	test_must_fail git tag -n 100 &&
 	test_must_fail git tag -l -m msg &&
-- 
2.11.0
Previous: Jeff KingNext: Junio C Hamano
Message 15 of 41 in “Various changes to the "tag" command”
  1. 0/8 Various changes to the "tag" commandÆvar Arnfjörð Bjarmason, Mar 18, 2017
  2. 1/8 tag: Remove a TODO item from the test suiteÆvar Arnfjörð Bjarmason, Mar 18, 2017
  3. Junio C HamanoMar 18, 2017
  4. 0/2 doc/SubmittingPatches: A couple of minor improvementsÆvar Arnfjörð Bjarmason, Mar 18, 2017
  5. 1/2 doc/SubmittingPatches: clarify the casing convention for "area: change..."Ævar Arnfjörð Bjarmason, Mar 18, 2017
  6. Junio C HamanoMar 18, 2017
  7. Ævar Arnfjörð BjarmasonMar 18, 2017
  8. Junio C HamanoMar 18, 2017
  9. 2/2 doc/SubmittingPatches: show how to get a CLI commit summaryÆvar Arnfjörð Bjarmason, Mar 18, 2017
  10. Junio C HamanoMar 18, 2017
  11. Jakub NarębskiMar 19, 2017
  12. Ævar Arnfjörð BjarmasonMar 19, 2017
  13. 5/8 tag: Implicitly supply --list given the -n optionÆvar Arnfjörð Bjarmason, Mar 18, 2017
  14. Jeff KingMar 20, 2017
  15. 2/8 tag: Refactor the options handling code to be less bizarroÆvar Arnfjörð Bjarmason, Mar 18, 2017
  16. Junio C HamanoMar 18, 2017
  17. Ævar Arnfjörð BjarmasonMar 18, 2017
  18. Junio C HamanoMar 18, 2017
  19. Ævar Arnfjörð BjarmasonMar 18, 2017
  20. 7/8 tag: Add tests for --with and --withoutÆvar Arnfjörð Bjarmason, Mar 18, 2017
  21. 8/8 tag: Change --point-at to default to HEADÆvar Arnfjörð Bjarmason, Mar 18, 2017
  22. Junio C HamanoMar 18, 2017
  23. Ævar Arnfjörð BjarmasonMar 18, 2017
  24. 3/8 tag: Change misleading --list <pattern> documentationÆvar Arnfjörð Bjarmason, Mar 18, 2017
  25. Junio C HamanoMar 18, 2017
  26. Ævar Arnfjörð BjarmasonMar 18, 2017
  27. Jeff KingMar 20, 2017
  28. Junio C HamanoMar 20, 2017
  29. Junio C HamanoMar 20, 2017
  30. Ævar Arnfjörð BjarmasonMar 20, 2017
  31. Jeff KingMar 20, 2017
  32. 4/8 tag: Implicitly supply --list given another list-like optionÆvar Arnfjörð Bjarmason, Mar 18, 2017
  33. Jeff KingMar 20, 2017
  34. Ævar Arnfjörð BjarmasonMar 20, 2017
  35. 6/8 ref-filter: Add --no-contains option to tag/branch/for-each-refÆvar Arnfjörð Bjarmason, Mar 18, 2017
  36. Jeff KingMar 20, 2017
  37. Ævar Arnfjörð BjarmasonMar 20, 2017
  38. Jeff KingMar 20, 2017
  39. Ævar Arnfjörð BjarmasonMar 20, 2017
  40. Jeff KingMar 20, 2017
  41. Junio C HamanoMar 20, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.