threads / patch / 10766

patchMake builtin-tag.c use parse_options.

Subject: [PATCH] Make builtin-tag.c use parse_options.

## tl;dr

11 messages between Nov 9, 2007 and Nov 12, 2007. Diffs are folded; open one to read it.

replies: 10people: 6as markdown or json

Carlos Rica· Nov 9, 2007, 13:42 UTC · lore

Also, this removes those tests ensuring that repeated -m options don't allocate memory more than once, because now this is done after parsing options, using the last one when more are given. The same for -F.

Signed-off-by: Carlos Rica <jasampler@gmail.com>
---
    Applied to "next".
    Comments welcomed.
 builtin-tag.c  |  141 ++++++++++++++++++++++++--------------------------------
 t/t7004-tag.sh |    8 +---
 2 files changed, 61 insertions(+), 88 deletions(-)
Show changes to 2 files +61 −88

builtin-tag.c, t/t7004-tag.sh

diff --git a/builtin-tag.c b/builtin-tag.c
index 66e5a58..5af1950 100644
--- a/builtin-tag.c
+++ b/builtin-tag.c
@@ -11,9 +11,15 @@
 #include "refs.h"
 #include "tag.h"
 #include "run-command.h"
-
-static const char builtin_tag_usage[] =
-  "git-tag [-n [<num>]] -l [<pattern>] | [-a | -s | -u <key-id>] [-f | -d | -v] [-m <msg> | -F <file>] <tagname> [<head>]";
+#include "parse-options.h"
+
+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 -v <tagname>...",
+	NULL
+};

 static char signingkey[1000];

