git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] [GSOC] interpret-trailer: easy parse trailer value
@ 2021-03-20 14:41 ZheNing Hu via GitGitGadget
  2021-03-20 14:41 ` [PATCH 1/2] [GSOC] commit: add --trailer option ZheNing Hu via GitGitGadget
                   ` (2 more replies)
  0 siblings, 3 replies; 23+ messages in thread
From: ZheNing Hu via GitGitGadget @ 2021-03-20 14:41 UTC (permalink / raw)
  To: git
  Cc: Bradley M. Kuhn, Junio C Hamano, Brandon Casey, Shourya Shukla,
	Christian Couder, Rafael Silva,
	Đoàn Trần Công Danh, Jeff King,
	Ævar Arnfjörð Bjarmason, ZheNing Hu

Hope this can help commit --trailer more useful!

Base on https://github.com/gitgitgadget/git/pull/901, The original patch
series was too long so now split it into two.

Helped by Junio and Jeff, Thanks.

ZheNing Hu (2):
  [GSOC] commit: add --trailer option
  [GSOC] interpret-trailer: easy parse trailer value

 Documentation/git-commit.txt             |  14 +-
 Documentation/git-interpret-trailers.txt |  23 ++
 builtin/commit.c                         |  55 ++--
 commit.c                                 |  34 +++
 commit.h                                 |   2 +
 t/t7502-commit-porcelain.sh              | 319 +++++++++++++++++++++++
 t/t7513-interpret-trailers.sh            |  23 ++
 trailer.c                                |  13 +-
 8 files changed, 448 insertions(+), 35 deletions(-)


base-commit: 13d7ab6b5d7929825b626f050b62a11241ea4945
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-911%2Fadlternative%2Ftrailer-easy-ident-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-911/adlternative/trailer-easy-ident-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/911
-- 
gitgitgadget

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH 1/2] [GSOC] commit: add --trailer option
  2021-03-20 14:41 [PATCH 0/2] [GSOC] interpret-trailer: easy parse trailer value ZheNing Hu via GitGitGadget
@ 2021-03-20 14:41 ` ZheNing Hu via GitGitGadget
  2021-03-20 14:41 ` [PATCH 2/2] [GSOC] interpret-trailer: easy parse trailer value ZheNing Hu via GitGitGadget
  2021-03-21  8:58 ` [PATCH v2 0/2] " ZheNing Hu via GitGitGadget
  2 siblings, 0 replies; 23+ messages in thread
From: ZheNing Hu via GitGitGadget @ 2021-03-20 14:41 UTC (permalink / raw)
  To: git
  Cc: Bradley M. Kuhn, Junio C Hamano, Brandon Casey, Shourya Shukla,
	Christian Couder, Rafael Silva,
	Đoàn Trần Công Danh, Jeff King,
	Ævar Arnfjörð Bjarmason, ZheNing Hu, ZheNing Hu

From: ZheNing Hu <adlternative@gmail.com>

Historically, Git has supported the 'Signed-off-by' commit trailer
using the '--signoff' and the '-s' option from the command line.
But users may need to provide other trailer information from the
command line such as "Helped-by", "Reported-by", "Mentored-by",

Now implement a new `--trailer <token>[(=|:)<value>]` option to pass
other trailers to `interpret-trailers` and insert them into commit
messages.

Signed-off-by: ZheNing Hu <adlternative@gmail.com>
---
 Documentation/git-commit.txt |  14 +-
 builtin/commit.c             |  22 +++
 t/t7502-commit-porcelain.sh  | 291 +++++++++++++++++++++++++++++++++++
 3 files changed, 326 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index 17150fa7eabe..3fe7ef33cb07 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt
@@ -14,7 +14,8 @@ SYNOPSIS
 	   [--allow-empty-message] [--no-verify] [-e] [--author=<author>]
 	   [--date=<date>] [--cleanup=<mode>] [--[no-]status]
 	   [-i | -o] [--pathspec-from-file=<file> [--pathspec-file-nul]]
-	   [-S[<keyid>]] [--] [<pathspec>...]
+	   [(--trailer <token>[(=|:)<value>])...] [-S[<keyid>]]
+	   [--] [<pathspec>...]
 
 DESCRIPTION
 -----------
@@ -166,6 +167,17 @@ The `-m` option is mutually exclusive with `-c`, `-C`, and `-F`.
 
 include::signoff-option.txt[]
 
+--trailer <token>[(=|:)<value>]::
+	Specify a (<token>, <value>) pair that should be applied as a
+	trailer. (e.g. `git commit --trailer "Signed-off-by:C O Mitter \
+	<committer@example.com>" --trailer "Helped-by:C O Mitter \
+	<committer@example.com>"` will add the "Signed-off-by" trailer
+	and the "Helped-by" trailer to the commit message.)
+	The `trailer.*` configuration variables
+	(linkgit:git-interpret-trailers[1]) can be used to define if
+	a duplicated trailer is omitted, where in the run of trailers
+	each trailer would appear, and other details.
+
 -n::
 --no-verify::
 	This option bypasses the pre-commit and commit-msg hooks.
diff --git a/builtin/commit.c b/builtin/commit.c
index 739110c5a7f6..4b06672bd07d 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -113,6 +113,7 @@ static int config_commit_verbose = -1; /* unspecified */
 static int no_post_rewrite, allow_empty_message, pathspec_file_nul;
 static char *untracked_files_arg, *force_date, *ignore_submodule_arg, *ignored_arg;
 static char *sign_commit, *pathspec_from_file;
+static struct strvec trailer_args = STRVEC_INIT;
 
 /*
  * The default commit message cleanup mode will remove the lines
@@ -131,6 +132,14 @@ static struct strbuf message = STRBUF_INIT;
 
 static enum wt_status_format status_format = STATUS_FORMAT_UNSPECIFIED;
 
+static int opt_pass_trailer(const struct option *opt, const char *arg, int unset)
+{
+	BUG_ON_OPT_NEG(unset);
+
+	strvec_pushl(&trailer_args, "--trailer", arg, NULL);
+	return 0;
+}
+
 static int opt_parse_porcelain(const struct option *opt, const char *arg, int unset)
 {
 	enum wt_status_format *value = (enum wt_status_format *)opt->value;
@@ -958,6 +967,18 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 
 	fclose(s->fp);
 
+	if (trailer_args.nr) {
+		struct child_process run_trailer = CHILD_PROCESS_INIT;
+
+		strvec_pushl(&run_trailer.args, "interpret-trailers",
+			     "--in-place", git_path_commit_editmsg(), NULL);
+		strvec_pushv(&run_trailer.args, trailer_args.v);
+		run_trailer.git_cmd = 1;
+		if (run_command(&run_trailer))
+			die(_("unable to pass trailers to --trailers"));
+		strvec_clear(&trailer_args);
+	}
+
 	/*
 	 * Reject an attempt to record a non-merge empty commit without
 	 * explicit --allow-empty. In the cherry-pick case, it may be
@@ -1507,6 +1528,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 		OPT_STRING(0, "fixup", &fixup_message, N_("commit"), N_("use autosquash formatted message to fixup specified commit")),
 		OPT_STRING(0, "squash", &squash_message, N_("commit"), N_("use autosquash formatted message to squash specified commit")),
 		OPT_BOOL(0, "reset-author", &renew_authorship, N_("the commit is authored by me now (used with -C/-c/--amend)")),
+		OPT_CALLBACK_F(0, "trailer", NULL, N_("trailer"), N_("add custom trailer(s)"), PARSE_OPT_NONEG, opt_pass_trailer),
 		OPT_BOOL('s', "signoff", &signoff, N_("add a Signed-off-by trailer")),
 		OPT_FILENAME('t', "template", &template_file, N_("use specified template file")),
 		OPT_BOOL('e', "edit", &edit_flag, N_("force edit of commit")),
diff --git a/t/t7502-commit-porcelain.sh b/t/t7502-commit-porcelain.sh
index 6396897cc818..74b1602c0ce6 100755
--- a/t/t7502-commit-porcelain.sh
+++ b/t/t7502-commit-porcelain.sh
@@ -38,6 +38,16 @@ check_summary_oneline() {
 	test_cmp exp act
 }
 
+trailer_commit_base () {
+	echo "fun" >>file &&
+	git add file &&
+	git commit -s --trailer "Signed-off-by=C1 E1 " \
+		--trailer "Helped-by:C2 E2 " \
+		--trailer "Reported-by=C3 E3" \
+		--trailer "Mentored-by:C4 E4" \
+		-m "hello"
+}
+
 test_expect_success 'output summary format' '
 
 	echo new >file1 &&
@@ -154,6 +164,287 @@ test_expect_success 'sign off' '
 
 '
 
+test_expect_success 'commit --trailer with "="' '
+	trailer_commit_base &&
+	cat >expected <<-\EOF &&
+	hello
+
+	Signed-off-by: C O Mitter <committer@example.com>
+	Signed-off-by: C1 E1
+	Helped-by: C2 E2
+	Reported-by: C3 E3
+	Mentored-by: C4 E4
+	EOF
+	git cat-file commit HEAD >commit.msg &&
+	sed -e "1,/^\$/d" commit.msg >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'commit --trailer with -c and "replace" as ifexists' '
+	trailer_commit_base &&
+	cat >expected <<-\EOF &&
+	hello
+
+	Signed-off-by: C O Mitter <committer@example.com>
+	Signed-off-by: C1 E1
+	Reported-by: C3 E3
+	Mentored-by: C4 E4
+	Helped-by: C3 E3
+	EOF
+	git -c trailer.ifexists="replace" \
+		commit --trailer "Mentored-by: C4 E4" \
+		 --trailer "Helped-by: C3 E3" \
+		--amend &&
+	git cat-file commit HEAD >commit.msg &&
+	sed -e "1,/^\$/d"  commit.msg >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'commit --trailer with -c and "add" as ifexists' '
+	trailer_commit_base &&
+	cat >expected <<-\EOF &&
+	hello
+
+	Signed-off-by: C O Mitter <committer@example.com>
+	Signed-off-by: C1 E1
+	Helped-by: C2 E2
+	Reported-by: C3 E3
+	Mentored-by: C4 E4
+	Reported-by: C3 E3
+	Mentored-by: C4 E4
+	EOF
+	git -c trailer.ifexists="add" \
+		commit --trailer "Reported-by: C3 E3" \
+		--trailer "Mentored-by: C4 E4" \
+		--amend &&
+	git cat-file commit HEAD >commit.msg &&
+	sed -e "1,/^\$/d"  commit.msg >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'commit --trailer with -c and "donothing" as ifexists' '
+	trailer_commit_base &&
+	cat >expected <<-\EOF &&
+	hello
+
+	Signed-off-by: C O Mitter <committer@example.com>
+	Signed-off-by: C1 E1
+	Helped-by: C2 E2
+	Reported-by: C3 E3
+	Mentored-by: C4 E4
+	Reviewed-by: C6 E6
+	EOF
+	git -c trailer.ifexists="donothing" \
+		commit --trailer "Mentored-by: C5 E5" \
+		--trailer "Reviewed-by: C6 E6" \
+		--amend &&
+	git cat-file commit HEAD >commit.msg &&
+	sed -e "1,/^\$/d"  commit.msg >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'commit --trailer with -c and "addIfDifferent" as ifexists' '
+	trailer_commit_base &&
+	cat >expected <<-\EOF &&
+	hello
+
+	Signed-off-by: C O Mitter <committer@example.com>
+	Signed-off-by: C1 E1
+	Helped-by: C2 E2
+	Reported-by: C3 E3
+	Mentored-by: C4 E4
+	Mentored-by: C5 E5
+	EOF
+	git -c trailer.ifexists="addIfDifferent" \
+		commit --trailer "Reported-by: C3 E3" \
+		--trailer "Mentored-by: C5 E5" \
+		--amend &&
+	git cat-file commit HEAD >commit.msg &&
+	sed -e "1,/^\$/d"  commit.msg >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'commit --trailer with -c and "addIfDifferentNeighbor" as ifexists' '
+	trailer_commit_base &&
+	cat >expected <<-\EOF &&
+	hello
+
+	Signed-off-by: C O Mitter <committer@example.com>
+	Signed-off-by: C1 E1
+	Helped-by: C2 E2
+	Reported-by: C3 E3
+	Mentored-by: C4 E4
+	Reported-by: C3 E3
+	EOF
+	git -c trailer.ifexists="addIfDifferentNeighbor" \
+		commit --trailer "Mentored-by: C4 E4" \
+		--trailer "Reported-by: C3 E3" \
+		--amend &&
+	git cat-file commit HEAD >commit.msg &&
+	sed -e "1,/^\$/d"  commit.msg >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'commit --trailer with -c and "end" as where' '
+	trailer_commit_base &&
+	cat >expected <<-\EOF &&
+	hello
+
+	Signed-off-by: C O Mitter <committer@example.com>
+	Signed-off-by: C1 E1
+	Helped-by: C2 E2
+	Reported-by: C3 E3
+	Mentored-by: C4 E4
+	Reported-by: C3 E3
+	Mentored-by: C4 E4
+	EOF
+	git -c trailer.where="end" \
+		commit --trailer "Reported-by: C3 E3" \
+		--trailer "Mentored-by: C4 E4" \
+		--amend &&
+	git cat-file commit HEAD >commit.msg &&
+	sed -e "1,/^\$/d" commit.msg >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'commit --trailer with -c and "start" as where' '
+	trailer_commit_base &&
+	cat >expected <<-\EOF &&
+	hello
+
+	Signed-off-by: C1 E1
+	Signed-off-by: C O Mitter <committer@example.com>
+	Signed-off-by: C1 E1
+	Helped-by: C2 E2
+	Reported-by: C3 E3
+	Mentored-by: C4 E4
+	EOF
+	git -c trailer.where="start" \
+		commit --trailer "Signed-off-by: C O Mitter <committer@example.com>" \
+		--trailer "Signed-off-by: C1 E1" \
+		--amend &&
+	git cat-file commit HEAD >commit.msg &&
+	sed -e "1,/^\$/d" commit.msg >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'commit --trailer with -c and "after" as where' '
+	trailer_commit_base &&
+	cat >expected <<-\EOF &&
+	hello
+
+	Signed-off-by: C O Mitter <committer@example.com>
+	Signed-off-by: C1 E1
+	Helped-by: C2 E2
+	Reported-by: C3 E3
+	Mentored-by: C4 E4
+	Mentored-by: C5 E5
+	EOF
+	git -c trailer.where="after" \
+		commit --trailer "Mentored-by: C4 E4" \
+		--trailer "Mentored-by: C5 E5" \
+		--amend &&
+	git cat-file commit HEAD >commit.msg &&
+	sed -e "1,/^\$/d" commit.msg >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'commit --trailer with -c and "before" as where' '
+	trailer_commit_base &&
+	cat >expected <<-\EOF &&
+	hello
+
+	Signed-off-by: C O Mitter <committer@example.com>
+	Signed-off-by: C1 E1
+	Helped-by: C2 E2
+	Reported-by: C3 E3
+	Mentored-by: C2 E2
+	Mentored-by: C3 E3
+	Mentored-by: C4 E4
+	EOF
+	git -c trailer.where="before" \
+		commit --trailer "Mentored-by: C3 E3" \
+		--trailer "Mentored-by: C2 E2" \
+		--amend &&
+	git cat-file commit HEAD >commit.msg &&
+	sed -e "1,/^\$/d" commit.msg >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'commit --trailer with -c and "donothing" as ifmissing' '
+	trailer_commit_base &&
+	cat >expected <<-\EOF &&
+	hello
+
+	Signed-off-by: C O Mitter <committer@example.com>
+	Signed-off-by: C1 E1
+	Helped-by: C2 E2
+	Reported-by: C3 E3
+	Mentored-by: C4 E4
+	Helped-by: C5 E5
+	EOF
+	git -c trailer.ifmissing="donothing" \
+		commit --trailer "Helped-by: C5 E5" \
+		--trailer "Based-by: C6 E6" \
+		--amend &&
+	git cat-file commit HEAD >commit.msg &&
+	sed -e "1,/^\$/d" commit.msg >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'commit --trailer with -c and "add" as ifmissing' '
+	trailer_commit_base &&
+	cat >expected <<-\EOF &&
+	hello
+
+	Signed-off-by: C O Mitter <committer@example.com>
+	Signed-off-by: C1 E1
+	Helped-by: C2 E2
+	Reported-by: C3 E3
+	Mentored-by: C4 E4
+	Helped-by: C5 E5
+	Based-by: C6 E6
+	EOF
+	git -c trailer.ifmissing="add" \
+		commit --trailer "Helped-by: C5 E5" \
+		--trailer "Based-by: C6 E6" \
+		--amend &&
+	git cat-file commit HEAD >commit.msg &&
+	sed -e "1,/^\$/d" commit.msg >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'commit --trailer with -c ack.key ' '
+	echo "fun" >>file1 &&
+	git add file1 &&
+	cat >expected <<-\EOF &&
+		hello
+
+		Acked-by: Peff
+	EOF
+	git -c trailer.ack.key="Acked-by" \
+		commit --trailer "ack = Peff" -m "hello" &&
+	git cat-file commit HEAD >commit.msg &&
+	sed -e "1,/^\$/d" commit.msg >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'commit --trailer with -c and ":=#" as separators' '
+	echo "fun" >>file1 &&
+	git add file1 &&
+	cat >expected <<-\EOF &&
+		I hate bug
+
+		Bug #42
+	EOF
+	git -c trailer.separators=":=#" \
+		-c trailer.bug.key="Bug #" \
+		commit --trailer "bug = 42" -m "I hate bug" &&
+	git cat-file commit HEAD >commit.msg &&
+	sed -e "1,/^\$/d" commit.msg >actual &&
+	test_cmp expected actual
+'
+
 test_expect_success 'multiple -m' '
 
 	>negative &&
-- 
gitgitgadget


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 2/2] [GSOC] interpret-trailer: easy parse trailer value
  2021-03-20 14:41 [PATCH 0/2] [GSOC] interpret-trailer: easy parse trailer value ZheNing Hu via GitGitGadget
  2021-03-20 14:41 ` [PATCH 1/2] [GSOC] commit: add --trailer option ZheNing Hu via GitGitGadget
