[v5,1/3] builtin/commit: use ARGV macro to collect trailers

Commit Message

John Passaro May 5, 2024, 6:49 p.m. UTC

From: John Passaro <[e-mail]>

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 <[e-mail]>
---
 builtin/commit.c | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

Comments

John Passaro May 7, 2024, 3:38 p.m. UTC | #1

On Sun, May 5, 2024 at 2:49 PM John Passaro via GitGitGadget
<[e-mail]> wrote:
>

Thank you for queuing the patch series! Hopeful and excited that this
may go in a future release.

> From: John Passaro <[e-mail]>
>
> 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 <[e-mail]>

Looking over the patch series I notice that I left a grammar error in
this description,
if the patch makes it to the "next" branch I hope you can amend the
message first,
as follows:

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-checks that `unset` is always false; both of these are
already implemented in the parse-option API.

> ---
>  builtin/commit.c | 10 +---------
>  1 file changed, 1 insertion(+), 9 deletions(-)
>
> diff --git a/builtin/commit.c b/builtin/commit.c
> index 6e1484446b0..5a3248370db 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -142,14 +142,6 @@ 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;
> @@ -1673,7 +1665,7 @@ 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")),
> --
> gitgitgadget
>

Junio C Hamano May 7, 2024, 5:06 p.m. UTC | #2

John Passaro <[e-mail]> writes:

>> From: John Passaro <[e-mail]>
>>
>> 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 <[e-mail]>
>
> Looking over the patch series I notice that I left a grammar error in
> this description,
> if the patch makes it to the "next" branch I hope you can amend the
> message first,
> as follows:
>
> 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-checks that `unset` is always false; both of these are
> already implemented in the parse-option API.

Thanks for stopping me before the topic got merged to 'next'.
Amended.

Patch

diff --git a/builtin/commit.c b/builtin/commit.c
index 6e1484446b0..5a3248370db 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -142,14 +142,6 @@  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;
@@ -1673,7 +1665,7 @@  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")),