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

Re: [PATCH 5/5] strbuf: simplify strbuf_expand_literal_cb()

From
Jeff King <peff@peff.net>
Date
Jun 27, 2023, 20:21 UTC
Message-ID
<20230627202159.GC1358527@coredump.intra.peff.net>
In-Reply-To
<3654f95d-5709-28f6-eeb2-ca62a1ee789c@web.de>
On Tue, Jun 27, 2023 at 06:32:22PM +0200, René Scharfe wrote:
Show 12 quoted lines
> A format-advancing version would also look a bit weird in an if/else
> cascade:
> 
> 	else if (strbuf_expand_literal(&sb, &format))
> 		; /* nothing */
> 	else ...
> 
> > I dunno. It is a little thing, and I am OK with it either way (I would
> > not even think of changing it if you were not already touching the
> > function).
> 
> Unsure.  Should I overcome my horror vacui and let the /* nothing */ in?

Heh. It is a little funny to have an empty block. But at the same time, it aligns the conditional with all of the skip_prefix() calls, which are advancing "format" as a side effect of matching.

So I could go either way (and we can always change it on top).

I think based on your responses that there's nothing that would require a re-roll. The only thing left from my review is the extra parentheses in format_commit_item, which could possibly be fixed up while applying.

-Peff
Previous: René Scharfe
Message 20 of 20 in “replace strbuf_expand()”
  1. 0/5 replace strbuf_expand()René Scharfe, Jun 17, 2023
  2. 1/5 pretty: factor out expand_separator()René Scharfe, Jun 17, 2023
  3. 2/5 strbuf: factor out strbuf_expand_step()René Scharfe, Jun 17, 2023
  4. Taylor BlauJun 19, 2023
  5. René ScharfeJun 20, 2023
  6. Taylor BlauJun 21, 2023
  7. Jeff KingJun 27, 2023
  8. René ScharfeJun 27, 2023
  9. 3/5 replace strbuf_expand_dict_cb() with strbuf_expand_step()René Scharfe, Jun 17, 2023
  10. Jeff KingJun 27, 2023
  11. Jeff KingJun 27, 2023
  12. René ScharfeJun 27, 2023
  13. 4/5 replace strbuf_expand() with strbuf_expand_step()René Scharfe, Jun 17, 2023
  14. Jeff KingJun 27, 2023
  15. René ScharfeJun 27, 2023
  16. Jeff KingJun 27, 2023
  17. 5/5 strbuf: simplify strbuf_expand_literal_cb()René Scharfe, Jun 17, 2023
  18. Jeff KingJun 27, 2023
  19. René ScharfeJun 27, 2023
  20. Jeff KingJun 27, 2023

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.