Re: Parsing trailers
- From
Jeff King <peff@peff.net>
- Date
- Jan 3, 2019, 07:50 UTC
- Message-ID
- <20190103075045.GD24925@sigill.intra.peff.net>
- In-Reply-To
- <CAFW+GMD9P-E4xCP+K5JAX70a6cgoHUzOXU-Tv3w6yhKGT_GaEg@mail.gmail.com>
On Wed, Jan 02, 2019 at 11:43:55PM -0800, William Chargin wrote:
Show 27 quoted lines
> > 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 think that gets weird in non-parse modes. For example, if I'm not trying to parse, but rather to _add_ a new trailer (because I'm writing out a commit message to feed to git-commit-tree), then I'd presumably want the normal configured separators.
I dunno. I don't think I've ever seen a trailer with a non-colon separator, nor have I ever used interpret-trailers for anything except parsing. But it obviously was designed with more flexibility in mind, and I suspect the patch above has a high chance of breaking something somewhere. Tying it to --parse seems a lot safer, since that was introduced exactly for this case.
-Peff