From: Mirko Faina <mroik@delayed.space>
Date: Tue, 10 Mar 2026 21:23:13 GMT
Subject: Re: [PATCH v7 4/5] format-patch: add commitListFormat config
Message-ID: <abCLFS3QP7rJHueq@exploit>
In-Reply-To: <xmqqikb3ws3e.fsf@gitster.g>

On Tue, Mar 10, 2026 at 09:45:57AM -0700, Junio C Hamano wrote:
> That syntax is the same as setting config.key=true; disabling the
> feature triggered by config.key is quite counter-intuitive, isn't
> it?
> 
> We are by default using "shortlog", but use of this configuration
> variable is a sign that the user wants to use a more modern custom
> format that is not the traditional "shortlog".  It would be quite
> natural to invoke the modern default by setting it to "true" (i.e.,
> "I want to enable the new format.commitlistformat feature, but I am
> not saying which format, and the "log:[%(count)/%(total)] %s" format
> is used).
> 
> Perhaps "format.commitlistformat = false" should disable the modern
> format and fall back to "shortlog", setting it to true (including
> the use of "valueless true" syntax) should enable it and use the
> modern default "log:[%c/%t] %s" format, and non-bool text should be
> used as a custom specification ("shortlog", or "log:<format>")?
> 
> I.e.
> 
> 	switch (git_parse_maybe_bool_text(value)) {
>         case 0: /* false */
> 		fmt_cover_letter_commit_list = "shortlog";
> 		break;
> 	case 1: /* true - use the modern default format */
> 		fmt_cover_letter_commit_list = "log:[%c/%t] %s";
> 		break;		
> 	default:
> 		fmt_cover_letter_commit_list = value;
> 		break;
> 	}
> 
> Hmm?

Mmh, what if instead we defined a prefix format just like shortlog?
Maybe call it something like "numbered" or something similar (not too
good with coming up with names).

I dislike the idea of having an option be multiple types. Should bool or
string, not both.