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

Re: [PATCH 4/8] pretty: fix parsing of half-valid "%<" and "%>" placeholders

From
Patrick Steinhardt <ps@pks.im>
Date
Mar 24, 2025, 10:10 UTC
Message-ID
<Z-Eveqmb7Et6aHrO@pks.im>
In-Reply-To
<CAN0heSpN-k886+RsZ0+djLd974Mq57B4quZK1yKXRMxCnOvzZw@mail.gmail.com>
On Thu, Mar 20, 2025 at 05:11:09PM +0100, Martin Ågren wrote:
Show 31 quoted lines
> On Thu, 20 Mar 2025 at 10:18, Patrick Steinhardt <ps@pks.im> wrote:
> >
> > On Wed, Mar 19, 2025 at 08:23:37AM +0100, Martin Ågren wrote:
> > > When parsing a "%<" or "%>", only store the parsed data after parsing
> > > successfully. The added test would have failed before this commit. It
> > > also shows how the existing behavior is hardly something someone can
> > > rely on since the non-consumed modifier ("%<(10,bad)") shows up verbatim
> > > in the pretty output.
> >
> > Ideally I'd expect us to die when seeing misformatted placeholders like
> > this. This is way less confusing to the user as otherwise things _look_
> > like they work, but we silently do the wrong thing.
> 
> Right. I can see how it makes some kind of sense to print what we don't
> understand when it's something short and simple like "%X". But for more
> complex "%X(first,second)" it's kind of obvious that a misspelled
> "X(fist,second)" isn't something you want in the output. The whole "if
> we can't parse, return zero as the number of consumed characters so that
> we can print verbatim while looking for next '%'" is a central piece of
> the design here. One could certainly imagine a "strict" mode.
> 
> > That being said, I have no idea whether we can do such a change now
> > without breaking existing usecases. As you rightfully argue the result
> > already is wrong, but with my proposal we'd completely refuse to do
> > anything. Which I'd argue is a good thing in the end.
> 
> I can see the value of a strict mode, with command line options and
> config switches and whatnot, maybe even a changed default behavior at
> some point. I'd rather punt on that for now. TBH, I'd be afraid to do a
> hard switch from "0 means print it instead" to "0 means die". I don't
> disagree that it would be a better end-game though, at some point.

Yup, I fully agree that this is a bit more of a risky change and that it doesn't have to be part of this patch series.

Patrick
Previous: Martin ÅgrenNext: Martin Ågren
Message 12 of 22 in “pretty: minor bugfixing, some refactorings”
  1. 0/8 pretty: minor bugfixing, some refactoringsMartin Ågren, Mar 19, 2025
  2. 1/8 pretty: tighten function signature to not take `void *`Martin Ågren, Mar 19, 2025
  3. Patrick SteinhardtMar 20, 2025
  4. 2/8 pretty: simplify if-else to reduce code duplicationMartin Ågren, Mar 19, 2025
  5. Patrick SteinhardtMar 20, 2025
  6. Martin ÅgrenMar 20, 2025
  7. Jeff KingMar 24, 2025
  8. 3/8 pretty: collect padding-related fields in separate structMartin Ågren, Mar 19, 2025
  9. 4/8 pretty: fix parsing of half-valid "%<" and "%>" placeholdersMartin Ågren, Mar 19, 2025
  10. Patrick SteinhardtMar 20, 2025
  11. Martin ÅgrenMar 20, 2025
  12. Patrick SteinhardtMar 24, 2025
  13. 5/8 pretty: after padding, reset padding infoMartin Ågren, Mar 19, 2025
  14. Patrick SteinhardtMar 20, 2025
  15. Martin ÅgrenMar 20, 2025
  16. 6/8 pretty: refactor parsing of line-wrapping "%w" placeholderMartin Ågren, Mar 19, 2025
  17. Patrick SteinhardtMar 20, 2025
  18. Martin ÅgrenMar 20, 2025
  19. 7/8 pretty: refactor parsing of magicMartin Ågren, Mar 19, 2025
  20. Patrick SteinhardtMar 20, 2025
  21. Martin ÅgrenMar 20, 2025
  22. 8/8 pretty: refactor parsing of decoration optionsMartin Ågren, Mar 19, 2025

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.