Re: [RFC 1/3] mailinfo: extract patch series id

[Nicolas Morey-Chaisemartin <NMoreyChaisemartin@suse.de>]

Le 14/11/2017 à 06:47, Junio C Hamano a écrit :
> Nicolas Morey-Chaisemartin <NMoreyChaisemartin@suse.de> writes:
>
>> Extract the patch ID and series length from the [PATCH N/M]
>>  prefix in the mail header
>>
>> Signed-off-by: Nicolas Morey-Chaisemartin <nicolas@morey-chaisemartin.com>
>> ---
>>  mailinfo.c | 35 +++++++++++++++++++++++++++++++++++
>>  mailinfo.h |  2 ++
>>  2 files changed, 37 insertions(+)
> As JTan already mentioned, relying on a substring "PATCH" may not be
> very reliable, and trying to locate "%d/%d]" feels like a better
> approach.
>
> cleanup_subject() is called only when keep_subject is false, so this
> code will not trigger in that case at all.  Is this intended?
>
> I would have expected that a new helper function would be written,
> without changing existing helpers like cleanup_subject(), and that
> new helper gets called by handle_info() after output_header_lines()
> helper is called for the "Subject".
Isn't that too late ?
If keep subject is not set, will cleanup_subject not drop all those info from the strbuf  ?
But yes it is not in the right function now.
But I would call the function before
            if (!mi->keep_subject) {

>
> Whenever mailinfo learns to glean a new useful piece of information,
> it should be made available to scripts that run "git mailinfo", too.
> Perhaps show something like
>
> 	PatchNumber: 1
> 	TotalPatches: 3
>
> at the end of handle_info() to mi->output?  I do not think existing
> tools mind too much, even if we added a for-debug output e.g.
>
> 	RawSubject: [RFC 1/3] mailinfo: extract patch series id
>
> to the output.
Will do.