Subject: [PATCH v4 0/3] builtin/tag.c: add --trailer option

From: "John Passaro via GitGitGadget" <[email-masked]>
Date: Tue, 30 Apr 2024 14:41:48 +0000

4th follow-up patch taking welcome feedback from Patrick and JCH. Net new
changes include separating from a 2-patch series to 3.

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.c: remove bespoke option callback
  builtin/commit.c: refactor --trailer logic
  builtin/tag.c: add --trailer option

 Documentation/git-tag.txt |  18 +++++-
 builtin/commit.c          |  20 +------
 builtin/tag.c             |  41 +++++++++++---
 t/t7004-tag.sh            | 114 ++++++++++++++++++++++++++++++++++++++
 trailer.c                 |  11 ++++
 trailer.h                 |   9 +++
 6 files changed, 185 insertions(+), 28 deletions(-)

base-commit: 786a3e4b8d754d2b14b1208b98eeb0a554ef19a8
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1723%2Fjpassaro%2Fjp%2Ftag-trailer-arg-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1723/jpassaro/jp/tag-trailer-arg-v4
Pull-Request: https://github.com/gitgitgadget/git/pull/1723

Range-diff vs v3:

 -:  ----------- > 1:  ce047c58aa8 builtin/commit.c: remove bespoke option callback
 1:  0c9517f434a ! 2:  8f53a54bbfe builtin/commit.c: refactor --trailer logic
     @@ Commit message
          Let's move this logic from git-commit to a new function in the trailer
          API, so that it can be re-used in other commands.
      
     -    Additionally, replace git-commit's bespoke callback for --trailer with
     -    the standard OPT_PASSTHRU_ARGV macro. This 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.
     -
     -    Signed-off-by: John Passaro <[email-masked]>
          Helped-by: Patrick Steinhardt <[email-masked]>
          Helped-by: Junio C Hamano <[email-masked]>
     +    Signed-off-by: John Passaro <[email-masked]>
      
       ## builtin/commit.c ##
      @@
     @@ builtin/commit.c
       
       static const char * const builtin_commit_usage[] = {
       	N_("git commit [-a | --interactive | --patch] [-s] [-v] [-u<mode>] [--amend]\n"
     -@@ builtin/commit.c: 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(opt->value, "--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;
      @@ builtin/commit.c: static int prepare_to_commit(const char *index_file, const char *prefix,
       	fclose(s->fp);
       
     @@ builtin/commit.c: static int prepare_to_commit(const char *index_file, const cha
       			die(_("unable to pass trailers to --trailers"));
       		strvec_clear(&trailer_args);
       	}
     -@@ builtin/commit.c: int cmd_commit(int argc, const char **argv, const char *prefix)
     - 		OPT_STRING(0, "fixup", &fixup_message, N_("[(amend|reword):]commit"), N_("use autosquash formatted message to fixup or amend/reword 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", &trailer_args, N_("trailer"), N_("add custom trailer(s)"), PARSE_OPT_NONEG, opt_pass_trailer),
     -+		OPT_PASSTHRU_ARGV(0, "trailer", &trailer_args, N_("trailer"), N_("add custom trailer(s)"), PARSE_OPT_NONEG),
     - 		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")),
      
       ## trailer.c ##
     -@@
     - #include "commit.h"
     - #include "trailer.h"
     - #include "list.h"
     -+#include "run-command.h"
     - /*
     -  * Copyright (c) 2013, 2014 Christian Couder <[email-masked]>
     -  */
      @@ trailer.c: void trailer_iterator_release(struct trailer_iterator *iter)
       	strbuf_release(&iter->val);
       	strbuf_release(&iter->key);
       }
      +
     -+int amend_file_with_trailers(const char *path, struct strvec const* 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;
     @@ trailer.c: void trailer_iterator_release(struct trailer_iterator *iter)
      
       ## trailer.h ##
      @@
     - 
       #include "list.h"
       #include "strbuf.h"
     -+#include "strvec.h"
       
     ++struct strvec;
     ++
       enum trailer_where {
       	WHERE_DEFAULT,
     + 	WHERE_END,
      @@ trailer.h: int trailer_iterator_advance(struct trailer_iterator *iter);
        */
       void trailer_iterator_release(struct trailer_iterator *iter);
     @@ trailer.h: int trailer_iterator_advance(struct trailer_iterator *iter);
      + * This calls run_command() and its return value is the same (i.e. 0 for
      + * success, various non-zero for other errors). See run-command.h.
      + */
     -+int amend_file_with_trailers(const char *path, struct strvec const* trailer_args);
     ++int amend_file_with_trailers(const char *path, const struct strvec *trailer_args);
      +
       #endif /* TRAILER_H */
 2:  5b6239167b8 ! 3:  f1d68337eda builtin/tag.c: add --trailer arg
     @@ Metadata
      Author: John Passaro <[email-masked]>
      
       ## Commit message ##
     -    builtin/tag.c: add --trailer arg
     +    builtin/tag.c: add --trailer option
      
          git-tag currently supports interpreting trailers from an annotated tag
     -    message, using --list --format="%(trailers)". There is no ergonomic way
     -    to add trailers to an annotated tag message.
     +    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.
      
     -    In a previous patch, we refactored git-commit's implementation of its
     -    --trailer arg to the trailer.h API. Let's use that new function to teach
     -    git-tag the same --trailer argument, emulating as much of git-commit's
     -    behavior as much as possible.
     +    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
     +    teach git-tag the same --trailer option, emulating as much of
     +    git-commit's behavior as much as possible.
      
     -    Signed-off-by: John Passaro <[email-masked]>
          Helped-by: Patrick Steinhardt <[email-masked]>
     +    Signed-off-by: John Passaro <[email-masked]>
      
       ## Documentation/git-tag.txt ##
      @@ Documentation/git-tag.txt: SYNOPSIS
 3:  d5335e30b0b < -:  ----------- po: update git-tag translations

-- 
gitgitgadget