From: Mirko Faina Date: Thu, 26 Feb 2026 21:40:09 GMT Subject: Re: [PATCH v2 2/2] format-patch: add commitListFormat config Message-ID: In-Reply-To: On Wed, Feb 25, 2026 at 01:14:13AM +0100, Mirko Faina wrote: > > > + 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?