@ 2021-03-20 14:41 ` ZheNing Hu via GitGitGadget
  2021-03-20 21:06   ` Junio C Hamano
  2021-03-21  8:58 ` [PATCH v2 0/2] " ZheNing Hu via GitGitGadget
  2 siblings, 1 reply; 23+ messages in thread
From: ZheNing Hu via GitGitGadget @ 2021-03-20 14:41 UTC (permalink / raw)
  To: git
  Cc: Bradley M. Kuhn, Junio C Hamano, Brandon Casey, Shourya Shukla,
	Christian Couder, Rafael Silva,
	Đoàn Trần Công Danh, Jeff King,
	Ævar Arnfjörð Bjarmason, ZheNing Hu, ZheNing Hu

From: ZheNing Hu <adlternative@gmail.com>

The original `-trailer` adding some trailers like
"Signed-off-by:C O <Mister@email.com>" is often too
verbose and error-prone.

Now add the syntax parse for the value of `--trailer`:
e.g. "Signed-off-by:@Junio", git will fuzzy search in the
commit history to find the latest one commit which matches
`--author=Junio`, and get the "author <email>" pair
`Junio C Hamano <gitster@pobox.com>` as the value of
`--trailer`, it will be a easy way to add trailers.
git commit --trailer` can also benefit from this.

Signed-off-by: ZheNing Hu <adlternative@gmail.com>
Helped-by: Junio C Hamano <gitster@pobox.com>
Helped-by: Jeff King <peff@peff.net>
---
 Documentation/git-interpret-trailers.txt | 23 ++++++++++++++++
 builtin/commit.c                         | 33 -----------------------
 commit.c                                 | 34 ++++++++++++++++++++++++
 commit.h                                 |  2 ++
 t/t7502-commit-porcelain.sh              | 28 +++++++++++++++++++
 t/t7513-interpret-trailers.sh            | 23 ++++++++++++++++
 trailer.c                                | 13 ++++++++-
 7 files changed, 122 insertions(+), 34 deletions(-)

diff --git a/Documentation/git-interpret-trailers.txt b/Documentation/git-interpret-trailers.txt
index 96ec6499f001..33e76f2a58fb 100644
--- a/Documentation/git-interpret-trailers.txt
+++ b/Documentation/git-interpret-trailers.txt
@@ -69,6 +69,29 @@ Note that 'trailers' do not follow and are not intended to follow many
 rules for RFC 822 headers. For example they do not follow
 the encoding rules and probably many other rules.
 
+Support to replace the value in the form of `@nickname`, provided that the
+commit with nickname as the author can be found in the your repository's git
+log. For example, in git's source code repository you can use commands:
+
+`git log --author="Junio"  --pretty="%an <%ae>" | sort |uniq `
+
+to find Junio's "name <email>" pair:
+
+Junio C Hamano <gitster@pobox.com>
+Junio C Hamano <junio@hera.kernel.org>
+Junio C Hamano <junio@kernel.org>
+Junio C Hamano <junio@pobox.com>
+...
+
+If you want to add a `Helped-by` trailer with Junio "name <email>" pair,
+you can use:
+
+`git commit --trailers "Helped-by:@Junio"`
+
+to insert the trailer to your commit messages:
+
+Helped-by: Junio C Hamano <gitster@pobox.com>
+
 OPTIONS
 -------
 --in-place::
diff --git a/builtin/commit.c b/builtin/commit.c
index 4b06672bd07d..58c020e3cfbf 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1043,39 +1043,6 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 	return 1;
 }
 
