Re: Parsing trailers

From: William Chargin <wchargin@gmail.com>
Date: Jan 3, 2019, 07:43 UTC
Message-ID: <CAFW+GMD9P-E4xCP+K5JAX70a6cgoHUzOXU-Tv3w6yhKGT_GaEg@mail.gmail.com>
In-Reply-To: <20190103070744.GA24149@sigill.intra.peff.net>

> That's what "%(trailers:only)" does (even if the original separator was
> something else). It also trims any extra whitespace.

Ooh, this is good to know: thanks. (I had found `print_tok_val` in `trailer.c` and assumed that this was the only place with the output logic, but I now see that `format_trailer_info` also exists in the same file.)

> IMHO this is a bug in --parse. It was always meant to give sane,
> normalized output

Okay; this is good to hear. In that case, what would you think about changing `interpret-trailers` as a whole to always emit colons? (Note that the command is already lossy: even with no flags, it replaces each separator character with the first character of `trailer.separators`.) This has the advantage that we avoid adding a configuration option of dubious value—it’s not clear to me why a user would actually _want_ to change the separator to anything other than a colon. The patch should be quite simple, too (only tested lightly on my machine):

diff --git a/trailer.c b/trailer.c
index 0796f326b3..722040e48c 100644
--- a/trailer.c
+++ b/trailer.c
@@ -156,7 +156,7 @@ static void print_tok_val(FILE *outfile, const
char *tok, const char *val)
        if (strchr(separators, c))
                fprintf(outfile, "%s%s\n", tok, val);
        else
-               fprintf(outfile, "%s%c %s\n", tok, separators[0], val);
+               fprintf(outfile, "%s: %s\n", tok, val);
 }

Is this veering too much from “bug fix” toward “backward-incompatible
behavior change” for your comfort?

I agree that either this or your (1) and (2) would eliminate the need
for `-z`, which is nice.

Best,
WC