Re: [PATCH] builtin-tag: fix fallouts from recent parsopt restriction.

3 messages, 2 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH] builtin-tag: fix fallouts from recent parsopt restriction.

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:43:59 

Pierre Habouzit [off-list ref] writes:

  Okay this is kind of disgusting, and I'm absolutely not pleased with
it (I mean I'm not pleased that parse_opt forces us to write things like
that). This hack allows:

  git tag -l -n10 <pattern>

and will then attach the <pattern> to the '-l' switch,...
Heh, it turns out that we were both stupid and blind.

Look at contrib/examples/git-tag.sh again.

The original addition of "-n <count>" was suboptimal and did not allow
"git tag -l -n10 <glob>", but I would actually call that a bug of the -n
<count> implementation (it wanted to affect working of other option, so
at that point it should have restructured the loop and made parsing of
options and carrying out of actions separate steps).

The -l option tells "git tag" to work in "list tags" mode, and in that
mode, the command itself takes zero or one (we could have taken more but
we didn't) glob to limit the listing.  The <glob> is not even an option
argument to option -l, but the arguments to "git tag" command itself.

So "git-tag -l" was a bad example of an option that takes optional
option-argument, and you can see that the conversion to parse_options()
done in 396865859918e9c7bf8ce74aae137c57da134610 (Make builtin-tag.c use
parse_options.) broke it.

IOW, the fixup I posted was not a workaround but happens to be a bugfix
to the above commit that did parse_options() conversion.

Re: [PATCH] builtin-tag: fix fallouts from recent parsopt restriction.

From: Pierre Habouzit <hidden>
Date: 2016-06-15 22:43:59 

On Mon, Dec 17, 2007 at 11:13:14AM +0000, Junio C Hamano wrote:
Pierre Habouzit [off-list ref] writes:
quoted
  Okay this is kind of disgusting, and I'm absolutely not pleased with
it (I mean I'm not pleased that parse_opt forces us to write things like
that). This hack allows:

  git tag -l -n10 <pattern>

and will then attach the <pattern> to the '-l' switch,...
Heh, it turns out that we were both stupid and blind.
  [...]

  indeed, but then this happens to be a better patch than yours IMHO:


From 5a3cdd255f17c7d7bc9245881f0d50146413113f Mon Sep 17 00:00:00 2001
From: Pierre Habouzit <redacted>
Date: Mon, 17 Dec 2007 12:54:55 +0100
Subject: [PATCH] git-tag: fix -l switch handling regression.

Signed-off-by: Pierre Habouzit <redacted>
---
 builtin-tag.c |   12 +++++-------
 1 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/builtin-tag.c b/builtin-tag.c
index 274901a..219633d 100644
--- a/builtin-tag.c
+++ b/builtin-tag.c
@@ -16,7 +16,7 @@
 static const char * const git_tag_usage[] = {
 	"git-tag [-a|-s|-u <key-id>] [-f] [-m <msg>|-F <file>] <tagname> [<head>]",
 	"git-tag -d <tagname>...",
-	"git-tag [-n [<num>]] -l [<pattern>]",
+	"git-tag -l [-n [<num>]] [<pattern>]",
 	"git-tag -v <tagname>...",
 	NULL
 };
@@ -370,13 +370,11 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 	struct ref_lock *lock;
 
 	int annotate = 0, sign = 0, force = 0, lines = 0,
-					delete = 0, verify = 0;
-	char *list = NULL, *msgfile = NULL, *keyid = NULL;
-	const char *no_pattern = "NO_PATTERN";
+		list = 0, delete = 0, verify = 0;
+	char *msgfile = NULL, *keyid = NULL;
 	struct msg_arg msg = { 0, STRBUF_INIT };
 	struct option options[] = {
-		{ OPTION_STRING, 'l', NULL, &list, "pattern", "list tag names",
-			PARSE_OPT_OPTARG, NULL, (intptr_t) no_pattern },
+		OPT_INTEGER('l', NULL, &list, "list tag names"),
 		{ OPTION_INTEGER, 'n', NULL, &lines, NULL,
 				"print n lines of each tag message",
 				PARSE_OPT_OPTARG, NULL, 1 },
@@ -408,7 +406,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 		annotate = 1;
 
 	if (list)
-		return list_tags(list == no_pattern ? NULL : list, lines);
+		return list_tags(argv[0], lines);
 	if (delete)
 		return for_each_tag_name(argv, delete_tag);
 	if (verify)
-- 
debian.1.5.3.7.1-dirty

Re: [PATCH] builtin-tag: fix fallouts from recent parsopt restriction.

From: Pierre Habouzit <hidden>
Date: 2016-06-15 22:43:59 

On Mon, Dec 17, 2007 at 11:56:48AM +0000, Pierre Habouzit wrote:
quoted hunk
On Mon, Dec 17, 2007 at 11:13:14AM +0000, Junio C Hamano wrote:
quoted
Pierre Habouzit [off-list ref] writes:
quoted
  Okay this is kind of disgusting, and I'm absolutely not pleased with
it (I mean I'm not pleased that parse_opt forces us to write things like
that). This hack allows:

  git tag -l -n10 <pattern>

and will then attach the <pattern> to the '-l' switch,...
Heh, it turns out that we were both stupid and blind.
  [...]

  indeed, but then this happens to be a better patch than yours IMHO:


From 5a3cdd255f17c7d7bc9245881f0d50146413113f Mon Sep 17 00:00:00 2001
From: Pierre Habouzit <redacted>
Date: Mon, 17 Dec 2007 12:54:55 +0100
Subject: [PATCH] git-tag: fix -l switch handling regression.

Signed-off-by: Pierre Habouzit <redacted>
---
 builtin-tag.c |   12 +++++-------
 1 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/builtin-tag.c b/builtin-tag.c
index 274901a..219633d 100644
--- a/builtin-tag.c
+++ b/builtin-tag.c
@@ -16,7 +16,7 @@
 static const char * const git_tag_usage[] = {
 	"git-tag [-a|-s|-u <key-id>] [-f] [-m <msg>|-F <file>] <tagname> [<head>]",
 	"git-tag -d <tagname>...",
-	"git-tag [-n [<num>]] -l [<pattern>]",
+	"git-tag -l [-n [<num>]] [<pattern>]",
 	"git-tag -v <tagname>...",
 	NULL
 };
@@ -370,13 +370,11 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 	struct ref_lock *lock;
 
 	int annotate = 0, sign = 0, force = 0, lines = 0,
-					delete = 0, verify = 0;
-	char *list = NULL, *msgfile = NULL, *keyid = NULL;
-	const char *no_pattern = "NO_PATTERN";
+		list = 0, delete = 0, verify = 0;
+	char *msgfile = NULL, *keyid = NULL;
 	struct msg_arg msg = { 0, STRBUF_INIT };
 	struct option options[] = {
-		{ OPTION_STRING, 'l', NULL, &list, "pattern", "list tag names",
-			PARSE_OPT_OPTARG, NULL, (intptr_t) no_pattern },
+		OPT_INTEGER('l', NULL, &list, "list tag names"),
                ^^^^^^^^^^^
      that should obviously be OPT_BOOLEAN
quoted hunk
 		{ OPTION_INTEGER, 'n', NULL, &lines, NULL,
 				"print n lines of each tag message",
 				PARSE_OPT_OPTARG, NULL, 1 },
@@ -408,7 +406,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 		annotate = 1;
 
 	if (list)
-		return list_tags(list == no_pattern ? NULL : list, lines);
+		return list_tags(argv[0], lines);
 	if (delete)
 		return for_each_tag_name(argv, delete_tag);
 	if (verify)
-- 
debian.1.5.3.7.1-dirty

-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help