-static const char *find_author_by_nickname(const char *name)
-{
-	struct rev_info revs;
-	struct commit *commit;
-	struct strbuf buf = STRBUF_INIT;
-	struct string_list mailmap = STRING_LIST_INIT_NODUP;
-	const char *av[20];
-	int ac = 0;
-
-	repo_init_revisions(the_repository, &revs, NULL);
-	strbuf_addf(&buf, "--author=%s", name);
-	av[++ac] = "--all";
-	av[++ac] = "-i";
-	av[++ac] = buf.buf;
-	av[++ac] = NULL;
-	setup_revisions(ac, av, &revs, NULL);
-	revs.mailmap = &mailmap;
-	read_mailmap(revs.mailmap);
-
-	if (prepare_revision_walk(&revs))
-		die(_("revision walk setup failed"));
-	commit = get_revision(&revs);
-	if (commit) {
-		struct pretty_print_context ctx = {0};
-		ctx.date_mode.type = DATE_NORMAL;
-		strbuf_release(&buf);
-		format_commit_message(commit, "%aN <%aE>", &buf, &ctx);
-		clear_mailmap(&mailmap);
-		return strbuf_detach(&buf, NULL);
-	}
-	die(_("--author '%s' is not 'Name <email>' and matches no existing author"), name);
-}
-
 static void handle_ignored_arg(struct wt_status *s)
 {
 	if (!ignored_arg)
diff --git a/commit.c b/commit.c
index 6ccd774841c6..e1aad52d2c4f 100644
--- a/commit.c
+++ b/commit.c
@@ -21,6 +21,7 @@
 #include "commit-reach.h"
 #include "run-command.h"
 #include "shallow.h"
+#include "mailmap.h"
 
 static struct commit_extra_header *read_commit_extra_header_lines(const char *buf, size_t len, const char **);
 
@@ -1703,3 +1704,36 @@ int run_commit_hook(int editor_is_used, const char *index_file,
 
 	return ret;
 }
+
+const char *find_author_by_nickname(const char *name)
+{
+	struct rev_info revs;
+	struct commit *commit;
+	struct strbuf buf = STRBUF_INIT;
+	struct string_list mailmap = STRING_LIST_INIT_NODUP;
+	const char *av[20];
+	int ac = 0;
+
+	repo_init_revisions(the_repository, &revs, NULL);
+	strbuf_addf(&buf, "--author=%s", name);
+	av[++ac] = "--all";
+	av[++ac] = "-i";
+	av[++ac] = buf.buf;
+	av[++ac] = NULL;
+	setup_revisions(ac, av, &revs, NULL);
+	revs.mailmap = &mailmap;
+	read_mailmap(revs.mailmap);
+
+	if (prepare_revision_walk(&revs))
+		die(_("revision walk setup failed"));
+	commit = get_revision(&revs);
+	if (commit) {
+		struct pretty_print_context ctx = {0};
+		ctx.date_mode.type = DATE_NORMAL;
+		strbuf_release(&buf);
+		format_commit_message(commit, "%aN <%aE>", &buf, &ctx);
+		clear_mailmap(&mailmap);
+		return strbuf_detach(&buf, NULL);
+	}
+	die(_("--author '%s' is not 'Name <email>' and matches no existing author"), name);
+}
diff --git a/commit.h b/commit.h
index 49c0f503964e..970a73ccd5be 100644
--- a/commit.h
+++ b/commit.h
@@ -371,4 +371,6 @@ int parse_buffer_signed_by_header(const char *buffer,
 				  struct strbuf *signature,
 				  const struct git_hash_algo *algop);
 
+const char *find_author_by_nickname(const char *name);
+
 #endif /* COMMIT_H */
diff --git a/t/t7502-commit-porcelain.sh b/t/t7502-commit-porcelain.sh
index 74b1602c0ce6..143690e2833c 100755
--- a/t/t7502-commit-porcelain.sh
+++ b/t/t7502-commit-porcelain.sh
@@ -445,6 +445,34 @@ test_expect_success 'commit --trailer with -c and ":=#" as separators' '
 	test_cmp expected actual
 '
 
+
+test_expect_success 'commit --trailer parse @nickname' '
+	echo "I love git" >file1 &&
+	git add file1 &&
+	git commit -m "yly" --author="batman <email1>" &&
+	echo "I love git" >file2 &&
+	git add file2 &&
+	git commit -m "yly" --author="jocker <email2>" &&
+	echo "I love git" >file3 &&
+	git add file3 &&
+	git commit -m "yly" \
+	--trailer "Reviewed-by:@bat" \
+	--trailer "Signed-off-by:@jock" \
+	--trailer "Helped-by:@email1" \
+	--trailer "Mentored-by:@email2" &&
+	git cat-file commit HEAD >commit.msg &&
+	sed -e "1,/^\$/d" commit.msg >actual &&
+	cat >expected <<-\EOF &&
+	yly
+
+	Reviewed-by: batman <email1>
+	Signed-off-by: jocker <email2>
+	Helped-by: batman <email1>
+	Mentored-by: jocker <email2>
+	EOF
+	test_cmp expected actual
+'
+
 test_expect_success 'multiple -m' '
 
 	>negative &&
diff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh
index 6602790b5f4c..f2f1ae3b2faf 100755
--- a/t/t7513-interpret-trailers.sh
+++ b/t/t7513-interpret-trailers.sh
@@ -63,6 +63,29 @@ test_expect_success 'without config' '
 	test_cmp expected actual
 '
 
+test_expect_success 'trailer parse @nickname' '
+	echo "I love git" >file1 &&
+	git add file1 &&
+	git commit -m "yly" --author="batman <email1>" &&
+	echo "I love git" >file2 &&
+	git add file2 &&
+	git commit -m "yly" --author="jocker <email2>" &&
+	git interpret-trailers \
+	--trailer "Reviewed-by:@bat" \
+	--trailer "Signed-off-by:@jock" \
+	--trailer "Helped-by:@email1" \
+	--trailer "Mentored-by:@email2" \
+	empty >actual &&
+	cat >expected <<-\EOF &&
+
+	Reviewed-by: batman <email1>
+	Signed-off-by: jocker <email2>
+	Helped-by: batman <email1>
+	Mentored-by: jocker <email2>
+	EOF
+	test_cmp expected actual
+'
+
 test_expect_success 'without config in another order' '
 	sed -e "s/ Z\$/ /" >expected <<-\EOF &&
 
diff --git a/trailer.c b/trailer.c
index 249ed618ed8e..21f367e7b761 100644
--- a/trailer.c
+++ b/trailer.c
@@ -6,6 +6,7 @@
 #include "tempfile.h"
 #include "trailer.h"
 #include "list.h"
+#include "revision.h"
 /*
  * Copyright (c) 2013, 2014 Christian Couder <chriscool@tuxfamily.org>
  */
@@ -633,11 +634,21 @@ static void parse_trailer(struct strbuf *tok, struct strbuf *val,
 	struct arg_item *item;
 	size_t tok_len;
 	struct list_head *pos;
+	const char *ae = NULL;
 
 	if (separator_pos != -1) {
 		strbuf_add(tok, trailer, separator_pos);
 		strbuf_trim(tok);
-		strbuf_addstr(val, trailer + separator_pos + 1);
+		if (trailer[separator_pos + 1] == '@') {
+			ae = find_author_by_nickname(trailer + separator_pos + 2);
+			reset_revision_walk();
+			if (ae) {
+				strbuf_addstr(val, ae);
+				free((char*)ae);
+			} else
+				strbuf_addstr(val, trailer + separator_pos + 1);
+		} else
+			strbuf_addstr(val, trailer + separator_pos + 1);
 		strbuf_trim(val);
 	} else {
 		strbuf_addstr(tok, trailer);
-- 
gitgitgadget

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [PATCH 2/2] [GSOC] interpret-trailer: easy parse trailer value
  2021-03-20 14:41 ` [PATCH 2/2] [GSOC] interpret-trailer: easy parse trailer value ZheNing Hu via GitGitGadget
@ 2021-03-20 21:06   ` Junio C Hamano
  2021-03-21  3:29     ` ZheNing Hu
  0 siblings, 1 reply; 23+ messages in thread
From: Junio C Hamano @ 2021-03-20 21:06 UTC (permalink / raw)
  To: ZheNing Hu via GitGitGadget
  Cc: git, Bradley M. Kuhn, Brandon Casey, Shourya Shukla,
	Christian Couder, Rafael Silva,
	Đoàn Trần Công Danh, Jeff King,
	Ævar Arnfjörð Bjarmason, ZheNing Hu

"ZheNing Hu via GitGitGadget" <gitgitgadget@gmail.com> writes:

> diff --git a/commit.h b/commit.h
> index 49c0f503964e..970a73ccd5be 100644
> --- a/commit.h
> +++ b/commit.h
> @@ -371,4 +371,6 @@ int parse_buffer_signed_by_header(const char *buffer,
>  				  struct strbuf *signature,
>  				  const struct git_hash_algo *algop);
>  
> +const char *find_author_by_nickname(const char *name);
> +
>  #endif /* COMMIT_H */

As I already said, we do not want to pretend that this is a
generally reusable helper function.  We should at least have a
comment to tell people to never add new callers to this function,
with explanation of the reason.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 2/2] [GSOC] interpret-trailer: easy parse trailer value
  2021-03-20 21:06   ` Junio C Hamano
@ 2021-03-21  3:29     ` ZheNing Hu
  2021-03-21 13:57       ` Junio C Hamano
  0 siblings, 1 reply; 23+ messages in thread
From: ZheNing Hu @ 2021-03-21  3:29 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: ZheNing Hu via GitGitGadget, Git List, Bradley M. Kuhn,
	Brandon Casey, Shourya Shukla, Christian Couder, Rafael Silva,
	Đoàn Trần Công Danh, Jeff King,
	Ævar Arnfjörð Bjarmason

Junio C Hamano <gitster@pobox.com> 于2021年3月21日周日 上午5:06写道:
>
> "ZheNing Hu via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > diff --git a/commit.h b/commit.h
> > index 49c0f503964e..970a73ccd5be 100644
> > --- a/commit.h
> > +++ b/commit.h
> > @@ -371,4 +371,6 @@ int parse_buffer_signed_by_header(const char *buffer,
> >                                 struct strbuf *signature,
> >                                 const struct git_hash_algo *algop);
> >
> > +const char *find_author_by_nickname(const char *name);
> > +
> >  #endif /* COMMIT_H */
>
> As I already said, we do not want to pretend that this is a
> generally reusable helper function.  We should at least have a
> comment to tell people to never add new callers to this function,
> with explanation of the reason.

Do you think this is appropriate?

@@ -370,5 +370,15 @@ int parse_buffer_signed_by_header(const char *buffer,
                                  struct strbuf *payload,
                                  struct strbuf *signature,
                                  const struct git_hash_algo *algop);
+/*
+ * Calling `find_author_by_nickname` to find the "author <email>" pair
+ * in the most recent commit which matches "--author=name".
+ *
+ * Note that `find_author_by_nickname` is not reusable, because it haven't
+ * reset flags for parsed objects. The only safe way to use
`find_author_by_nickname`
+ * (without rewriting the revision traversal machinery) is to spawn a
+ * subprocess and do find_author_by_nickname() in it.
+ */
+const char *find_author_by_nickname(const char *name);

Thanks.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH v2 0/2] [GSOC] interpret-trailer: easy parse trailer value
  2021-03-20 14:41 [PATCH 0/2] [GSOC] interpret-trailer: easy parse trailer value ZheNing Hu via GitGitGadget
  2021-03-20 14:41 ` [PATCH 1/2] [GSOC] commit: add --trailer option ZheNing Hu via GitGitGadget
  2021-03-20 14:41 ` [PATCH 2/2] [GSOC] interpret-trailer: easy parse trailer value ZheNing Hu via GitGitGadget
@ 2021-03-21  8:58 ` ZheNing Hu via GitGitGadget
  2021-03-21  8:58   ` [PATCH v2 1/2] [GSOC] commit: add --trailer option ZheNing Hu via GitGitGadget
  2021-03-21  8:58   ` [PATCH v2 2/2] [GSOC] interpret-trailer: easy parse trailer value ZheNing Hu via GitGitGadget
  2 siblings, 2 replies; 23+ messages in thread
From: ZheNing Hu via GitGitGadget @ 2021-03-21  8:58 UTC (permalink / raw)
  To: git
  Cc: Bradley M. Kuhn, Junio C Hamano, Brandon Casey, Shourya Shukla,
	Christian Couder, Rafael Silva,
	Đoàn Trần Công Danh, Jeff King,
	Ævar Arnfjörð Bjarmason, ZheNing Hu

Hope this can help commit --trailer more useful!

Base on https://github.com/gitgitgadget/git/pull/901, The original patch
series was too long so now split it into two.

Helped by Junio and Jeff, Thanks.

ZheNing Hu (2):
  [GSOC] commit: add --trailer option
  [GSOC] interpret-trailer: easy parse trailer value

 Documentation/git-commit.txt             |  14 +-
 Documentation/git-interpret-trailers.txt |  23 ++
 builtin/commit.c                         |  55 ++--
 commit.c                                 |  34 +++
 commit.h                                 |  10 +
 t/t7502-commit-porcelain.sh              | 319 +++++++++++++++++++++++
 t/t7513-interpret-trailers.sh            |  23 ++
 trailer.c                                |  13 +-
 8 files changed, 456 insertions(+), 35 deletions(-)


base-commit: 13d7ab6b5d7929825b626f050b62a11241ea4945
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-911%2Fadlternative%2Ftrailer-easy-ident-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-911/adlternative/trailer-easy-ident-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/911

Range-diff vs v1:

 1:  2378e3b4c1ae = 1:  2378e3b4c1ae [GSOC] commit: add --trailer option
 2:  ca521d3c01d6 ! 2:  8b8b236a4ffb [GSOC] interpret-trailer: easy parse trailer value
     @@ commit.c: int run_commit_hook(int editor_is_used, const char *index_file,
      
       ## commit.h ##
      @@ commit.h: int parse_buffer_signed_by_header(const char *buffer,
     + 				  struct strbuf *payload,
       				  struct strbuf *signature,
       				  const struct git_hash_algo *algop);
     - 
     ++/*
     ++ * Calling `find_author_by_nickname` to find the "author <email>" pair
     ++ * in the most recent commit which matches "--author=name".
     ++ *
     ++ * Note that `find_author_by_nickname` is not reusable, because it haven't
     ++ * reset flags for parsed objects. The only safe way to use `find_author_by_nickname`
     ++ * (without rewriting the revision traversal machinery) is to spawn a
     ++ * subprocess and do find_author_by_nickname() in it.
     ++ */
      +const char *find_author_by_nickname(const char *name);
     -+
     + 
       #endif /* COMMIT_H */
      
       ## t/t7502-commit-porcelain.sh ##

-- 
gitgitgadget

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH v2 1/2] [GSOC] commit: add --trailer option
  2021-03-21  8:58 ` [PATCH v2 0/2] " ZheNing Hu via GitGitGadget
@ 2021-03-21  8:58   ` ZheNing Hu via GitGitGadget
  2021-03-21  8:58   ` [PATCH v2 2/2] [GSOC] interpret-trailer: easy parse trailer value ZheNing Hu via GitGitGadget
  1 sibling, 0 replies; 23+ messages in thread
From: ZheNing Hu via GitGitGadget @ 2021-03-21  8:58 UTC (permalink / raw)
  To: git
  Cc: Bradley M. Kuhn, Junio C Hamano, Brandon Casey, Shourya Shukla,
	Christian Couder, Rafael Silva,
	Đoàn Trần Công Danh, Jeff King,
	Ævar Arnfjörð Bjarmason, ZheNing Hu, ZheNing Hu

From: ZheNing Hu <adlternative@gmail.com>

Historically, Git has supported the 'Signed-off-by' commit trailer
using the '--signoff' and the '-s' option from the command line.
But users may need to provide other trailer information from the
command line such as "Helped-by", "Reported-by", "Mentored-by",

Now implement a new `--trailer <token>[(=|:)<value>]` option to pass
other trailers to `interpret-trailers` and insert them into commit
messages.

Signed-off-by: ZheNing Hu <adlternative@gmail.com>
---
 Documentation/git-commit.txt |  14 +-
 builtin/commit.c             |  22 +++
 t/t7502-commit-porcelain.sh  | 291 +++++++++++++++++++++++++++++++++++
 3 files changed, 326 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index 17150fa7eabe..3fe7ef33cb07 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt
@@ -14,7 +14,8 @@ SYNOPSIS
 	   [--allow-empty-message] [--no-verify] [-e] [--author=<author>]
 	   [--date=<date>] [--cleanup=<mode>] [--[no-]status]
 	   [-i | -o] [--pathspec-from-file=<file> [--pathspec-file-nul]]
-	   [-S[<keyid>]] [--] [<pathspec>...]
+	   [(--trailer <token>[(=|:)<value>])...] [-S[<keyid>]]
+	   [--] [<pathspec>...]
 
 DESCRIPTION
 -----------
@@ -166,6 +167,17 @@ The `-m` option is mutually exclusive with `-c`, `-C`, and `-F`.
 
 include::signoff-option.txt[]
 
+--trailer <token>[(=|:)<value>]::
+	Specify a (<token>, <value>) pair that should be applied as a
+	trailer. (e.g. `git commit --trailer "Signed-off-by:C O Mitter \
+	<committer@example.com>" --trailer "Helped-by:C O Mitter \
+	<committer@example.com>"` will add the "Signed-off-by" trailer
+	and the "Helped-by" trailer to the commit message.)
+	The `trailer.*` configuration variables
+	(linkgit:git-interpret-trailers[1]) can be used to define if
+	a duplicated trailer is omitted, where in the run of trailers
+	each trailer would appear, and other details.
+
 -n::
 --no-verify::
 	This option bypasses the pre-commit and commit-msg hooks.
diff --git a/builtin/commit.c b/builtin/commit.c
index 739110c5a7f6..4b06672bd07d 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -113,6 +113,7 @@ static int config_commit_verbose = -1; /* unspecified */
 static int no_post_rewrite, allow_empty_message, pathspec_file_nul;
 static char *untracked_files_arg, *force_date, *ignore_submodule_arg, *ignored_arg;
 static char *sign_commit, *pathspec_from_file;
+static struct strvec trailer_args = STRVEC_INIT;
 
 /*
  * The default commit message cleanup mode will remove the lines
@@ -131,6 +132,14 @@ static struct strbuf message = STRBUF_INIT;
 
 static enum wt_status_format status_format = STATUS_FORMAT_UNSPECIFIED;
 
+static int opt_pass_trailer(const struct option *opt, const char *arg, int unset)
+{
+	BUG_ON_OPT_NEG(unset);
+
+	strvec_pushl(&trailer_args, "--trailer", arg, NULL);
+	return 0;
+}
+
 static int opt_parse_porcelain(const struct option *opt, const char *arg, int unset)
 {
 	enum wt_status_format *value = (enum wt_status_format *)opt->value;
@@ -958,6 +967,18 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 
 	fclose(s->fp);
 
+	if (trailer_args.nr) {
+		struct child_process run_trailer = CHILD_PROCESS_INIT;
+
+		strvec_pushl(&run_trailer.args, "interpret-trailers",
+			     "--in-place", git_path_commit_editmsg(), NULL);
+		strvec_pushv(&run_trailer.args, trailer_args.v);
+		run_trailer.git_cmd = 1;
+		if (run_command(&run_trailer))
+			die(_("unable to pass trailers to --trailers"));
+		strvec_clear(&trailer_args);
+	}
+
 	/*
 	 * Reject an attempt to record a non-merge empty commit without
 	 * explicit --allow-empty. In the cherry-pick case, it may be
@@ -1507,6 +1528,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 		OPT_STRING(0, "fixup", &fixup_message, N_("commit"), N_("use autosquash formatted message to fixup specified commit")),
 		OPT_STRING(0, "squash", &squash_message, N_("commit"), N_("use autosquash formatted message to squash specified commit")),
 		OPT_BOOL(0, "reset-author", &renew_authorship, N_("the commit is authored by me now (used with -C/-c/--amend)")),
+		OPT_CALLBACK_F(0, "trailer", NULL, N_("trailer"), N_("add custom trailer(s)"), PARSE_OPT_NONEG, opt_pass_trailer),
 		OPT_BOOL('s', "signoff", &signoff, N_("add a Signed-off-by trailer")),
 		OPT_FILENAME('t', "template", &template_file, N_("use specified template file")),
 		OPT_BOOL('e', "edit", &edit_flag, N_("force edit of commit")),
diff --git a/t/t7502-commit-porcelain.sh b/t/t7502-commit-porcelain.sh
index 6396897cc818..74b1602c0ce6 100755
--- a/t/t7502-commit-porcelain.sh
+++ b/t/t7502-commit-porcelain.sh
@@ -38,6 +38,16 @@ check_summary_oneline() {
 	test_cmp exp act
 }
 
+trailer_commit_base () {
+	echo "fun" >>file &&
+	git add file &&
+	git commit -s --trailer "Signed-off-by=C1 E1 " \
+		--trailer "Helped-by:C2 E2 " \
+		--trailer "Reported-by=C3 E3" \
+		--trailer "Mentored-by:C4 E4" \
+		-m "hello"
+}
+
 test_expect_success 'output summary format' '
 
 	echo new >file1 &&
@@ -154,6 +164,287 @@ test_expect_success 'sign off' '
 
 '
 
+test_expect_success 'commit --trailer with "="' '
+	trailer_commit_base &&
+	cat >expected <<-\EOF &&
+	hello
+
+	Signed-off-by: C O Mitter <committer@example.com>
+	Signed-off-by: C1 E1
+	Helped-by: C2 E2
+	Reported-by: C3 E3
+	Mentored-by: C4 E4
+	EOF
+	git cat-file commit HEAD >commit.msg &&
+	sed -e "1,/^\$/d" commit.msg >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'commit --trailer with -c and "replace" as ifexists' '
+	trailer_commit_base &&
+	cat >expected <<-\EOF &&
+	hello
+
+	Signed-off-by: C O Mitter <committer@example.com>
+	Signed-off-by: C1 E1
+	Reported-by: C3 E3
+	Mentored-by: C4 E4
+	Helped-by: C3 E3
+	EOF
+	git -c trailer.ifexists="replace" \
+		commit --trailer "Mentored-by: C4 E4" \
+		 --trailer "Helped-by: C3 E3" \
+		--amend &&
+	git cat-file commit HEAD >commit.msg &&
+	sed -e "1,/^\$/d"  commit.msg >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'commit --trailer with -c and "add" as ifexists' '
+	trailer_commit_base &&
+	cat >expected <<-\EOF &&
+	hello
+
+	Signed-off-by: C O Mitter <committer@example.com>
+	Signed-off-by: C1 E1
+	Helped-by: C2 E2
+	Reported-by: C3 E3
+	Mentored-by: C4 E4
+	Reported-by: C3 E3
+	Mentored-by: C4 E4
+	EOF
+	git -c trailer.ifexists="add" \
+		commit --trailer "Reported-by: C3 E3" \
+		--trailer "Mentored-by: C4 E4" \
+		--amend &&
+	git cat-file commit HEAD >commit.msg &&
+	sed -e "1,/^\$/d"  commit.msg >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'commit --trailer with -c and "donothing" as ifexists' '
+	trailer_commit_base &&
+	cat >expected <<-\EOF &&
+	hello
+
+	Signed-off-by: C O Mitter <committer@example.com>
+	Signed-off-by: C1 E1
+	Helped-by: C2 E2
+	Reported-by: C3 E3
+	Mentored-by: C4 E4
+	Reviewed-by: C6 E6
+	EOF
+	git -c trailer.ifexists="donothing" \
+		commit --trailer "Mentored-by: C5 E5" \
+		--trailer "Reviewed-by: C6 E6" \
+		--amend &&
+	git cat-file commit HEAD >commit.msg &&
+	sed -e "1,/^\$/d"  commit.msg >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'commit --trailer with -c and "addIfDifferent" as ifexists' '
+	trailer_commit_base &&
+	cat >expected <<-\EOF &&
+	hello
+
+	Signed-off-by: C O Mitter <committer@example.com>
+	Signed-off-by: C1 E1
+	Helped-by: C2 E2
+	Reported-by: C3 E3
+	Mentored-by: C4 E4
+	Mentored-by: C5 E5
+	EOF
+	git -c trailer.ifexists="addIfDifferent" \
+		commit --trailer "Reported-by: C3 E3" \
+		--trailer "Mentored-by: C5 E5" \
+		--amend &&
+	git cat-file commit HEAD >commit.msg &&
+	sed -e "1,/^\$/d"  commit.msg >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'commit --trailer with -c and "addIfDifferentNeighbor" as ifexists' '
+	trailer_commit_base &&
+	cat >expected <<-\EOF &&
+	hello
+
+	Signed-off-by: C O Mitter <committer@example.com>
+	Signed-off-by: C1 E1
+	Helped-by: C2 E2
+	Reported-by: C3 E3
+	Mentored-by: C4 E4
+	Reported-by: C3 E3
+	EOF
+	git -c trailer.ifexists="addIfDifferentNeighbor" \
+		commit --trailer "Mentored-by: C4 E4" \
+		--trailer "Reported-by: C3 E3" \
+		--amend &&
+	git cat-file commit HEAD >commit.msg &&
+	sed -e "1,/^\$/d"  commit.msg >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'commit --trailer with -c and "end" as where' '
+	trailer_commit_base &&
+	cat >expected <<-\EOF &&
+	hello
+
+	Signed-off-by: C O Mitter <committer@example.com>
+	Signed-off-by: C1 E1
+	Helped-by: C2 E2
+	Reported-by: C3 E3
+	Mentored-by: C4 E4
+	Reported-by: C3 E3
+	Mentored-by: C4 E4
+	EOF
+	git -c trailer.where="end" \
+		commit --trailer "Reported-by: C3 E3" \
+		--trailer "Mentored-by: C4 E4" \
+		--amend &&
+	git cat-file commit HEAD >commit.msg &&
+	sed -e "1,/^\$/d" commit.msg >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'commit --trailer with -c and "start" as where' '
+	trailer_commit_base &&
+	cat >expected <<-\EOF &&
+	hello
+
+	Signed-off-by: C1 E1
+	Signed-off-by: C O Mitter <committer@example.com>
+	Signed-off-by: C1 E1
+	Helped-by: C2 E2
+	Reported-by: C3 E3
+	Mentored-by: C4 E4
+	EOF
+	git -c trailer.where="start" \
+		commit --trailer "Signed-off-by: C O Mitter <committer@example.com>" \
+		--trailer "Signed-off-by: C1 E1" \
+		--amend &&
+	git cat-file commit HEAD >commit.msg &&
+	sed -e "1,/^\$/d" commit.msg >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'commit --trailer with -c and "after" as where' '
+	trailer_commit_base &&
+	cat >expected <<-\EOF &&
+	hello
+
+	Signed-off-by: C O Mitter <committer@example.com>
+	Signed-off-by: C1 E1
+	Helped-by: C2 E2
+	Reported-by: C3 E3
+	Mentored-by: C4 E4
+	Mentored-by: C5 E5
+	EOF
+	git -c trailer.where="after" \
+		commit --trailer "Mentored-by: C4 E4" \
+		--trailer "Mentored-by: C5 E5" \
+		--amend &&
+	git cat-file commit HEAD >commit.msg &&
+	sed -e "1,/^\$/d" commit.msg >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'commit --trailer with -c and "before" as where' '
+	trailer_commit_base &&
+	cat >expected <<-\EOF &&
+	hello
+
+	Signed-off-by: C O Mitter <committer@example.com>
+	Signed-off-by: C1 E1
+	Helped-by: C2 E2
+	Reported-by: C3 E3
+	Mentored-by: C2 E2
+	Mentored-by: C3 E3
+	Mentored-by: C4 E4
+	EOF
+	git -c trailer.where="before" \
+		commit --trailer "Mentored-by: C3 E3" \
+		--trailer "Mentored-by: C2 E2" \
+		--amend &&
+	git cat-file commit HEAD >commit.msg &&
+	sed -e "1,/^\$/d" commit.msg >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'commit --trailer with -c and "donothing" as ifmissing' '
+	trailer_commit_base &&
+	cat >expected <<-\EOF &&
+	hello
+
+	Signed-off-by: C O Mitter <committer@example.com>
+	Signed-off-by: C1 E1
+	Helped-by: C2 E2
+	Reported-by: C3 E3
+	Mentored-by: C4 E4
+	Helped-by: C5 E5
+	EOF
+	git -c trailer.ifmissing="donothing" \
+		commit --trailer "Helped-by: C5 E5" \
+		--trailer "Based-by: C6 E6" \
+		--amend &&
+	git cat-file commit HEAD >commit.msg &&
+	sed -e "1,/^\$/d" commit.msg >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'commit --trailer with -c and "add" as ifmissing' '
+	trailer_commit_base &&
+	cat >expected <<-\EOF &&
+	hello
+
+	Signed-off-by: C O Mitter <committer@example.com>
+	Signed-off-by: C1 E1
+	Helped-by: C2 E2
+	Reported-by: C3 E3
+	Mentored-by: C4 E4
+	Helped-by: C5 E5
+	Based-by: C6 E6
+	EOF
+	git -c trailer.ifmissing="add" \
+		commit --trailer "Helped-by: C5 E5" \
+		--trailer "Based-by: C6 E6" \
+		--amend &&
+	git cat-file commit HEAD >commit.msg &&
+	sed -e "1,/^\$/d" commit.msg >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'commit --trailer with -c ack.key ' '
+	echo "fun" >>file1 &&
+	git add file1 &&
+	cat >expected <<-\EOF &&
+		hello
+
+		Acked-by: Peff
+	EOF
+	git -c trailer.ack.key="Acked-by" \
+		commit --trailer "ack = Peff" -m "hello" &&
+	git cat-file commit HEAD >commit.msg &&
+	sed -e "1,/^\$/d" commit.msg >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'commit --trailer with -c and ":=#" as separators' '
+	echo "fun" >>file1 &&
+	git add file1 &&
+	cat >expected <<-\EOF &&
+		I hate bug
+
+		Bug #42
+	EOF
+	git -c trailer.separators=":=#" \
+		-c trailer.bug.key="Bug #" \
+		commit --trailer "bug = 42" -m "I hate bug" &&
+	git cat-file commit HEAD >commit.msg &&
+	sed -e "1,/^\$/d" commit.msg >actual &&
+	test_cmp expected actual
+'
+
 test_expect_success 'multiple -m' '
 
 	>negative &&
-- 
gitgitgadget


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v2 2/2] [GSOC] interpret-trailer: easy parse trailer value
  2021-03-21  8:58 ` [PATCH v2 0/2] " ZheNing Hu via GitGitGadget
  2021-03-21  8:58   ` [PATCH v2 1/2] [GSOC] commit: add --trailer option ZheNing Hu via GitGitGadget
@ 2021-03-21  8:58   ` ZheNing Hu via GitGitGadget
  2021-03-21  9:17     ` Bagas Sanjaya
  2021-03-21 13:15     ` Christian Couder
  1 sibling, 2 replies; 23+ messages in thread
From: ZheNing Hu via GitGitGadget @ 2021-03-21  8:58 UTC (permalink / raw)
  To: git
  Cc: Bradley M. Kuhn, Junio C Hamano, Brandon Casey, Shourya Shukla,
	Christian Couder, Rafael Silva,
	Đoàn Trần Công Danh, Jeff King,
	Ævar Arnfjörð Bjarmason, ZheNing Hu, ZheNing Hu

From: ZheNing Hu <adlternative@gmail.com>

The original `-trailer` adding some trailers like
"Signed-off-by:C O <Mister@email.com>" is often too
verbose and error-prone.

Now add the syntax parse for the value of `--trailer`:
e.g. "Signed-off-by:@Junio", git will fuzzy search in the
commit history to find the latest one commit which matches
`--author=Junio`, and get the "author <email>" pair
`Junio C Hamano <gitster@pobox.com>` as the value of
`--trailer`, it will be a easy way to add trailers.
git commit --trailer` can also benefit from this.

Signed-off-by: ZheNing Hu <adlternative@gmail.com>
Helped-by: Junio C Hamano <gitster@pobox.com>
Helped-by: Jeff King <peff@peff.net>
---
 Documentation/git-interpret-trailers.txt | 23 ++++++++++++++++
 builtin/commit.c                         | 33 -----------------------
 commit.c                                 | 34 ++++++++++++++++++++++++
 commit.h                                 | 10 +++++++
 t/t7502-commit-porcelain.sh              | 28 +++++++++++++++++++
 t/t7513-interpret-trailers.sh            | 23 ++++++++++++++++
 trailer.c                                | 13 ++++++++-
 7 files changed, 130 insertions(+), 34 deletions(-)

diff --git a/Documentation/git-interpret-trailers.txt b/Documentation/git-interpret-trailers.txt
index 96ec6499f001..33e76f2a58fb 100644
--- a/Documentation/git-interpret-trailers.txt
+++ b/Documentation/git-interpret-trailers.txt
@@ -69,6 +69,29 @@ Note that 'trailers' do not follow and are not intended to follow many
 rules for RFC 822 headers. For example they do not follow
 the encoding rules and probably many other rules.
 
+Support to replace the value in the form of `@nickname`, provided that the
+commit with nickname as the author can be found in the your repository's git
+log. For example, in git's source code repository you can use commands:
+
+`git log --author="Junio"  --pretty="%an <%ae>" | sort |uniq `
+
+to find Junio's "name <email>" pair:
+
+Junio C Hamano <gitster@pobox.com>
+Junio C Hamano <junio@hera.kernel.org>
+Junio C Hamano <junio@kernel.org>
+Junio C Hamano <junio@pobox.com>
+...
+
+If you want to add a `Helped-by` trailer with Junio "name <email>" pair,
+you can use:
+
+`git commit --trailers "Helped-by:@Junio"`
+
+to insert the trailer to your commit messages:
+
+Helped-by: Junio C Hamano <gitster@pobox.com>
+
 OPTIONS
 -------
 --in-place::
diff --git a/builtin/commit.c b/builtin/commit.c
index 4b06672bd07d..58c020e3cfbf 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1043,39 +1043,6 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 	return 1;
 }
 
-static const char *find_author_by_nickname(const char *name)
-{
-	struct rev_info revs;
-	struct commit *commit;
-	struct strbuf buf = STRBUF_INIT;
-	struct string_list mailmap = STRING_LIST_INIT_NODUP;
-	const char *av[20];
-	int ac = 0;
-
-	repo_init_revisions(the_repository, &revs, NULL);
-	strbuf_addf(&buf, "--author=%s", name);
-	av[++ac] = "--all";
-	av[++ac] = "-i";
-	av[++ac] = buf.buf;
-	av[++ac] = NULL;
-	setup_revisions(ac, av, &revs, NULL);
-	revs.mailmap = &mailmap;
-	read_mailmap(revs.mailmap);
-
-	if (prepare_revision_walk(&revs))
-		die(_("revision walk setup failed"));
-	commit = get_revision(&revs);
-	if (commit) {
-		struct pretty_print_context ctx = {0};
-		ctx.date_mode.type = DATE_NORMAL;
-		strbuf_release(&buf);
-		format_commit_message(commit, "%aN <%aE>", &buf, &ctx);
-		clear_mailmap(&mailmap);
-		return strbuf_detach(&buf, NULL);
-	}
-	die(_("--author '%s' is not 'Name <email>' and matches no existing author"), name);
-}
-
 static void handle_ignored_arg(struct wt_status *s)
 {
 	if (!ignored_arg)
diff --git a/commit.c b/commit.c
index 6ccd774841c6..e1aad52d2c4f 100644
--- a/commit.c
+++ b/commit.c
@@ -21,6 +21,7 @@
 #include "commit-reach.h"
 #include "run-command.h"
 #include "shallow.h"
+#include "mailmap.h"
 
 static struct commit_extra_header *read_commit_extra_header_lines(const char *buf, size_t len, const char **);
 
@@ -1703,3 +1704,36 @@ int run_commit_hook(int editor_is_used, const char *index_file,
 
 	return ret;
 }
+
+const char *find_author_by_nickname(const char *name)
+{
+	struct rev_info revs;
+	struct commit *commit;
+	struct strbuf buf = STRBUF_INIT;
+	struct string_list mailmap = STRING_LIST_INIT_NODUP;
+	const char *av[20];
+	int ac = 0;
+
+	repo_init_revisions(the_repository, &revs, NULL);
+	strbuf_addf(&buf, "--author=%s", name);
+	av[++ac] = "--all";
+	av[++ac] = "-i";
+	av[++ac] = buf.buf;
+	av[++ac] = NULL;
+	setup_revisions(ac, av, &revs, NULL);
+	revs.mailmap = &mailmap;
+	read_mailmap(revs.mailmap);
+
+	if (prepare_revision_walk(&revs))
+		die(_("revision walk setup failed"));
+	commit = get_revision(&revs);
+	if (commit) {
+		struct pretty_print_context ctx = {0};
+		ctx.date_mode.type = DATE_NORMAL;
+		strbuf_release(&buf);
+		format_commit_message(commit, "%aN <%aE>", &buf, &ctx);
+		clear_mailmap(&mailmap);
+		return strbuf_detach(&buf, NULL);
+	}
+	die(_("--author '%s' is not 'Name <email>' and matches no existing author"), name);
+}
diff --git a/commit.h b/commit.h
index 49c0f503964e..cb9f7cd13f09 100644
--- a/commit.h
+++ b/commit.h
@@ -370,5 +370,15 @@ int parse_buffer_signed_by_header(const char *buffer,
 				  struct strbuf *payload,
 				  struct strbuf *signature,
 				  const struct git_hash_algo *algop);
+/*
+ * Calling `find_author_by_nickname` to find the "author <email>" pair
+ * in the most recent commit which matches "--author=name".
+ *
+ * Note that `find_author_by_nickname` is not reusable, because it haven't
+ * reset flags for parsed objects. The only safe way to use `find_author_by_nickname`
+ * (without rewriting the revision traversal machinery) is to spawn a
+ * subprocess and do find_author_by_nickname() in it.
+ */
+const char *find_author_by_nickname(const char *name);
 
 #endif /* COMMIT_H */
diff --git a/t/t7502-commit-porcelain.sh b/t/t7502-commit-porcelain.sh
index 74b1602c0ce6..143690e2833c 100755
--- a/t/t7502-commit-porcelain.sh
+++ b/t/t7502-commit-porcelain.sh
@@ -445,6 +445,34 @@ test_expect_success 'commit --trailer with -c and ":=#" as separators' '
 	test_cmp expected actual
 '
 
+
+test_expect_success 'commit --trailer parse @nickname' '
+	echo "I love git" >file1 &&
+	git add file1 &&
+	git commit -m "yly" --author="batman <email1>" &&
+	echo "I love git" >file2 &&
+	git add file2 &&
+	git commit -m "yly" --author="jocker <email2>" &&
+	echo "I love git" >file3 &&
+	git add file3 &&
+	git commit -m "yly" \
+	--trailer "Reviewed-by:@bat" \
+	--trailer "Signed-off-by:@jock" \
+	--trailer "Helped-by:@email1" \
+	--trailer "Mentored-by:@email2" &&
+	git cat-file commit HEAD >commit.msg &&
+	sed -e "1,/^\$/d" commit.msg >actual &&
+	cat >expected <<-\EOF &&
+	yly
+
+	Reviewed-by: batman <email1>
+	Signed-off-by: jocker <email2>
+	Helped-by: batman <email1>
+	Mentored-by: jocker <email2>
+	EOF
+	test_cmp expected actual
+'
+
 test_expect_success 'multiple -m' '
 
 	>negative &&
diff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh
index 6602790b5f4c..f2f1ae3b2faf 100755
--- a/t/t7513-interpret-trailers.sh
+++ b/t/t7513-interpret-trailers.sh
@@ -63,6 +63,29 @@ test_expect_success 'without config' '
 	test_cmp expected actual
 '
 
+test_expect_success 'trailer parse @nickname' '
+	echo "I love git" >file1 &&
+	git add file1 &&
+	git commit -m "yly" --author="batman <email1>" &&
+	echo "I love git" >file2 &&
+	git add file2 &&
+	git commit -m "yly" --author="jocker <email2>" &&
+	git interpret-trailers \
+	--trailer "Reviewed-by:@bat" \
+	--trailer "Signed-off-by:@jock" \
+	--trailer "Helped-by:@email1" \
+	--trailer "Mentored-by:@email2" \
+	empty >actual &&
+	cat >expected <<-\EOF &&
+
+	Reviewed-by: batman <email1>
+	Signed-off-by: jocker <email2>
+	Helped-by: batman <email1>
+	Mentored-by: jocker <email2>
+	EOF
+	test_cmp expected actual
+'
+
 test_expect_success 'without config in another order' '
 	sed -e "s/ Z\$/ /" >expected <<-\EOF &&
 
diff --git a/trailer.c b/trailer.c
index 249ed618ed8e..21f367e7b761 100644
--- a/trailer.c
+++ b/trailer.c
@@ -6,6 +6,7 @@
 #include "tempfile.h"
 #include "trailer.h"
 #include "list.h"
+#include "revision.h"
 /*
  * Copyright (c) 2013, 2014 Christian Couder <chriscool@tuxfamily.org>
  */
@@ -633,11 +634,21 @@ static void parse_trailer(struct strbuf *tok, struct strbuf *val,
 	struct arg_item *item;
 	size_t tok_len;
 	struct list_head *pos;
+	const char *ae = NULL;
 
 	if (separator_pos != -1) {
 		strbuf_add(tok, trailer, separator_pos);
 		strbuf_trim(tok);
-		strbuf_addstr(val, trailer + separator_pos + 1);
+		if (trailer[separator_pos + 1] == '@') {
+			ae = find_author_by_nickname(trailer + separator_pos + 2);
+			reset_revision_walk();
+			if (ae) {
+				strbuf_addstr(val, ae);
+				free((char*)ae);
+			} else
+				strbuf_addstr(val, trailer + separator_pos + 1);
+		} else
+			strbuf_addstr(val, trailer + separator_pos + 1);
 		strbuf_trim(val);
 	} else {
 		strbuf_addstr(tok, trailer);
-- 
gitgitgadget

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [PATCH v2 2/2] [GSOC] interpret-trailer: easy parse trailer value
  2021-03-21  8:58   ` [PATCH v2 2/2] [GSOC] interpret-trailer: easy parse trailer value ZheNing Hu via GitGitGadget
@ 2021-03-21  9:17     ` Bagas Sanjaya
  2021-03-21 14:04       ` ZheNing Hu
  2021-03-21 16:49       ` Junio C Hamano
  2021-03-21 13:15     ` Christian Couder
  1 sibling, 2 replies; 23+ messages in thread
From: Bagas Sanjaya @ 2021-03-21  9:17 UTC (permalink / raw)
  To: ZheNing Hu via GitGitGadget
  Cc: Bradley M. Kuhn, Junio C Hamano, Brandon Casey, Shourya Shukla,
	Christian Couder, Rafael Silva,
	Đoàn Trần Công Danh, Jeff King,
	Ævar Arnfjörð Bjarmason, ZheNing Hu, git


On 21/03/21 15.58, ZheNing Hu via GitGitGadget wrote:
> +test_expect_success 'commit --trailer parse @nickname' '
> +	echo "I love git" >file1 &&
> +	git add file1 &&
> +	git commit -m "yly" --author="batman <email1>" &&
> +	echo "I love git" >file2 &&
> +	git add file2 &&
> +	git commit -m "yly" --author="jocker <email2>" &&
> +	echo "I love git" >file3 &&
> +	git add file3 &&
> +	git commit -m "yly" \
> +	--trailer "Reviewed-by:@bat" \
> +	--trailer "Signed-off-by:@jock" \
> +	--trailer "Helped-by:@email1" \
> +	--trailer "Mentored-by:@email2" &&
> +	git cat-file commit HEAD >commit.msg &&
> +	sed -e "1,/^\$/d" commit.msg >actual &&
> +	cat >expected <<-\EOF &&
> +	yly
> +
> +	Reviewed-by: batman <email1>
> +	Signed-off-by: jocker <email2>
> +	Helped-by: batman <email1>
> +	Mentored-by: jocker <email2>
> +	EOF
> +	test_cmp expected actual
> +'
> +
>   test_expect_success 'multiple -m' '
>   
>   	>negative &&
> +test_expect_success 'trailer parse @nickname' '
> +	echo "I love git" >file1 &&
> +	git add file1 &&
> +	git commit -m "yly" --author="batman <email1>" &&
> +	echo "I love git" >file2 &&
> +	git add file2 &&
> +	git commit -m "yly" --author="jocker <email2>" &&
> +	git interpret-trailers \
> +	--trailer "Reviewed-by:@bat" \
> +	--trailer "Signed-off-by:@jock" \
> +	--trailer "Helped-by:@email1" \
> +	--trailer "Mentored-by:@email2" \
> +	empty >actual &&
> +	cat >expected <<-\EOF &&
> +
> +	Reviewed-by: batman <email1>
> +	Signed-off-by: jocker <email2>
> +	Helped-by: batman <email1>
> +	Mentored-by: jocker <email2>
> +	EOF
> +	test_cmp expected actual
> +'
> +
>   test_expect_success 'without config in another order' '
>   	sed -e "s/ Z\$/ /" >expected <<-\EOF &&
I think please consider this case: When I add --trailer "Reviewed-by:@bat", and there are two
identity pairs that match (`batman <email1>` and `batman <email2>`), I need to choose one that
will be in the trailer (for example because <email1> is primary email). So a disambiguation
prompt should be added, something like:

```
There are <N> identities that match, please choose one that will be added to the trailer:
1) batman <email1>
2) batman <email2>
...
n) batman <emailn>
```

The prompt can be repeated for each trailer values that are non-unique.

Thanks

-- 
An old man doll... just what I always wanted! - Clara

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2 2/2] [GSOC] interpret-trailer: easy parse trailer value
  2021-03-21  8:58   ` [PATCH v2 2/2] [GSOC] interpret-trailer: easy parse trailer value ZheNing Hu via GitGitGadget
  2021-03-21  9:17     ` Bagas Sanjaya
@ 2021-03-21 13:15     ` Christian Couder
  2021-03-21 13:49       ` ZheNing Hu
  2021-03-21 16:52       ` Junio C Hamano
  1 sibling, 2 replies; 23+ messages in thread
From: Christian Couder @ 2021-03-21 13:15 UTC (permalink / raw)
  To: ZheNing Hu via GitGitGadget
  Cc: git, Bradley M. Kuhn, Junio C Hamano, Brandon Casey,
	Shourya Shukla, Rafael Silva,
	Đoàn Trần Công Danh, Jeff King,
	Ævar Arnfjörð Bjarmason, ZheNing Hu

On Sun, Mar 21, 2021 at 9:58 AM ZheNing Hu via GitGitGadget
<gitgitgadget@gmail.com> wrote:
>
> From: ZheNing Hu <adlternative@gmail.com>
>
> The original `-trailer` adding some trailers like

s/-trailer/--trailer/

> "Signed-off-by:C O <Mister@email.com>" is often too
> verbose and error-prone.

The 'trailer.<token>.command' config option can already be used to
help with that, for example:

-------
$ git config trailer.sign.key "Signed-off-by: "
$ git config trailer.sign.ifexists replace
$ git config trailer.sign.command "git log --author='\$ARG' -1
--format='format:%aN <%aE>'"
$ git interpret-trailers --trailer sign=Linus<<EOF
subject

body
EOF
subject

body

Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
-------

So even without this patch, after your first patch that implements
`git commit --trailer ...`, it should be easy to setup something less
verbose and less error-prone.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2 2/2] [GSOC] interpret-trailer: easy parse trailer value
  2021-03-21 13:15     ` Christian Couder
@ 2021-03-21 13:49       ` ZheNing Hu
  2021-03-21 16:52       ` Junio C Hamano
  1 sibling, 0 replies; 23+ messages in thread
From: ZheNing Hu @ 2021-03-21 13:49 UTC (permalink / raw)
  To: Christian Couder
  Cc: ZheNing Hu via GitGitGadget, git, Bradley M. Kuhn,
	Junio C Hamano, Brandon Casey, Shourya Shukla, Rafael Silva,
	Đoàn Trần Công Danh, Jeff King,
	Ævar Arnfjörð Bjarmason

Christian Couder <christian.couder@gmail.com> 于2021年3月21日周日 下午9:15写道:
>
> On Sun, Mar 21, 2021 at 9:58 AM ZheNing Hu via GitGitGadget
> <gitgitgadget@gmail.com> wrote:
> >
> > From: ZheNing Hu <adlternative@gmail.com>
> >
> > The original `-trailer` adding some trailers like
>
> s/-trailer/--trailer/
>
> > "Signed-off-by:C O <Mister@email.com>" is often too
> > verbose and error-prone.
>
> The 'trailer.<token>.command' config option can already be used to
> help with that, for example:
>
> -------
> $ git config trailer.sign.key "Signed-off-by: "
> $ git config trailer.sign.ifexists replace
> $ git config trailer.sign.command "git log --author='\$ARG' -1
> --format='format:%aN <%aE>'"
> $ git interpret-trailers --trailer sign=Linus<<EOF
> subject
>
> body
> EOF
> subject
>
> body
>
> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
> -------
>
> So even without this patch, after your first patch that implements
> `git commit --trailer ...`, it should be easy to setup something less
> verbose and less error-prone.

Hey, Christian,
`@nickname` can provides a "quick" way for those users
who haven't config as you mention. I hope I am not wrong.

--
ZheNing Hu

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 2/2] [GSOC] interpret-trailer: easy parse trailer value
  2021-03-21  3:29     ` ZheNing Hu
