git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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
Previous: William Chargin
Message 8 of 8 in “Parsing trailers”
  1. William CharginDec 23, 2018
  2. Christian CouderDec 24, 2018
  3. William CharginDec 24, 2018
  4. Christian CouderDec 26, 2018
  5. William CharginDec 26, 2018
  6. Jeff KingJan 3, 2019
  7. William CharginJan 3, 2019
  8. Jeff KingJan 3, 2019

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.