@@ -308,101 +314,74 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 {
 	struct strbuf buf;
 	unsigned char object[20], prev[20];
-	int annotate = 0, sign = 0, force = 0, lines = 0, message = 0;
 	char ref[PATH_MAX];
 	const char *object_ref, *tag;
-	int i;
 	struct ref_lock *lock;

+	int annotate = 0, sign = 0, force = 0, lines = 0,
+					delete = 0, verify = 0;
+	char *list = NULL, *msg = NULL, *msgfile = NULL, *keyid = NULL;
+	const char *no_pattern = "NO_PATTERN";
+	struct option options[] = {
+		{ OPTION_STRING, 'l', NULL, &list, "pattern", "list tag names",
+			PARSE_OPT_OPTARG, NULL, (intptr_t) no_pattern },
+		{ OPTION_INTEGER, 'n', NULL, &lines, NULL,
+				"print n lines of each tag message",
+				PARSE_OPT_OPTARG, NULL, 1 },
+		OPT_BOOLEAN('d', NULL, &delete, "delete tags"),
+		OPT_BOOLEAN('v', NULL, &verify, "verify tags"),
+
+		OPT_GROUP("Tag creation options"),
+		OPT_BOOLEAN('a', NULL, &annotate,
+					"annotated tag, needs a message"),
+		OPT_STRING('m', NULL, &msg, "msg", "message for the tag"),
+		OPT_STRING('F', NULL, &msgfile, "file", "message in a file"),
+		OPT_BOOLEAN('s', NULL, &sign, "annotated and GPG-signed tag"),
+		OPT_STRING('u', NULL, &keyid, "key-id",
+					"use another key to sign the tag"),
+		OPT_BOOLEAN('f', NULL, &force, "replace the tag if exists"),
+		OPT_END()
+	};
+
 	git_config(git_tag_config);
-	strbuf_init(&buf, 0);

-	for (i = 1; i < argc; i++) {
-		const char *arg = argv[i];
+	argc = parse_options(argc, argv, options, git_tag_usage, 0);

-		if (arg[0] != '-')
-			break;
-		if (!strcmp(arg, "-a")) {
-			annotate = 1;
-			continue;
-		}
-		if (!strcmp(arg, "-s")) {
-			annotate = 1;
-			sign = 1;
-			continue;
-		}
-		if (!strcmp(arg, "-f")) {
-			force = 1;
-			continue;
-		}
-		if (!strcmp(arg, "-n")) {
-			if (i + 1 == argc || *argv[i + 1] == '-')
-				/* no argument */
-				lines = 1;
-			else
-				lines = isdigit(*argv[++i]) ?
-					atoi(argv[i]) : 1;
-			continue;
-		}
-		if (!strcmp(arg, "-m")) {
-			annotate = 1;
-			i++;
-			if (i == argc)
-				die("option -m needs an argument.");
-			if (message)
-				die("only one -F or -m option is allowed.");
-			strbuf_addstr(&buf, argv[i]);
-			message = 1;
-			continue;
-		}
-		if (!strcmp(arg, "-F")) {
-			annotate = 1;
-			i++;
-			if (i == argc)
-				die("option -F needs an argument.");
-			if (message)
-				die("only one -F or -m option is allowed.");
-
-			if (!strcmp(argv[i], "-")) {
+	if (list)
+		return list_tags(list == no_pattern ? NULL : list, lines);
+	if (delete)
+		return for_each_tag_name(argv, delete_tag);
+	if (verify)
+		return for_each_tag_name(argv, verify_tag);
+
+	strbuf_init(&buf, 0);
+	if (msg || msgfile) {
+		if (msg && msgfile)
+			die("only one -F or -m option is allowed.");
+		annotate = 1;
+		if (msg)
+			strbuf_addstr(&buf, msg);
+		else {
+			if (!strcmp(msgfile, "-")) {
 				if (strbuf_read(&buf, 0, 1024) < 0)
-					die("cannot read %s", argv[i]);
+					die("cannot read %s", msgfile);
 			} else {
-				if (strbuf_read_file(&buf, argv[i], 1024) < 0)
+				if (strbuf_read_file(&buf, msgfile, 1024) < 0)
 					die("could not open or read '%s': %s",
-						argv[i], strerror(errno));
+						msgfile, strerror(errno));
 			}
-			message = 1;
-			continue;
-		}
-		if (!strcmp(arg, "-u")) {
-			annotate = 1;
-			sign = 1;
-			i++;
-			if (i == argc)
-				die("option -u needs an argument.");
-			if (strlcpy(signingkey, argv[i], sizeof(signingkey))
-							>= sizeof(signingkey))
-				die("argument to option -u too long");
-			continue;
 		}
-		if (!strcmp(arg, "-l"))
-			return list_tags(argv[i + 1], lines);
-		if (!strcmp(arg, "-d"))
-			return for_each_tag_name(argv + i + 1, delete_tag);
-		if (!strcmp(arg, "-v"))
-			return for_each_tag_name(argv + i + 1, verify_tag);
-		usage(builtin_tag_usage);
 	}

-	if (i == argc) {
+	if (argc == 0) {
 		if (annotate)
-			usage(builtin_tag_usage);
+			usage_with_options(git_tag_usage, options);
 		return list_tags(NULL, lines);
 	}
-	tag = argv[i++];
+	tag = argv[0];

-	object_ref = i < argc ? argv[i] : "HEAD";
-	if (i + 1 < argc)
+	object_ref = argc == 2 ? argv[1] : "HEAD";
+	if (argc > 2)
 		die("too many params");

 	if (get_sha1(object_ref, object))
@@ -419,7 +398,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 		die("tag '%s' already exists", tag);

 	if (annotate)
-		create_tag(object, tag, &buf, message, sign, object);
+		create_tag(object, tag, &buf, msg || msgfile, sign, object);

 	lock = lock_any_ref_for_update(ref, prev, 0);
 	if (!lock)
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 0d07bc3..4b09d28 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -339,20 +339,14 @@ test_expect_success \
 '

 test_expect_success \
-	'trying to create tags giving many -m or -F options should fail' '
+	'trying to create tags giving both -m or -F options should fail' '
 	echo "message file 1" >msgfile1 &&
 	echo "message file 2" >msgfile2 &&
 	! tag_exists msgtag &&
-	! git-tag -m "message 1" -m "message 2" msgtag &&
-	! tag_exists msgtag &&
-	! git-tag -F msgfile1 -F msgfile2 msgtag &&
-	! tag_exists msgtag &&
 	! git-tag -m "message 1" -F msgfile1 msgtag &&
 	! tag_exists msgtag &&
 	! git-tag -F msgfile1 -m "message 1" msgtag &&
 	! tag_exists msgtag &&
-	! git-tag -F msgfile1 -m "message 1" -F msgfile2 msgtag &&
-	! tag_exists msgtag &&
 	! git-tag -m "message 1" -F msgfile1 -m "message 2" msgtag &&
 	! tag_exists msgtag
 '
-- 
1.5.3.4
Jakub Narebski· Nov 9, 2007, 13:57 UTC · re: Carlos Rica · lore

Re: [PATCH] Make builtin-tag.c use parse_options.

Carlos Rica wrote:
> +     struct option options[] = {
> +             { OPTION_STRING, 'l', NULL, &list, "pattern", "list tag names",
> +                     PARSE_OPT_OPTARG, NULL, (intptr_t) no_pattern },
> +             OPT_STRING('F', NULL, &msgfile, "file", "message in a file"),
Does it matter that you use OPTION_STRING here and OPT_STRING macro there?
-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git
Johannes Schindelin· Nov 9, 2007, 14:31 UTC · re: Jakub Narebski · lore

Re: [PATCH] Make builtin-tag.c use parse_options.

Hi,
[re Cc:ing jasam]
On Fri, 9 Nov 2007, Jakub Narebski wrote:
Show 9 quoted lines
> Carlos Rica wrote:
> 
> > +     struct option options[] = {
> > +             { OPTION_STRING, 'l', NULL, &list, "pattern", "list tag names",
> > +                     PARSE_OPT_OPTARG, NULL, (intptr_t) no_pattern },
> 
> > +             OPT_STRING('F', NULL, &msgfile, "file", "message in a file"),
> 
> Does it matter that you use OPTION_STRING here and OPT_STRING macro there?

I guess it is because of the PARSE_OPT_OPTARG thing, together with no_pattern. We need to know if -l was specified, even if no argument was passed in.

Hth, Dscho

Junio C Hamano· Nov 10, 2007, 06:07 UTC · re: Carlos Rica · lore

Re: [PATCH] Make builtin-tag.c use parse_options.

Carlos Rica <jasampler@gmail.com> writes:
> Also, this removes those tests ensuring that repeated
> -m options don't allocate memory more than once, because now
> this is done after parsing options, using the last one
> when more are given. The same for -F.

The reason for this change is...? Is this because it is cumbersome to detect and refuse multiple -m options using the parseopt API? If so, the API may be what needs to be fixed. Taking the last one and discarding earlier ones feels to me an arbitrary choice.

While I freely admit that I do not particularly find the "One -m introduces one new line, concatenated to form the final paragraph" handling of multiple -m options done by git-commit nice nor useful, I suspect that it would make more sense to make git-tag and git-commit handle multiple -m option consistently, if you are going to change the existing semantics. Since some people really seem to like multiple -m handling of git-commit, the avenue of the least resistance for better consistency would be to accept and concatenate (with LF in between) multiple -m options.

With multiple -F, I think erroring out would be the sensible thing to do, but some people might prefer concatenation. I do not care either way as long as commit and tag behave consistently.

Junio C Hamano· Nov 10, 2007, 09:26 UTC · re: Junio C Hamano · lore

Re: [PATCH] Make builtin-tag.c use parse_options.

Junio C Hamano <gitster@pobox.com> writes:
Show 15 quoted lines
> While I freely admit that I do not particularly find the "One -m
> introduces one new line, concatenated to form the final
> paragraph" handling of multiple -m options done by git-commit
> nice nor useful, I suspect that it would make more sense to make
> git-tag and git-commit handle multiple -m option consistently,
> if you are going to change the existing semantics.  Since some
> people really seem to like multiple -m handling of git-commit,
> the avenue of the least resistance for better consistency would
> be to accept and concatenate (with LF in between) multiple -m
> options.
>
> With multiple -F, I think erroring out would be the sensible
> thing to do, but some people might prefer concatenation.  I do
> not care either way as long as commit and tag behave
> consistently.
Alas, this exposes a regression in kh/commit series.
 t/t7501-commit.sh |   23 +++++++++++++++++++++++
 1 files changed, 23 insertions(+), 0 deletions(-)
Show changes to t/t7501-commit.sh +23 −0
diff --git a/t/t7501-commit.sh b/t/t7501-commit.sh
index 1b444d4..bf5dd86 100644
--- a/t/t7501-commit.sh
+++ b/t/t7501-commit.sh
@@ -178,4 +178,27 @@ test_expect_success 'amend commit to fix author' '
 	diff expected current
 
 '
+
+test_expect_success 'sign off' '
+
+	>positive &&
+	git add positive &&
+	git commit -s -m "thank you" &&
+	actual=$(git cat-file commit HEAD | sed -ne "s/Signed-off-by: //p") &&
+	expected=$(git var GIT_COMMITTER_IDENT | sed -e "s/>.*/>/") &&
+	test "z$actual" = "z$expected"
+
+'
+
+test_expect_success 'multiple -m' '
+
+	>negative &&
+	git add negative &&
+	git commit -m "one" -m "two" -m "three" &&
+	actual=$(git cat-file commit HEAD | sed -e "1,/^\$/d") &&
+	expected=$(echo one; echo; echo two; echo; echo three) &&
+	test "z$actual" = "z$expected"
+
+'
+
 test_done
Junio C Hamano· Nov 10, 2007, 09:41 UTC · re: Junio C Hamano · lore

Re: [PATCH] Make builtin-tag.c use parse_options.

This is an updated patch to the test script...
 t/t7501-commit.sh |   69 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 69 insertions(+), 0 deletions(-)
Show changes to t/t7501-commit.sh +69 −0
diff --git a/t/t7501-commit.sh b/t/t7501-commit.sh
index b151b51..4dc35bd 100644
--- a/t/t7501-commit.sh
+++ b/t/t7501-commit.sh
@@ -163,4 +163,73 @@ test_expect_success 'partial commit that involves removal (3)' '
 
 '
 
+author="The Real Author <someguy@his.email.org>"
+test_expect_success 'amend commit to fix author' '
+
+	oldtick=$GIT_AUTHOR_DATE &&
+	test_tick &&
+	git reset --hard &&
+	git cat-file -p HEAD |
+	sed -e "s/author.*/author $author $oldtick/" \
+		-e "s/^\(committer.*> \).*$/\1$GIT_COMMITTER_DATE/" > \
+		expected &&
+	git commit --amend --author="$author" &&
+	git cat-file -p HEAD > current &&
+	diff expected current
+
+'
+
+test_expect_success 'sign off (1)' '
+
+	echo 1 >positive &&
+	git add positive &&
+	git commit -s -m "thank you" &&
+	git cat-file commit HEAD | sed -e "1,/^\$/d" >actual &&
+	(
+		echo thank you
+		echo
+		git var GIT_COMMITTER_IDENT |
+		sed -e "s/>.*/>/" -e "s/^/Signed-off-by: /"
+	) >expected &&
+	diff -u expected actual
+
+'
+
+test_expect_success 'sign off (2)' '
+
+	echo 2 >positive &&
+	git add positive &&
+	existing="Signed-off-by: Watch This <watchthis@example.com>" &&
+	git commit -s -m "thank you
+
+$existing" &&
+	git cat-file commit HEAD | sed -e "1,/^\$/d" >actual &&
+	(
+		echo thank you
+		echo
+		echo $existing
+		git var GIT_COMMITTER_IDENT |
+		sed -e "s/>.*/>/" -e "s/^/Signed-off-by: /"
+	) >expected &&
+	diff -u expected actual
+
+'
+
+test_expect_success 'multiple -m' '
+
+	>negative &&
+	git add negative &&
+	git commit -m "one" -m "two" -m "three" &&
+	git cat-file commit HEAD | sed -e "1,/^\$/d" >actual &&
+	(
+		echo one
+		echo
+		echo two
+		echo
+		echo three
+	) >expected &&
+	diff -u expected actual
+
+'
+
 test_done
Carlos Rica· Nov 10, 2007, 12:25 UTC · re: Junio C Hamano · lore

Re: [PATCH] Make builtin-tag.c use parse_options.

2007/11/10, Junio C Hamano <gitster@pobox.com>:
Show 28 quoted lines
> Carlos Rica <jasampler@gmail.com> writes:
>
> > Also, this removes those tests ensuring that repeated
> > -m options don't allocate memory more than once, because now
> > this is done after parsing options, using the last one
> > when more are given. The same for -F.
>
> The reason for this change is...?  Is this because it is
> cumbersome to detect and refuse multiple -m options using the
> parseopt API?  If so, the API may be what needs to be fixed.
> Taking the last one and discarding earlier ones feels to me an
> arbitrary choice.
>
> While I freely admit that I do not particularly find the "One -m
> introduces one new line, concatenated to form the final
> paragraph" handling of multiple -m options done by git-commit
> nice nor useful, I suspect that it would make more sense to make
> git-tag and git-commit handle multiple -m option consistently,
> if you are going to change the existing semantics.  Since some
> people really seem to like multiple -m handling of git-commit,
> the avenue of the least resistance for better consistency would
> be to accept and concatenate (with LF in between) multiple -m
> options.
>
> With multiple -F, I think erroring out would be the sensible
> thing to do, but some people might prefer concatenation.  I do
> not care either way as long as commit and tag behave
> consistently.

A solution not needing memory allocation into the option parser could be setting a callback running over the repeated option arguments, passing them to the function one per each call. Then, the user will be able to decide if he wants the arguments concatenated or only need one of them and prefers erroring out.

Is this already possible with the current parser or the callback mode only calls using the last option?

Pierre Habouzit· Nov 10, 2007, 13:13 UTC · re: Carlos Rica · lore

Re: [PATCH] Make builtin-tag.c use parse_options.

On Sat, Nov 10, 2007 at 12:25:44PM +0000, Carlos Rica wrote:
> 2007/11/10, Junio C Hamano <gitster@pobox.com>:
> > Carlos Rica <jasampler@gmail.com> writes:
Show 8 quoted lines
> A solution not needing memory allocation into the option parser
> could be setting a callback running over the repeated option
> arguments, passing them to the function one per each call.
> Then, the user will be able to decide if he wants the arguments
> concatenated or only need one of them and prefers erroring out.
> 
> Is this already possible with the current parser or the callback
> mode only calls using the last option?
  Everything is possible, you just have to code it. With a callback
you have in the struct option two places to store "things". The void*
value pointer and the intptr_t defval. _Usually_ the void* is the
pointer to the data that will be _written_ and the defval the data that
will be put into the void* under some circumstances (e.g. when your
option is negated).
  For Your case I'd go with some kind of string list pointed into the
void * value, defval has no or little use. You don't really care about
allocating memory in the option parser, I mean, option parsing is done
once at the initialization phase. It's not evil. In pseudo-C here is how
I would write the callback:
int parse_opt_stringlist(const struct option *opt, const char *arg, int unset)
{
    string_list **l = opt->value;
    string_list_elem *e;
    if (unset) { /* negationg option clears the list */
	while (*l) {
	    string_list_elem_free(string_list_pop(l));
	}
	return 0;
    }
    e = string_list_elem_new();
    e->data = arg;
    string_list_push(l, e);
    return 0;
}
  And you're done, you can do what you want with that list from the caller.
There probably is such a structure in git, if not, it can probably be hacked
in a few lines.
  Remember, callbacks give you _full_ control on what you can do in the option
parser, and if you're not happy with Turing complete expressivity, there isn't
anything I can do for you :P Note that if you do write such a generic
callback, it belongs to parse-options.[hc].
-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org
Carlos Rica· Nov 12, 2007, 13:09 UTC · re: Junio C Hamano · lore

Re: [PATCH] Make builtin-tag.c use parse_options.

2007/11/10, Junio C Hamano <gitster@pobox.com>:
Show 12 quoted lines
> Carlos Rica <jasampler@gmail.com> writes:
>
> > Also, this removes those tests ensuring that repeated
> > -m options don't allocate memory more than once, because now
> > this is done after parsing options, using the last one
> > when more are given. The same for -F.
>
> The reason for this change is...?  Is this because it is
> cumbersome to detect and refuse multiple -m options using the
> parseopt API?  If so, the API may be what needs to be fixed.
> Taking the last one and discarding earlier ones feels to me an
> arbitrary choice.

You can do many things with repeated options. Here in git-tag we considered two different ways to manage them: Concatenating values for the option and/or refusing more than one. I found that current option-parser can do both from the client using callbacks, as Pierre shows me, so I think it is the right way to do it.

Pierre, by default, I think that the parser should print an error when more than one option of the same type is given, in order to report it to the command-line user, but make this behaviour optional for the programmer. Specifically, I thought in this last option:

enum parse_opt_option_flags {
	PARSE_OPT_OPTARG  = 1,
	PARSE_OPT_NOARG   = 2,
	PARSE_OPT_ALLOWREP = 4
};
Show 15 quoted lines
> While I freely admit that I do not particularly find the "One -m
> introduces one new line, concatenated to form the final
> paragraph" handling of multiple -m options done by git-commit
> nice nor useful, I suspect that it would make more sense to make
> git-tag and git-commit handle multiple -m option consistently,
> if you are going to change the existing semantics.  Since some
> people really seem to like multiple -m handling of git-commit,
> the avenue of the least resistance for better consistency would
> be to accept and concatenate (with LF in between) multiple -m
> options.
>
> With multiple -F, I think erroring out would be the sensible
> thing to do, but some people might prefer concatenation.  I do
> not care either way as long as commit and tag behave
> consistently.

Then, Kristian, what are you willing to do in such case? It seems easier for me to concatenate of -m and -F options, even when both types are given. I don't know why "people" want multiple -m options, but I think that mixing -m and -F options could be interesting for them too. If someone know if this have been discussed and decided already, please give me the link.

Pierre Habouzit· Nov 12, 2007, 14:52 UTC · re: Carlos Rica · lore

Re: [PATCH] Make builtin-tag.c use parse_options.

On Mon, Nov 12, 2007 at 01:09:37PM +0000, Carlos Rica wrote:
Show 22 quoted lines
> 2007/11/10, Junio C Hamano <gitster@pobox.com>:
> > Carlos Rica <jasampler@gmail.com> writes:
> >
> > > Also, this removes those tests ensuring that repeated
> > > -m options don't allocate memory more than once, because now
> > > this is done after parsing options, using the last one
> > > when more are given. The same for -F.
> >
> > The reason for this change is...?  Is this because it is
> > cumbersome to detect and refuse multiple -m options using the
> > parseopt API?  If so, the API may be what needs to be fixed.
> > Taking the last one and discarding earlier ones feels to me an
> > arbitrary choice.
> 
> You can do many things with repeated options.
> Here in git-tag we considered two different ways to manage them:
> Concatenating values for the option and/or refusing more than one.
> I found that current option-parser can do both from the client
> using callbacks, as Pierre shows me, so I think it is the right way to do it.
> 
> Pierre, by default, I think that the parser should print an error
> when more than one option of the same type is given,
  I beg to differ. It makes sense for OPTION_STRING options, but not for
other. Though you cannot always detect that.
Also note that:
(1) repeating options was already silent in many git commands, so it's
    not really a regression ;
(2) for many commands it actually make sense to allow repeating (for
    _BOOLEAN e.g.). And I'd argue that for OPTION_BIT it also makes
    sense as well.
Show 9 quoted lines
> in order to report it to the command-line user, but make this
> behaviour optional for the programmer.  Specifically, I thought in
> this last option:
> 
> enum parse_opt_option_flags {
> 	PARSE_OPT_OPTARG   = 1,
> 	PARSE_OPT_NOARG    = 2,
> 	PARSE_OPT_ALLOWREP = 4
> };
  To do that you need to keep a list of the triggered commands to do
that, there is no way to achieve that reliably right now. As taking the
last one and discarding the other is the usual way for option parsers I
never saw this as a big issue.
-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org
Kristian Høgsberg· Nov 12, 2007, 19:48 UTC · re: Carlos Rica · lore

Re: [PATCH] Make builtin-tag.c use parse_options.

On Mon, 2007-11-12 at 14:09 +0100, Carlos Rica wrote:
> 2007/11/10, Junio C Hamano <gitster@pobox.com>:
> > Carlos Rica <jasampler@gmail.com> writes:
...
Show 6 quoted lines
> Then, Kristian, what are you willing to do in such case?
> It seems easier for me to concatenate of -m and -F options, even when
> both types are given. I don't know why "people" want multiple -m options,
> but I think that mixing -m and -F options could be interesting for them too.
> If someone know if this have been discussed and decided already,
> please give me the link.

I should be pretty easy to just append the contents of multiple fies, even inter-mingled with -m options. We just do a callback like Johannes just did for -m in builtin-commit.c for -F and append to the same strbuf. strbuf_read() already appends, so the callback could look something like:

static int opt_parse_F(const struct option *opt, const char *arg, int
unset)
{
        struct strbuf *buf = opt->value;
	if (!strcmp(arg, "-")) {
                if (isatty(0))
                        fprintf(stderr, "(reading log message from
standard input)\n");
                if (strbuf_read(&sb, 0, 0) < 0)
                        die("could not read log from standard input");
	} else {
                if (strbuf_read_file(&sb, logfile, 0) < 0)
                        die("could not read log file '%s': %s",
                            logfile, strerror(errno));
	}
}

Shouldn't be too hard :) Kristian

← back to recent threads