@ 2021-03-21 13:57       ` Junio C Hamano
  2021-03-21 14:12         ` ZheNing Hu
  0 siblings, 1 reply; 23+ messages in thread
From: Junio C Hamano @ 2021-03-21 13:57 UTC (permalink / raw)
  To: ZheNing Hu
  Cc: ZheNing Hu via GitGitGadget, Git List, Bradley M. Kuhn,
	Brandon Casey, Shourya Shukla, Christian Couder, Rafael Silva,
	Đoàn Trần Công Danh, Jeff King,
	Ævar Arnfjörð Bjarmason

ZheNing Hu <adlternative@gmail.com> writes:

> Do you think this is appropriate?
>
> @@ -370,5 +370,15 @@ int parse_buffer_signed_by_header(const char *buffer,
>                                   struct strbuf *payload,
>                                   struct strbuf *signature,
>                                   const struct git_hash_algo *algop);
> +/*
> + * Calling `find_author_by_nickname` to find the "author <email>" pair
> + * in the most recent commit which matches "--author=name".
> + *
> + * Note that `find_author_by_nickname` is not reusable, because it haven't
> + * reset flags for parsed objects. The only safe way to use
> `find_author_by_nickname`
> + * (without rewriting the revision traversal machinery) is to spawn a
> + * subprocess and do find_author_by_nickname() in it.
> + */

