Re: [PATCH v2 2/2] format-patch: add commitListFormat config
- From
Mirko Faina <mroik@delayed.space>
- Date
- Feb 26, 2026, 21:40 UTC
- Message-ID
- <aaC81Hk3tO5N2Rl0@exploit>
- In-Reply-To
- <aZ46xqCusF1av-va@exploit>
On Wed, Feb 25, 2026 at 01:14:13AM +0100, Mirko Faina wrote:
Show 27 quoted lines
> > > + strbuf_release(&tmp);
> > > + return 0;
> > > + }
> > > if (!strcmp(var, "format.outputdirectory")) {
> > > FREE_AND_NULL(cfg->config_output_directory);
> > > return git_config_string(&cfg->config_output_directory, var, value);
> > > @@ -2318,6 +2333,13 @@ int cmd_format_patch(int argc,
> > > goto done;
> > > total = list.nr;
> > >
> > > + if (cover_letter_fmt && (strcmp(cover_letter_fmt, "shortlog") && strncmp(cover_letter_fmt, "log:", 4))) {
> >
> > Overly long line.
>
> Will fix.
>
> > Stepping back a bit, even if we do not validate the format *here*,
> > shouldn't the code that does use cover_letter_fmt later in the
> > control flow *already* be checking the validity of the format and
> > complaining? If that happens early enough, perhaps we do not want
> > to have an extra "early check and die" here.
>
> That is true, and initially I did not introduce a check here, but
> make_cover_letter() is called after the cover letter file has already
> been created. Failing before format-patch could create a file or print
> anything on screeen seemed more clean to me, that's the only reason
> there's a check there.May I have a confirmation on this. Is it ok to leave the extra check here or would you like me to remove it and just let make_cover_letter() handle it?