threads / rfc / 63053

RFC patch, 6 partsAdd --subject-extra-prefix flag to format-patch

Subject: [GSoC][RFC PATCH 0/6] Add --subject-extra-prefix flag to format-patch

## tl;dr

15 messages between Mar 3, 2025 and Mar 5, 2025. Diffs are folded; open one to read it.

replies: 14people: 4as markdown or json

Lucas Seiki Oshiro· Mar 3, 2025, 22:00 UTC · lore
Hi!

This is a "scratch-my-own-itch" patch, adding a new flag to format-patch. This flag allows the user add an extra prefix to the subject, placed before the actual prefix, like the "[GSoC]" in the subject of this patchset.

This is specially targeted for new users or developers from mentoring projects like GSoC or Outreachy. Currently, if we want to add to the subject something like "[Newbie]", "[GSoC]" or "[Outreachy]", we need to perform some hacks like passing --subject-prefix="GSoC][PATCH" or changing it manually in the patch content. 

With this new flag, this patchset could be created by using:
`git format-patch -6 --subject-extra-prefix="GSoC" --rfc --to="..."`

This would be only a feature request but I thought it would be better to present a first solution, so I'm sending my implementation as a RFC patchset.

Thanks!
Lucas Seiki Oshiro (6):
  builtin/log: add subject-extra-prefix flag to format-patch
  builtin/log: die if -k and --suject-extra-prefix are used together
  revision: add subject_extra_field to struct rev_info
  builtin/log: fill subject extra prefix in format-patch
  log-tree: add subject prefix in output email subject
  t4014: add tests for the new flag --subject-extra-prefix
 builtin/log.c           |  7 +++++++
 log-tree.c              | 12 ++++++++----
 revision.h              |  2 ++
 t/t4014-format-patch.sh | 40 ++++++++++++++++++++++++++++++++++++++++
 4 files changed, 57 insertions(+), 4 deletions(-)
-- 
2.39.5 (Apple Git-154)
Lucas Seiki Oshiro· Mar 3, 2025, 22:00 UTC · re: Lucas Seiki Oshiro · lore

[GSoC][RFC PATCH 1/6] builtin/log: add subject-extra-prefix flag to format-patch

Add a new flag --subject-extra-prefix to format-patch. This flag is primarily targeted for new contributors that send emails with the suject starting with "[Newbie]", "[FirstTimer]" or "[Newcomer]" and developers from mentoring programs, which may start the subject with "[GSoC]", "[Outreachy]" or similar.

Signed-off-by: Lucas Seiki Oshiro <lucasseikioshiro@gmail.com>
---
 builtin/log.c | 4 ++++
 1 file changed, 4 insertions(+)
