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

Re: [PATCH 5/8] pretty: after padding, reset padding info

From
Patrick Steinhardt <ps@pks.im>
Date
Mar 20, 2025, 09:18 UTC
Message-ID
<Z9vdTmVbIJLa9PGO@pks.im>
In-Reply-To
<e34ae37982e76179aee780c70b48aaaf959a307b.1742367347.git.martin.agren@gmail.com>
On Wed, Mar 19, 2025 at 08:23:38AM +0100, Martin Ågren wrote:
Show 21 quoted lines
> After handling a padding directive ("%<" or "%>"), we leave the `struct
> padding_args` in a halfway state. We modify it a bit as we apply the
> padding/truncation so that by the time we're done, it can't be in quite
> as many states as when we started. Still, we don't fully restore it to
> its default, no-action state.
> 
> "%<" and "%>" should only affect the next placeholder, but leaving a bit
> of state around doesn't make it obvious that we don't spill any of it
> into our handling of later placeholders. The previous commit closed off
> a way of populating only half the `struct padding_args`, thereby fixing
> a bug that *also* relied on then having the other half contain this kind
> of lingering data.
> 
> After that fix, I haven't figured out a way to provoke a bug using just
> this here half of the issue. Still, after handling padding, let's drop
> all remnants of the previous "%<" or "%>".
> 
> Unlike the bug fixed in the previous commit, this could have some
> realistic chance of regressing something for someone if they've actually
> been using such state leftovers (knowingly or not). Still, it seems
> worthwhile to try to tighten this.

Yeah, I agree. It's very surprising that we retain only a subset of state, and that does feel like a bug to me.

Show 21 quoted lines
> This change to pretty.c would have been sufficient to make the test
> added in the previous commit pass. Belt and suspenders.
> 
> Signed-off-by: Martin Ågren <martin.agren@gmail.com>
> ---
>  pretty.c                      | 2 ++
>  t/t4205-log-pretty-formats.sh | 9 +++++++++
>  2 files changed, 11 insertions(+)
> 
> diff --git a/pretty.c b/pretty.c
> index a4fa052f8b..f53e77ed86 100644
> --- a/pretty.c
> +++ b/pretty.c
> @@ -1893,6 +1893,8 @@ static size_t format_and_pad_commit(struct strbuf *sb, /* in UTF-8 */
>  	}
>  	strbuf_release(&local_sb);
>  	c->pad.flush_type = no_flush;
> +	c->pad.truncate = trunc_none;
> +	c->pad.padding = 0;
>  	return total_consumed;
>  }

This is using the same default values now as you started to use in the preceding commit. It might make sense to introduce a macro or function to initialize the structure so that we don't duplicate initialization.

Patrick
Previous: Martin ÅgrenNext: Martin Ågren
Message 14 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.