Telling people not to add any new caller is good, but everything
after "because" does not make sense to me.

I do not think calling find_author_by_nickname() in a subprocess
alone would not help somebody who wants to do this, either.  We'd be
doing a moral equivalent of that call, but the result has to be
communicated back to the parent process,

In the longer term, we'd probably want to have a pre-computed table
of contributors, like we have precomputed files for reachability
bitmaps, commit DAG topology, and such, but that is obviously far
outside of the scope of this series.

> +const char *find_author_by_nickname(const char *name);

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2 2/2] [GSOC] interpret-trailer: easy parse trailer value
  2021-03-21  9:17     ` Bagas Sanjaya
@ 2021-03-21 14:04       ` ZheNing Hu
  2021-03-21 16:49       ` Junio C Hamano
  1 sibling, 0 replies; 23+ messages in thread
From: ZheNing Hu @ 2021-03-21 14:04 UTC (permalink / raw)
  To: Bagas Sanjaya
  Cc: ZheNing Hu via GitGitGadget, Bradley M. Kuhn, Junio C Hamano,
	Brandon Casey, Shourya Shukla, Christian Couder, Rafael Silva,
	Đoàn Trần Công Danh, Jeff King,
	Ævar Arnfjörð Bjarmason, Git List

> I think please consider this case: When I add --trailer "Reviewed-by:@bat", and there are two
> identity pairs that match (`batman <email1>` and `batman <email2>`), I need to choose one that
> will be in the trailer (for example because <email1> is primary email). So a disambiguation
> prompt should be added, something like:
>
> ```
> There are <N> identities that match, please choose one that will be added to the trailer:
> 1) batman <email1>
> 2) batman <email2>
> ...
> n) batman <emailn>
> ```
>
Hi,

I think this kind of prompt will be beneficial to users, temporarily
adopt the method of
`find_author_by_nickname()` to get the last match commit "name <eamil>" pair.
If I want to find all these matched "name <eamil>" pairs and exclude
duplicates, I may need to
think about it for a while.

> The prompt can be repeated for each trailer values that are non-unique.
>
> Thanks
>
> --
> An old man doll... just what I always wanted! - Clara

Thanks.
--
ZheNing Hu

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 2/2] [GSOC] interpret-trailer: easy parse trailer value
  2021-03-21 13:57       ` Junio C Hamano