Show changes to builtin/log.c +4 −0
diff --git a/builtin/log.c b/builtin/log.c
index e41f88945e..95667c93ca 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -2036,6 +2036,7 @@ int cmd_format_patch(int argc,
 	struct strbuf rdiff2 = STRBUF_INIT;
 	struct strbuf rdiff_title = STRBUF_INIT;
 	const char *rfc = NULL;
+	const char *extra_prefix = NULL;
 	int creation_factor = -1;
 	const char *signature = git_version_string;
 	char *signature_to_free = NULL;
@@ -2079,6 +2080,9 @@ int cmd_format_patch(int argc,
 		OPT_CALLBACK_F(0, "subject-prefix", &cfg, N_("prefix"),
 			    N_("use [<prefix>] instead of [PATCH]"),
 			    PARSE_OPT_NONEG, subject_prefix_callback),
+		OPT_STRING(0, "subject-extra-prefix", &extra_prefix,
+			   N_("subject-extra-prefix"),
+			   N_("add an extra prefix at the beginning of the subject")),
 		OPT_CALLBACK_F('o', "output-directory", &output_directory,
 			    N_("dir"), N_("store resulting files in <dir>"),
 			    PARSE_OPT_NONEG, output_directory_callback),

-- 
2.39.5 (Apple Git-154)
Lucas Seiki Oshiro· Mar 3, 2025, 22:00 UTC · re: Lucas Seiki Oshiro · lore

[GSoC][RFC PATCH 2/6] builtin/log: die if -k and --suject-extra-prefix are used together

When calling format-patch, -k keeps the commit title as the patch subject and can't be used with other options that depend on the existence of a prefix. This way, -k shouldn't also be used with --subject-extra-prefix, as it introduces an extra prefix.

If -k and --subject-extra-prefix are used together, die.
Signed-off-by: Lucas Seiki Oshiro <lucasseikioshiro@gmail.com>
---
 builtin/log.c | 2 ++
 1 file changed, 2 insertions(+)
Show changes to builtin/log.c +2 −0
diff --git a/builtin/log.c b/builtin/log.c
index 95667c93ca..52dc0301cf 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -2256,6 +2256,8 @@ int cmd_format_patch(int argc,
 		die(_("options '%s' and '%s' cannot be used together"), "-n", "-k");
 	if (cfg.keep_subject && cfg.subject_prefix)
 		die(_("options '%s' and '%s' cannot be used together"), "--subject-prefix/--rfc", "-k");
+	if (cfg.keep_subject && extra_prefix)
+		die(_("options '%s' and '%s' cannot be used together"), "--subject-extra-prefix", "-k");
 	rev.preserve_subject = cfg.keep_subject;
 
 	argc = setup_revisions(argc, argv, &rev, &s_r_opt);

-- 
2.39.5 (Apple Git-154)
Eric Sunshine· Mar 3, 2025, 22:36 UTC · re: Lucas Seiki Oshiro · lore

Re: [GSoC][RFC PATCH 2/6] builtin/log: die if -k and --suject-extra-prefix are used together

On Mon, Mar 3, 2025 at 5:01 PM Lucas Seiki Oshiro <lucasseikioshiro@gmail.com> wrote:

> builtin/log: die if -k and --suject-extra-prefix are used together
s/suject/subject/
Show 8 quoted lines
> When calling format-patch, -k keeps the commit title as the patch subject
> and can't be used with other options that depend on the existence of a
> prefix. This way, -k shouldn't also be used with --subject-extra-prefix,
> as it introduces an extra prefix.
>
> If -k and --subject-extra-prefix are used together, die.
>
> Signed-off-by: Lucas Seiki Oshiro <lucasseikioshiro@gmail.com>
Lucas Seiki Oshiro· Mar 3, 2025, 22:00 UTC · re: Lucas Seiki Oshiro · lore

[GSoC][RFC PATCH 3/6] revision: add subject_extra_field to struct rev_info

Currently, the `struct rev_info` is responsible for holding information about the prefix that is added at the beginning of the subject when `format-patch` is called. This way, it makes sense that it also holds the information of the extra prefix, if it is present.

Add a new field called `subject_extra_field` to `struct rev_info` and use NULL as its default value.

Signed-off-by: Lucas Seiki Oshiro <lucasseikioshiro@gmail.com>
---
 revision.h | 2 ++
 1 file changed, 2 insertions(+)
Show changes to revision.h +2 −0
diff --git a/revision.h b/revision.h
index 71e984c452..70927ef1fb 100644
--- a/revision.h
+++ b/revision.h
@@ -294,6 +294,7 @@ struct rev_info {
 	const char	*extra_headers;
 	const char	*log_reencode;
 	const char	*subject_prefix;
+	const char	*subject_extra_prefix;
 	int		patch_name_max;
 	int		no_inline;
 	int		show_log_size;
@@ -413,6 +414,7 @@ struct rev_info {
 	.expand_tabs_in_log = -1, \
 	.commit_format = CMIT_FMT_DEFAULT, \
 	.expand_tabs_in_log_default = 8, \
+        .subject_extra_prefix = NULL, \
 }
 
 /**

-- 
2.39.5 (Apple Git-154)
Lucas Seiki Oshiro· Mar 3, 2025, 22:00 UTC · re: Lucas Seiki Oshiro · lore

[GSoC][RFC PATCH 4/6] builtin/log: fill subject extra prefix in format-patch

Since the `struct rev_info` is the responsible of holding the subject extra prefix string, copy to its `subject_extra_prefix` field the string provided by --subject-extra-prefix when calling `format-patch`.

Signed-off-by: Lucas Seiki Oshiro <lucasseikioshiro@gmail.com>
---
 builtin/log.c | 1 +
 1 file changed, 1 insertion(+)
Show changes to builtin/log.c +1 −0
diff --git a/builtin/log.c b/builtin/log.c
index 52dc0301cf..034ad44ef0 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -2205,6 +2205,7 @@ int cmd_format_patch(int argc,
 		rev.reroll_count = reroll_count;
 	}
 
+	rev.subject_extra_prefix = extra_prefix;
 	rev.subject_prefix = cfg.sprefix.buf;
 
 	for (i = 0; i < cfg.extra_hdr.nr; i++) {

-- 
2.39.5 (Apple Git-154)
Lucas Seiki Oshiro· Mar 3, 2025, 22:00 UTC · re: Lucas Seiki Oshiro · lore

[GSoC][RFC PATCH 5/6] log-tree: add subject prefix in output email subject

When an extra subject prefix is provided by the user when calling `format-patch --subject-extra-prefix`, insert it between brackets at the beginning of the the patch subject. Don't change the behavior when this flag is not used.

For example, this `format-patch` call:
`git format-patch -1 --subject-extra-prefix EXTRA -n -v2`
will create a patch with the subject beginning with:
`Subject: [EXTRA][PATCH v2 1/1]`
Signed-off-by: Lucas Seiki Oshiro <lucasseikioshiro@gmail.com>
---
 log-tree.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)
Show changes to log-tree.c +8 −4
diff --git a/log-tree.c b/log-tree.c
index 8b184d6776..d72ab25585 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -478,17 +478,21 @@ void fmt_output_commit(struct strbuf *filename,
 
 void fmt_output_email_subject(struct strbuf *sb, struct rev_info *opt)
 {
+	strbuf_addstr(sb, "Subject: ");
+
+	if (opt->subject_extra_prefix && *opt->subject_extra_prefix)
+		strbuf_addf(sb, "[%s]",
+			    opt->subject_extra_prefix);
+
 	if (opt->total > 0) {
-		strbuf_addf(sb, "Subject: [%s%s%0*d/%d] ",
+		strbuf_addf(sb, "[%s%s%0*d/%d] ",
 			    opt->subject_prefix,
 			    *opt->subject_prefix ? " " : "",
 			    decimal_width(opt->total),
 			    opt->nr, opt->total);
 	} else if (opt->total == 0 && opt->subject_prefix && *opt->subject_prefix) {
-		strbuf_addf(sb, "Subject: [%s] ",
+		strbuf_addf(sb, "[%s] ",
 			    opt->subject_prefix);
-	} else {
-		strbuf_addstr(sb, "Subject: ");
 	}
 }
 

-- 
2.39.5 (Apple Git-154)
Lucas Seiki Oshiro· Mar 3, 2025, 22:00 UTC · re: Lucas Seiki Oshiro · lore

[GSoC][RFC PATCH 6/6] t4014: add tests for the new flag --subject-extra-prefix

Add tests for `format-patch --subject-extra-prefix` asserting that it works correctly in the following situations:

- without other flags related to the prefix;
- with the --rfc flag, which should add RFC to the prefix;
- with the -n flag, which should add the numbering to the prefix;
- with the -v flag, which should add the patch version to the prefix;
- with the -k flag, diying as they aren't meant to be used together.
Signed-off-by: Lucas Seiki Oshiro <lucasseikioshiro@gmail.com>
---
 t/t4014-format-patch.sh | 40 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)
Show changes to t/t4014-format-patch.sh +40 −0
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 884f83fb8a..1256ce2197 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -1444,6 +1444,46 @@ test_expect_success '--rfc and -k cannot be used together' '
 	test_cmp expect.err actual.err
 '
 
+cat >expect <<'EOF'
+Subject: [EXTRA][PATCH] header with . in it
+EOF
+test_expect_success '--subject-extra-prefix adds extra prefix' '
+	git format-patch -1 --stdout --subject-extra-prefix=EXTRA >patch &&
+	grep ^Subject: patch >actual &&
+	test_cmp expect actual
+'
+
+cat >expect <<'EOF'
+Subject: [EXTRA][RFC PATCH] header with . in it
+EOF
+test_expect_success '--subject-extra-prefix works with --rfc' '
+	git format-patch --rfc -1 --stdout --subject-extra-prefix=EXTRA >patch &&
+	grep ^Subject: patch >actual &&
+	test_cmp expect actual
+'
+
+cat >expect <<'EOF'
+Subject: [EXTRA][PATCH 1/1] header with . in it
+EOF
+test_expect_success '--subject-extra-prefix works with numbered patches' '
+	git format-patch -n -1 --stdout --subject-extra-prefix=EXTRA >patch &&
+	grep ^Subject: patch >actual &&
+	test_cmp expect actual
+'
+
+cat >expect <<'EOF'
+Subject: [EXTRA][PATCH v2] header with . in it
+EOF
+test_expect_success '--subject-extra-prefix works with -v' '
+	git format-patch -v2 -1 --stdout --subject-extra-prefix=EXTRA >patch &&
+	grep ^Subject: patch >actual &&
+	test_cmp expect actual
+'
+
+test_expect_failure '--subject-extra-prefix does not run with -k' '
+	git format-patch -k -1 --stdout --subject-extra-prefix=EXTRA >/dev/null
+'
+
 test_expect_success '--from=ident notices bogus ident' '
 	test_must_fail git format-patch -1 --stdout --from=foo >patch
 '

-- 
2.39.5 (Apple Git-154)
Junio C Hamano· Mar 3, 2025, 23:08 UTC · re: Lucas Seiki Oshiro · lore

Re: [GSoC][RFC PATCH 0/6] Add --subject-extra-prefix flag to format-patch

Lucas Seiki Oshiro <lucasseikioshiro@gmail.com> writes:
> Subject: Re: [GSoC][RFC PATCH 0/6] Add --subject-extra-prefix flag to format-patch

Hmph, instead use "--rfc=GSoC" to do [GSoC PATCH n/m] and please do not pile more "extra" on top?

Patrick Steinhardt· Mar 4, 2025, 07:32 UTC · re: Junio C Hamano · lore

Re: [GSoC][RFC PATCH 0/6] Add --subject-extra-prefix flag to format-patch

On Mon, Mar 03, 2025 at 03:08:43PM -0800, Junio C Hamano wrote:
Show 6 quoted lines
> Lucas Seiki Oshiro <lucasseikioshiro@gmail.com> writes:
> 
> > Subject: Re: [GSoC][RFC PATCH 0/6] Add --subject-extra-prefix flag to format-patch
> 
> Hmph, instead use "--rfc=GSoC" to do [GSoC PATCH n/m] and please do
> not pile more "extra" on top?

I do understand though that `--rfc=GSoC` is not exactly discoverable for a newcomer. Maybe a simpler fix would be to introduce an alias for that option that is more discoverable? I don't have a great idea for how to name it -- the best that comes to mind is `--patch-prefix`, which mirrors `--subject-prefix`.

    --patch-prefix=<prefix>
    --rfc
        Prepends the string `<prefix>` to the subject prefix. As the
        subject prefix defaults to "PATCH", you’ll get "<prefix> PATCH"
        by default.
        When `--rfc` is given, the prefix will be set to "RFC" . RFC
        means "Request For Comments"; use this when sending an
        experimental patch for discussion rather than application.
        "--rfc=WIP" may also be a useful way to indicate that a patch is
        not complete yet ("WIP" stands for "Work In Progress").
        If the convention of the receiving community for a particular
        extra string is to have it after the subject prefix, the string
        <prefix> can be prefixed with a dash ("-") to signal that the rest
        of the <prefix> string should be appended to the subject prefix
        instead, e.g., --patch-prefix='-(WIP)' results in "PATCH (WIP)".

We would hide away the optional value `[=<rfc>]` of the `--rfc` option and instead advertise `--patch-prefix=<prefix>`. This would of course only be a cosmetic change, we'd still accept the optional argument so that we don't break backwards compatibility.

Patrick
Junio C Hamano· Mar 4, 2025, 22:06 UTC · re: Patrick Steinhardt · lore

Re: [GSoC][RFC PATCH 0/6] Add --subject-extra-prefix flag to format-patch

Patrick Steinhardt <ps@pks.im> writes:
Show 5 quoted lines
> I do understand though that `--rfc=GSoC` is not exactly discoverable for
> a newcomer. Maybe a simpler fix would be to introduce an alias for that
> option that is more discoverable? I don't have a great idea for how to
> name it -- the best that comes to mind is `--patch-prefix`, which
> mirrors `--subject-prefix`.

The problem with it is that having "patch" vs "subject" prefix will make it confusing which one is which.

Let's step back and examine what we already have.
 * The "--subject-prefix" is configurable and format.subjectPrefix
   is meant to go to .git/config for repositories that work with
   projects with special needs (they tell you to use not [PATCH] but
   subproject specific moniker like [PATCH rcu] instead).
 * The "--rfc[=<word>]" has no configuration and it is deliberately
   so, as it is meant to be used as needed, on occasions that call
   for it, pretty much decided per invocation.

Are these two points above not clear in our documentation? If so, we should fix it, but I do not think giving a synonym to --rfc is a good way to clarify it.

If it is a GSoC topic sent to this list, it _will_ be GSoC topic for the entire iterations of the same topic. So it would be good to use --subject-prefix="GSoC PATCH" (or configure format.subjectPrefix in the repository to use that, if a student works with us only on GSoC topics during the Summer-of-Code program duration), and on occasion, when students are not so sure about their patches (yet), use of --rfc would be a good way, which would give us "[RFC GSoC PATCH 0/6]", to mark the patches as such.

Another problem with --patch-prefix synonym is that --rfc is not necessarily a prefix. With

	git format-patch --rfc='-WIP' --subject-prefix='GSoC PATCH'
you would get "[GSoC PATCH WIP 0/6]" instead.

In any case, I think it is the root cause of this disease to treat GSoC mark as something extra that requires a new marker that is separate from the usual PATCH marker. Using whatever mechanism you would use your string in place for the standard PATCH mark to make it say "GSoC PATCH" would be the most appropriate, without mucking with an extra option, be it '--rfc' or '--subject-extra-prefix', I would say.

Thanks.
Junio C Hamano· Mar 4, 2025, 13:05 UTC · re: Junio C Hamano · lore

Re: [GSoC][RFC PATCH 0/6] Add --subject-extra-prefix flag to format-patch

Junio C Hamano <gitster@pobox.com> writes:
Show 6 quoted lines
> Lucas Seiki Oshiro <lucasseikioshiro@gmail.com> writes:
>
>> Subject: Re: [GSoC][RFC PATCH 0/6] Add --subject-extra-prefix flag to format-patch
>
> Hmph, instead use "--rfc=GSoC" to do [GSoC PATCH n/m] and please do
> not pile more "extra" on top?
Or even --subject-prefix="GSoC PATCH".

Once you stop treating [GSoC] as if it is an independent prefix, everything will fall into place naturally without adding anything extra.

To put it differently, which one wastes more precious horizontal screen estate on the Subject line, where practically only 50 or so columns are usable?

    Subject: [A][B][C][D][PATCH 0/6] This is my first contribution
    Subject: [A B C D PATCH 0/6] This is my first contribution
Lucas Seiki Oshiro· Mar 4, 2025, 23:19 UTC · re: Junio C Hamano · lore

Re: [GSoC][RFC PATCH 0/6] Add --subject-extra-prefix flag to format-patch

> Once you stop treating [GSoC] as if it is an independent prefix,
> everything will fall into place naturally without adding anything
> extra.

So, instead of changing the behavior of format-patch, would it be better if we changed the documentation to ask these patches to be sent with `[GSoC PATCH]` or similar instead of `[GSoC][PATCH]`?

I mean, in https://git.github.io/General-Microproject-Information/ :

""" When you submit your patch, please mention that you plan to apply for the GSoC or Outreachy. You can use "[GSoC][PATCH ...]" or "[Outreachy][PATCH ...]" in the subject of the emails you send for that purpose. """

Junio C Hamano· Mar 5, 2025, 02:06 UTC · re: Lucas Seiki Oshiro · lore

Re: [GSoC][RFC PATCH 0/6] Add --subject-extra-prefix flag to format-patch

Lucas Seiki Oshiro <lucasseikioshiro@gmail.com> writes:
Show 7 quoted lines
>> Once you stop treating [GSoC] as if it is an independent prefix,
>> everything will fall into place naturally without adding anything
>> extra.
>
> So, instead of changing the behavior of format-patch, would it be
> better if we changed the documentation to ask these patches to be
> sent with `[GSoC PATCH]` or similar instead of `[GSoC][PATCH]`?

Yup, I do not know offhand who are managing these descriptions, but having extra moniker in the same [square bracket] as PATCH is a more common practice, and we should follow suit.

Thanks.
Lucas Seiki Oshiro· Mar 5, 2025, 14:26 UTC · re: Junio C Hamano · lore

Re: [GSoC][RFC PATCH 0/6] Add --subject-extra-prefix flag to format-patch

> Yup, I do not know offhand who are managing these descriptions, but
> having extra moniker in the same [square bracket] as PATCH is a more
> common practice, and we should follow suit.
Ok, thanks!

I just created a PR in git.github.io <http://git.github.io/> replacing the mentions of `[GSoC][PATCH]` by `[GSoC PATCH]`:

https://github.com/git/git.github.io/pull/756

← back to recent threads