Jacob Keller <jacob.e.keller@xxxxxxxxx> writes:
> Its frustrating to download an mbox file and then have to manually re-sort
> or re-apply patches because it failed to extract things nicely. I don't know
> if this approach is the best solution, or whether there's something else we
> could do instead. In theory we probably want something more robust for the
> M/N extraction over using strchr, memrchr, and whatnot....
>
> builtin/mailsplit.c | 126 ++++++++++++++++++++++++++++++++++++++++----
> 1 file changed, 116 insertions(+), 10 deletions(-)
>
> diff --git a/builtin/mailsplit.c b/builtin/mailsplit.c
> index 3af9ddb8ae5c..5255f4056e91 100644
> --- a/builtin/mailsplit.c
> +++ b/builtin/mailsplit.c
> @@ -10,7 +10,7 @@
> #include "strbuf.h"
>
> static const char git_mailsplit_usage[] =
> -"git mailsplit [-d<prec>] [-f<n>] [-b] [--keep-cr] -o<directory> [(<mbox>|<Maildir>)...]";
> +"git mailsplit [-d<prec>] [-f<n>] [-b] [--keep-cr] [--extract-patch-from-subject] -o<directory> [(<mbox>|<Maildir>)...]";
"extract patch from subject" is quite a mouthful and more
importantly, I suspect that it is different from what the option
does (unless the sender is cramming the log message and the patch on
a single title line).
This seems to try parsing the numbers out of the prefixes like
"[PATCH v2 04/18]" but I wonder if it has to understand four and
eighteen in such a header. Doesn't the sending side make sure that
the patches will be in the right order if you sort the message in
your MUA by the subject textually? After all, such a numbering
convention is to aid humans to read them in order in a mailbox.
I also wonder if it is enough to run the commit title (before
stripping out the "[PATCH v4 04/18]" part) through
format_sanitized_subject() to derive the patch file name. At the
receiving end, they have to be prepared to take a non-number with
your patch anyway, and when things go wrong, peeking the directory
may become easier. I dunno.
> + if (found) {
> + *nr = atoi(m_buf.buf);
> + *total = atoi(n_buf.buf);
> + } else {
> + *nr = 0;
> + *total = 0;
> + }
Having said all that, I also wonder if we want to simplify this to
the other extreme. When we accept such a 18-patch series that is
stored in a mailbox file in a wrong order, we do not even care about
"/18" part of "[PATCH v2 04/18]". So instead of worrying about
total at all, how about "--use-patchnum-from-subject" that only uses
the "04" part of "[PATCH v2 04/18]" as the output filename? We
would probably still want to count how many messges we processed and
as long as we need to parse out "04" out of "04/18", we'd probably
parse "18" as well, so we can maintain a bitmap to ensure we saw all
the messages without duplicates, or something.
The reason why I care is because such a scheme would be easier for
existing consumers that are prepared to take ...
> - char *name = xstrfmt("%s/%0*d", dir, nr_prec, ++skip);
... only numbers in their filenames, and not ...
> ...
> + name = xstrfmt("%s/%0*d_%0*d_%0*d", dir, nr_prec, total, nr_prec, nr, nr_prec, ++skip);
... a string like "04_18".
Overall, I like the idea of massaging a mailbox that got patches in
shuffled order to make it usable. In addition to the "sort the
shuffled mess" benefit, it would allow us to drop the cover letter
and pick only the patch messages. I just am not happy with the
output "04_18", and the parsing code, as you said, is yucky indeed.
And of course totally outside the Git tools scope, I wonder if this
is something "formail" should be able to do more naturally.