@ 2021-03-21 14:12         ` ZheNing Hu
  0 siblings, 0 replies; 23+ messages in thread
From: ZheNing Hu @ 2021-03-21 14:12 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: ZheNing Hu via GitGitGadget, Git List, Bradley M. Kuhn,
	Brandon Casey, Shourya Shukla, Christian Couder, Rafael Silva,
	Đoàn Trần Công Danh, Jeff King,
	Ævar Arnfjörð Bjarmason

Junio C Hamano <gitster@pobox.com> 于2021年3月21日周日 下午9:57写道:
>
> ZheNing Hu <adlternative@gmail.com> writes:
>
> > Do you think this is appropriate?
> >
> > @@ -370,5 +370,15 @@ int parse_buffer_signed_by_header(const char *buffer,
> >                                   struct strbuf *payload,
> >                                   struct strbuf *signature,
> >                                   const struct git_hash_algo *algop);
> > +/*
> > + * Calling `find_author_by_nickname` to find the "author <email>" pair
> > + * in the most recent commit which matches "--author=name".
> > + *
> > + * Note that `find_author_by_nickname` is not reusable, because it haven't
> > + * reset flags for parsed objects. The only safe way to use
> > `find_author_by_nickname`
> > + * (without rewriting the revision traversal machinery) is to spawn a
> > + * subprocess and do find_author_by_nickname() in it.
> > + */
>
> Telling people not to add any new caller is good, but everything
> after "because" does not make sense to me.
>
> I do not think calling find_author_by_nickname() in a subprocess
> alone would not help somebody who wants to do this, either.  We'd be
> doing a moral equivalent of that call, but the result has to be
> communicated back to the parent process,
>

What I am thinking about here is that `commit --trailer` itself jumps to a
sub-process to do this, but this does depend on the fact that
`interpret-trailers`
 itself does not have a traversal, and indeed should not be arbitrarily call it.

> In the longer term, we'd probably want to have a pre-computed table
> of contributors, like we have precomputed files for reachability
> bitmaps, commit DAG topology, and such, but that is obviously far
> outside of the scope of this series.
>

Indeed this will be a very big project. But `.mailmap` always makes me
feel similar.

> > +const char *find_author_by_nickname(const char *name);

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2 2/2] [GSOC] interpret-trailer: easy parse trailer value
  2021-03-21  9:17     ` Bagas Sanjaya
  2021-03-21 14:04       ` ZheNing Hu
@ 2021-03-21 16:49       ` Junio C Hamano
  2021-03-22  6:01         ` Bagas Sanjaya
  1 sibling, 1 reply; 23+ messages in thread
From: Junio C Hamano @ 2021-03-21 16:49 UTC (permalink / raw)
  To: Bagas Sanjaya
  Cc: ZheNing Hu via GitGitGadget, Bradley M. Kuhn, Brandon Casey,
	Shourya Shukla, Christian Couder, Rafael Silva,
	Đoàn Trần Công Danh, Jeff King,
	Ævar Arnfjörð Bjarmason, ZheNing Hu, git

Bagas Sanjaya <bagasdotme@gmail.com> writes:

> will be in the trailer (for example because <email1> is primary email). So a disambiguation
> prompt should be added, something like:

No, it just should error out without going interactive.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2 2/2] [GSOC] interpret-trailer: easy parse trailer value
  2021-03-21 13:15     ` Christian Couder
  2021-03-21 13:49       ` ZheNing Hu
@ 2021-03-21 16:52       ` Junio C Hamano
  2021-03-22  0:39         ` ZheNing Hu
  1 sibling, 1 reply; 23+ messages in thread
From: Junio C Hamano @ 2021-03-21 16:52 UTC (permalink / raw)
  To: Christian Couder
  Cc: ZheNing Hu via GitGitGadget, git, Bradley M. Kuhn, Brandon Casey,
	Shourya Shukla, Rafael Silva,
	Đoàn Trần Công Danh, Jeff King,
	Ævar Arnfjörð Bjarmason, ZheNing Hu

Christian Couder <christian.couder@gmail.com> writes:

>> "Signed-off-by:C O <Mister@email.com>" is often too
>> verbose and error-prone.
>
> The 'trailer.<token>.command' config option can already be used to
> help with that, for example:
>
> -------
> $ git config trailer.sign.key "Signed-off-by: "
> $ git config trailer.sign.ifexists replace
> $ git config trailer.sign.command "git log --author='\$ARG' -1
> --format='format:%aN <%aE>'"
> $ git interpret-trailers --trailer sign=Linus<<EOF
> subject
>
> body
> EOF
> subject
>
> body
>
> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
> -------
>
> So even without this patch, after your first patch that implements
> `git commit --trailer ...`, it should be easy to setup something less
> verbose and less error-prone.

It is nice that it makes the complexity of 2/2 unnecessary ;-)

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2 2/2] [GSOC] interpret-trailer: easy parse trailer value
  2021-03-21 16:52       ` Junio C Hamano
