From: Dominique Martinet <asmadeus@codewreck•org>
To: Junio C Hamano <gitster@pobox•com>, Jeff King <peff@peff•net>
Cc: git@vger•kernel.org, "René Scharfe" <l.s.r@web•de>,
"Julien Moutinho" <julm@sourcephile•fr>
Subject: Re: [RFC PATCH] builtin/format-patch: print a warning for skipped merge commits?
Date: Sat, 3 Jan 2026 21:24:57 +0900 [thread overview]
Message-ID: <aVkKmcER2K8D9U4T@codewreck.org> (raw)
In-Reply-To: <20260102073358.GC2581074@coredump.intra.peff.net> <xmqqo6nfdyl4.fsf@gitster.g>
Thank you both for taking the time to reply over new year
Junio C Hamano wrote on Wed, Dec 31, 2025 at 02:12:07PM +0900:
> Dominique Martinet <asmadeus@codewreck•org> writes:
> > This RFC patch illustrates how we could easily print a warning, but
> > perhaps the warning would only make sense if no other commit has been
> > formatted?
>
> Yeah, when nothing is shown but the given range is not empty, it
> would not be too annoying to give an advice message.
>
> On the other hand, I do not think it is a good idea to say anything
> extra when the user gave a range "trunk..mytopic" that has repeated
> back-merges from trunk into mytopic, to format what s/he worked on
> the mytopic branch. They _expect_ these back-merges to be ignored,
> and it would be purely an unwanted noise.
Okay, I can see this being confusing to people not used to format-patch
even with a range, but I agree it'll be annoying more often than not in
general so I'm fine with this.
It makes it a bit cumbersome to print details about the commit(s) being
skipped though, so it's probably simpler to do a generic message like
"No patch generated. Note merge commits are skipped." like this?
-----
diff --git a/builtin/log.c b/builtin/log.c
index d4cf9c59c81a..1ff3c5a4c960 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1901,6 +1901,7 @@ int cmd_format_patch(int argc,
char *to_free = NULL;
struct setup_revision_opt s_r_opt;
size_t nr = 0, total, i;
+ int seen_merge = 0;
int use_stdout = 0;
int start_number = -1;
int just_numbers = 0;
@@ -2044,7 +2045,6 @@ int cmd_format_patch(int argc,
rev.expand_tabs_in_log_default = 0;
rev.verbose_header = 1;
rev.diff = 1;
- rev.max_parents = 1;
rev.diffopt.flags.recursive = 1;
rev.diffopt.no_free = 1;
memset(&s_r_opt, 0, sizeof(s_r_opt));
@@ -2274,6 +2274,10 @@ int cmd_format_patch(int argc,
die(_("revision walk setup failed"));
rev.boundary = 1;
while ((commit = get_revision(&rev)) != NULL) {
+ if (commit->parent && commit->parents->next) {
+ seen_merge = 1;
+ continue;
+ }
if (commit->object.flags & BOUNDARY) {
boundary_count++;
origin = (boundary_count == 1) ? commit : NULL;
@@ -2287,9 +2291,12 @@ int cmd_format_patch(int argc,
REALLOC_ARRAY(list, nr);
list[nr - 1] = commit;
}
- if (nr == 0)
+ if (nr == 0) {
+ if (seen_merge)
+ warning(_("No patch generated. Note merge commits are skipped."));
/* nothing to do */
goto done;
+ }
total = nr;
if (cover_letter == -1) {
if (cfg.config_cover_letter == COVER_AUTO)
-----
I'll send that as v2 with tests fixed/added when time permits.
Jeff King wrote on Fri, Jan 02, 2026 at 02:33:58AM -0500:
> On Wed, Dec 31, 2025 at 12:42:17PM +0900, Dominique Martinet wrote:
>
> > @@ -2274,6 +2273,11 @@ int cmd_format_patch(int argc,
> > die(_("revision walk setup failed"));
> > rev.boundary = 1;
> > while ((commit = get_revision(&rev)) != NULL) {
> > + if (commit->parents->next) {
> > + warning(_("skipped merge commit %s"),
> > + oid_to_hex(&commit->object.oid));
> > + continue;
> > + }
>
> I don't have any thoughts on whether the patch is overall a good
> direction or not, but I suspect this line will segfault if we ever see a
> root commit. You probably want:
>
> if (commit->parents && commit->parents->next)
Thank you for pointing that out!
--
Dominique Martinet | Asmadeus
next prev parent reply other threads:[~2026-01-03 12:25 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-12-31 3:42 [RFC PATCH] builtin/format-patch: print a warning for skipped merge commits? Dominique Martinet
2025-12-31 5:12 ` Junio C Hamano
2026-01-03 12:24 ` Dominique Martinet [this message]
2026-01-04 2:27 ` Junio C Hamano
2026-02-01 8:38 ` Dominique Martinet
2026-01-02 7:33 ` Jeff King
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aVkKmcER2K8D9U4T@codewreck.org \
--to=asmadeus@codewreck$(echo .)org \
--cc=git@vger$(echo .)kernel.org \
--cc=gitster@pobox$(echo .)com \
--cc=julm@sourcephile$(echo .)fr \
--cc=l.s.r@web$(echo .)de \
--cc=peff@peff$(echo .)net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox