| Message ID | ca521d3c01d652e09e716fb447b0b26da1a014e8.1616251299.git.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | interpret-trailer: easy parse trailer value | expand |
"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.
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.
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);
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);
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);