@ 2021-03-22  0:39         ` ZheNing Hu
  2021-03-22  7:26           ` Christian Couder
  2021-03-22 17:07           ` Junio C Hamano
  0 siblings, 2 replies; 23+ messages in thread
From: ZheNing Hu @ 2021-03-22  0:39 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Christian Couder, ZheNing Hu via GitGitGadget, git,
	Bradley M. Kuhn, Brandon Casey, Shourya Shukla, Rafael Silva,
	Đoàn Trần Công Danh, Jeff King,
	Ævar Arnfjörð Bjarmason

Junio C Hamano <gitster@pobox.com> 于2021年3月22日周一 上午12:52写道:
>
> Christian Couder <christian.couder@gmail.com> writes:
>
> >> "Signed-off-by:C O <Mister@email.com>" is often too
> >> verbose and error-prone.
> >
> > The 'trailer.<token>.command' config option can already be used to
> > help with that, for example:
> >
> > -------
> > $ git config trailer.sign.key "Signed-off-by: "
> > $ git config trailer.sign.ifexists replace
> > $ git config trailer.sign.command "git log --author='\$ARG' -1
> > --format='format:%aN <%aE>'"
> > $ git interpret-trailers --trailer sign=Linus<<EOF
> > subject
> >
> > body
> > EOF
> > subject
> >
> > body
> >
> > Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
> > -------
> >
> > So even without this patch, after your first patch that implements
> > `git commit --trailer ...`, it should be easy to setup something less
> > verbose and less error-prone.
>
> It is nice that it makes the complexity of 2/2 unnecessary ;-)

A little frustrated, both `--own-identity` and `@nickname` seem to be
 rejected. I will roll back to the first patch.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2 2/2] [GSOC] interpret-trailer: easy parse trailer value
  2021-03-21 16:49       ` Junio C Hamano
