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

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 14, 2017, 05:47 UTC
Message-ID: <xmqqfu9h4div.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <2252b046-a608-b2aa-d67a-8f7e95fe2dbc@suse.de>

Nicolas Morey-Chaisemartin <NMoreyChaisemartin@suse.de> writes:

Show 8 quoted lines

> 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".

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.