| Message ID | pull.1723.v5.git.1714934950.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | builtin/tag.c: add --trailer option | expand |
On Sun, May 05, 2024 at 06:49:07PM +0000, John Passaro via GitGitGadget wrote: > 5th follow-up patch taking welcome feedback from Patrick and JCH. Net new > changes include suggested tweaks in documentation, error messaging, code > formatting, and patch description. > > Since git-tag --list --format="%(trailers)" can interpret trailers from > annotated tag messages, it seems natural to support --trailer when writing a > new tag message. > > git-commit accomplishes this by taking --trailer arguments and passing them > to git-interpret-trailer. This patch series refactors that logic and uses it > to implement --trailer on git-tag. This version looks good to me, thanks! Patrick
Patrick Steinhardt <ps@pks.im> writes: > On Sun, May 05, 2024 at 06:49:07PM +0000, John Passaro via GitGitGadget wrote: >> 5th follow-up patch taking welcome feedback from Patrick and JCH. Net new >> changes include suggested tweaks in documentation, error messaging, code >> formatting, and patch description. >> >> Since git-tag --list --format="%(trailers)" can interpret trailers from >> annotated tag messages, it seems natural to support --trailer when writing a >> new tag message. >> >> git-commit accomplishes this by taking --trailer arguments and passing them >> to git-interpret-trailer. This patch series refactors that logic and uses it >> to implement --trailer on git-tag. > > This version looks good to me, thanks! Thanks. Will queue.
5th follow-up patch taking welcome feedback from Patrick and JCH. Net new changes include suggested tweaks in documentation, error messaging, code formatting, and patch description. Since git-tag --list --format="%(trailers)" can interpret trailers from annotated tag messages, it seems natural to support --trailer when writing a new tag message. git-commit accomplishes this by taking --trailer arguments and passing them to git-interpret-trailer. This patch series refactors that logic and uses it to implement --trailer on git-tag. John Passaro (3): builtin/commit: use ARGV macro to collect trailers builtin/commit: refactor --trailer logic builtin/tag: add --trailer option Documentation/git-tag.txt | 16 +++++- builtin/commit.c | 20 +------ builtin/tag.c | 38 ++++++++++--- t/t7004-tag.sh | 114 ++++++++++++++++++++++++++++++++++++++ trailer.c | 12 ++++ trailer.h | 9 +++ 6 files changed, 181 insertions(+), 28 deletions(-) base-commit: d4cc1ec35f3bcce816b69986ca41943f6ce21377 Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1723%2Fjpassaro%2Fjp%2Ftag-trailer-arg-v5 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1723/jpassaro/jp/tag-trailer-arg-v5 Pull-Request: https://github.com/gitgitgadget/git/pull/1723 Range-diff vs v4: 1: ce047c58aa8 ! 1: 85f45a57f35 builtin/commit.c: remove bespoke option callback @@ Metadata Author: John Passaro <john.a.passaro@gmail.com> ## Commit message ## - builtin/commit.c: remove bespoke option callback + builtin/commit: use ARGV macro to collect trailers - Replace git-commit's bespoke callback for --trailer with the standard - OPT_PASSTHRU_ARGV macro. The bespoke callback was only adding its values - to a strvec and sanity-checking that `unset` is always false; both of - these are already implemented in the parse-option API. + Replace git-commit's callback for --trailer with the standard + OPT_PASSTHRU_ARGV macro. The callback only adds its values to a strvec + and sanity-checking that `unset` is always false; both of these are + already implemented in the parse-option API. Signed-off-by: John Passaro <john.a.passaro@gmail.com> 2: 8f53a54bbfe ! 2: 75bb5cc0e8a builtin/commit.c: refactor --trailer logic @@ Metadata Author: John Passaro <john.a.passaro@gmail.com> ## Commit message ## - builtin/commit.c: refactor --trailer logic + builtin/commit: refactor --trailer logic git-commit adds user trailers to the commit message by passing its `--trailer` arguments to a child process running `git-interpret-trailers @@ trailer.c: void trailer_iterator_release(struct trailer_iterator *iter) strbuf_release(&iter->key); } + -+int amend_file_with_trailers(const char *path, const struct strvec *trailer_args) { ++int amend_file_with_trailers(const char *path, const struct strvec *trailer_args) ++{ + struct child_process run_trailer = CHILD_PROCESS_INIT; + + run_trailer.git_cmd = 1; 3: f1d68337eda ! 3: d8b3f6229bc builtin/tag.c: add --trailer option @@ Metadata Author: John Passaro <john.a.passaro@gmail.com> ## Commit message ## - builtin/tag.c: add --trailer option + builtin/tag: add --trailer option - git-tag currently supports interpreting trailers from an annotated tag - message, using --list --format="%(trailers)". However, to add a trailer - to a tag message, you must do so using `-F` or by editing the message. + git-tag supports interpreting trailers from an annotated tag message, + using --list --format="%(trailers)". However, the available methods to + add a trailer to a tag message (namely -F or --editor) are not as + ergonomic. In a previous patch, we moved git-commit's implementation of its --trailer option to the trailer.h API. Let's use that new function to @@ Documentation/git-tag.txt: This option is only applicable when listing tags with +--trailer <token>[(=|:)<value>]:: + Specify a (<token>, <value>) pair that should be applied as a -+ trailer. (e.g. `git tag --trailer "Signed-off-by:T A Ger \ -+ <tagger@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 tag message.) ++ trailer. (e.g. `git tag --trailer "Custom-Key: value"` ++ will add a "Custom-Key" trailer to the tag 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. -+ The trailers can be seen in `git tag --list` using ++ The trailers can be extracted in `git tag --list`, using + `--format="%(trailers)"` placeholder. + -e:: @@ builtin/tag.c: static void create_tag(const struct object_id *object, const char + } + } else if (trailer_args->nr) { + strbuf_reset(buf); -+ if (strbuf_read_file(buf, path, 0) < 0) { -+ fprintf(stderr, -+ _("Please supply the message using either -m or -F option.\n")); -+ exit(1); -+ } ++ if (strbuf_read_file(buf, path, 0) < 0) ++ die_errno(_("failed to read '%s'"), path); } }