@ 2021-03-22  6:01         ` Bagas Sanjaya
  2021-03-22 18:45           ` Junio C Hamano
  0 siblings, 1 reply; 23+ messages in thread
From: Bagas Sanjaya @ 2021-03-22  6:01 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: ZheNing Hu via GitGitGadget, Bradley M. Kuhn, Brandon Casey,
	Shourya Shukla, Christian Couder, Rafael Silva,
	Đoàn Trần Công Danh, Jeff King,
	Ævar Arnfjörð Bjarmason, ZheNing Hu, git

On 21/03/21 23.49, Junio C Hamano wrote:
> Bagas Sanjaya <bagasdotme@gmail.com> writes:
> 
>> will be in the trailer (for example because <email1> is primary email). So a disambiguation
>> prompt should be added, something like:
> 
> No, it just should error out without going interactive.
>

I mean the error should be:

```
error: @batman match multiple identities:
1) batman <email1>
2) batman <email2>
...
n) batman <emailn>

Please disambiguate by running "git commit --trailer" with full identity, like:
     git commit --trailer="<someone> <<email>>" <options>...

```

-- 
An old man doll... just what I always wanted! - Clara

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2 2/2] [GSOC] interpret-trailer: easy parse trailer value
  2021-03-22  0:39         ` ZheNing Hu
@ 2021-03-22  7:26           ` Christian Couder
  2021-03-22  8:28             ` ZheNing Hu
  2021-03-22 17:07           ` Junio C Hamano
  1 sibling, 1 reply; 23+ messages in thread
From: Christian Couder @ 2021-03-22  7:26 UTC (permalink / raw)
  To: ZheNing Hu
  Cc: Junio C Hamano, ZheNing Hu via GitGitGadget, git,
	Bradley M. Kuhn, Brandon Casey, Shourya Shukla, Rafael Silva,
	Đoàn Trần Công Danh, Jeff King,
	Ævar Arnfjörð Bjarmason

On Mon, Mar 22, 2021 at 1:39 AM ZheNing Hu <adlternative@gmail.com> wrote:
>
> Junio C Hamano <gitster@pobox.com> 于2021年3月22日周一 上午12:52写道:
> >
> > Christian Couder <christian.couder@gmail.com> writes:

> > > So even without this patch, after your first patch that implements
> > > `git commit --trailer ...`, it should be easy to setup something less
> > > verbose and less error-prone.
> >
> > It is nice that it makes the complexity of 2/2 unnecessary ;-)
>
> A little frustrated, both `--own-identity` and `@nickname` seem to be
>  rejected. I will roll back to the first patch.

It's good to want to implement new features, and to come up quickly
with patches, but it's also good to give people time on this list to
discuss the goals you would like to achieve, and which features are
already available or should be implement to achieve the goals.

Thanks for working on this topic anyway!

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2 2/2] [GSOC] interpret-trailer: easy parse trailer value
  2021-03-22  7:26           ` Christian Couder
@ 2021-03-22  8:28             ` ZheNing Hu
  0 siblings, 0 replies; 23+ messages in thread
From: ZheNing Hu @ 2021-03-22  8:28 UTC (permalink / raw)
  To: Christian Couder
  Cc: Junio C Hamano, ZheNing Hu via GitGitGadget, git,
	Bradley M. Kuhn, Brandon Casey, Shourya Shukla, Rafael Silva,
	Đoàn Trần Công Danh, Jeff King,
	Ævar Arnfjörð Bjarmason

Christian Couder <christian.couder@gmail.com> 于2021年3月22日周一 下午3:26写道:
>
> On Mon, Mar 22, 2021 at 1:39 AM ZheNing Hu <adlternative@gmail.com> wrote:
> >
> > Junio C Hamano <gitster@pobox.com> 于2021年3月22日周一 上午12:52写道:
> > >
> > > Christian Couder <christian.couder@gmail.com> writes:
>
> > > > So even without this patch, after your first patch that implements
> > > > `git commit --trailer ...`, it should be easy to setup something less
> > > > verbose and less error-prone.
> > >
> > > It is nice that it makes the complexity of 2/2 unnecessary ;-)
> >
> > A little frustrated, both `--own-identity` and `@nickname` seem to be
> >  rejected. I will roll back to the first patch.
>
> It's good to want to implement new features, and to come up quickly
> with patches, but it's also good to give people time on this list to
> discuss the goals you would like to achieve, and which features are
> already available or should be implement to achieve the goals.
>

I basically started coding after receiving the first reviewer's comment.
It seems that I should be more patient.

> Thanks for working on this topic anyway!

Thanks, Christian.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2 2/2] [GSOC] interpret-trailer: easy parse trailer value
  2021-03-22  0:39         ` ZheNing Hu
  2021-03-22  7:26           ` Christian Couder
@ 2021-03-22 17:07           ` Junio C Hamano
  2021-03-23  5:04             ` ZheNing Hu
  1 sibling, 1 reply; 23+ messages in thread
From: Junio C Hamano @ 2021-03-22 17:07 UTC (permalink / raw)
  To: ZheNing Hu
  Cc: Christian Couder, ZheNing Hu via GitGitGadget, git,
	Bradley M. Kuhn, Brandon Casey, Shourya Shukla, Rafael Silva,
	Đoàn Trần Công Danh, Jeff King,
	Ævar Arnfjörð Bjarmason

ZheNing Hu <adlternative@gmail.com> writes:

> Junio C Hamano <gitster@pobox.com> 于2021年3月22日周一 上午12:52写道:
>>
>> Christian Couder <christian.couder@gmail.com> writes:
>> ...
>> > So even without this patch, after your first patch that implements
>> > `git commit --trailer ...`, it should be easy to setup something less
>> > verbose and less error-prone.
>>
>> It is nice that it makes the complexity of 2/2 unnecessary ;-)
>
> A little frustrated, both `--own-identity` and `@nickname` seem to be
>  rejected. I will roll back to the first patch.

If existing functionality can be used to achieve the same end result
without any extra effort by end-users, that's a happy endgame, isn't
it?  There isn't a reason to be frustrated---the users benefit with
the new --trailer option without adding (hence having to learn) new
extra features like --own-ident or @name, which is a big plus.



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2 2/2] [GSOC] interpret-trailer: easy parse trailer value
  2021-03-22  6:01         ` Bagas Sanjaya
@ 2021-03-22 18:45           ` Junio C Hamano
  0 siblings, 0 replies; 23+ messages in thread
From: Junio C Hamano @ 2021-03-22 18:45 UTC (permalink / raw)
  To: Bagas Sanjaya
  Cc: ZheNing Hu via GitGitGadget, Bradley M. Kuhn, Brandon Casey,
	Shourya Shukla, Christian Couder, Rafael Silva,
	Đoàn Trần Công Danh, Jeff King,
	Ævar Arnfjörð Bjarmason, ZheNing Hu, git

Bagas Sanjaya <bagasdotme@gmail.com> writes:

> I mean the error should be:
>
> ```
> error: @batman match multiple identities:
> 1) batman <email1>
> 2) batman <email2>
> ...
> n) batman <emailn>
>
> Please disambiguate by running "git commit --trailer" with full identity, like:
>     git commit --trailer="<someone> <<email>>" <options>...
>
> ```

Detecting error alone is probably prohibitively expensive, as it
takes you to scan _all_ commits down to the beginning of time to
pruve that a given pattern matches one and only one author ident.

Assuming that we would pay that cost (I doubt we would want to,
though), additional cost to show all possible hits would be small
and manageable, I think.


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2 2/2] [GSOC] interpret-trailer: easy parse trailer value
  2021-03-22 17:07           ` Junio C Hamano
@ 2021-03-23  5:04             ` ZheNing Hu
  0 siblings, 0 replies; 23+ messages in thread
From: ZheNing Hu @ 2021-03-23  5:04 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Christian Couder, ZheNing Hu via GitGitGadget, git,
	Bradley M. Kuhn, Brandon Casey, Shourya Shukla, Rafael Silva,
	Đoàn Trần Công Danh, Jeff King,
	Ævar Arnfjörð Bjarmason

Junio C Hamano <gitster@pobox.com> 于2021年3月23日周二 上午1:07写道:
>
> ZheNing Hu <adlternative@gmail.com> writes:
>
> > Junio C Hamano <gitster@pobox.com> 于2021年3月22日周一 上午12:52写道:
> >>
> >> Christian Couder <christian.couder@gmail.com> writes:
> >> ...
> >> > So even without this patch, after your first patch that implements
> >> > `git commit --trailer ...`, it should be easy to setup something less
> >> > verbose and less error-prone.
> >>
> >> It is nice that it makes the complexity of 2/2 unnecessary ;-)
> >
> > A little frustrated, both `--own-identity` and `@nickname` seem to be
> >  rejected. I will roll back to the first patch.
>
> If existing functionality can be used to achieve the same end result
> without any extra effort by end-users, that's a happy endgame, isn't
> it?  There isn't a reason to be frustrated---the users benefit with
> the new --trailer option without adding (hence having to learn) new
> extra features like --own-ident or @name, which is a big plus.
>
>

Thanks, Junio, you are right :)

^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2021-03-23  5:05 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-20 14:41 [PATCH 0/2] [GSOC] interpret-trailer: easy parse trailer value ZheNing Hu via GitGitGadget
2021-03-20 14:41 ` [PATCH 1/2] [GSOC] commit: add --trailer option ZheNing Hu via GitGitGadget
2021-03-20 14:41 ` [PATCH 2/2] [GSOC] interpret-trailer: easy parse trailer value ZheNing Hu via GitGitGadget
2021-03-20 21:06   ` Junio C Hamano
2021-03-21  3:29     ` ZheNing Hu
2021-03-21 13:57       ` Junio C Hamano
2021-03-21 14:12         ` ZheNing Hu
2021-03-21  8:58 ` [PATCH v2 0/2] " ZheNing Hu via GitGitGadget
2021-03-21  8:58   ` [PATCH v2 1/2] [GSOC] commit: add --trailer option ZheNing Hu via GitGitGadget
2021-03-21  8:58   ` [PATCH v2 2/2] [GSOC] interpret-trailer: easy parse trailer value ZheNing Hu via GitGitGadget
2021-03-21  9:17     ` Bagas Sanjaya
2021-03-21 14:04       ` ZheNing Hu
2021-03-21 16:49       ` Junio C Hamano
2021-03-22  6:01         ` Bagas Sanjaya
2021-03-22 18:45           ` Junio C Hamano
2021-03-21 13:15     ` Christian Couder
2021-03-21 13:49       ` ZheNing Hu
2021-03-21 16:52       ` Junio C Hamano
2021-03-22  0:39         ` ZheNing Hu
2021-03-22  7:26           ` Christian Couder
2021-03-22  8:28             ` ZheNing Hu
2021-03-22 17:07           ` Junio C Hamano
2021-03-23  5:04             ` ZheNing Hu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).