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

Re: [PATCH] git: replace strbuf_addstr with strbuf_addch for all strings of length 2

From
Eric Sunshine <sunshine@sunshineco.com>
Date
Jan 18, 2023, 18:53 UTC
Message-ID
<CAPig+cTrc7V+P6h039EsPwwRE4rw-qYGKHgGnGmp0TohfzHykA@mail.gmail.com>
In-Reply-To
<xmqqr0vr6d80.fsf@gitster.g>
On Wed, Jan 18, 2023 at 11:14 AM Junio C Hamano <gitster@pobox.com> wrote:
Show 15 quoted lines
> > From: Seija Kijin <doremylover123@gmail.com>
> > This helps reduce overhead of calculating the length
>
> > diff --git a/builtin/am.c b/builtin/am.c
> >       strbuf_addstr(&sb, "GIT_AUTHOR_NAME=");
> >       sq_quote_buf(&sb, state->author_name);
> > -     strbuf_addch(&sb, '\n');
> >
> > -     strbuf_addstr(&sb, "GIT_AUTHOR_EMAIL=");
> > +     strbuf_addstr(&sb, "\nGIT_AUTHOR_EMAIL=");
>
> This may reduce the number of lines, but markedly worsens the
> readability of the resulting code.  Each of the three-line blocks in
> the original used to be logically complete and independent unit, but
> now each of them depend on what the last block wants.
Very much agree with this and all your other review comments.
Show 7 quoted lines
> > -             strbuf_addchars(dest, ' ', 2);
> > -             strbuf_addstr(dest, "From inner merge:");
> > +             strbuf_addstr(dest, "  From inner merge:");
> >               strbuf_addchars(dest, ' ', opt->priv->call_depth * 2);
>
> Ditto, even though this is not as horrible as the change to builtin/am.c
> we saw earlier.

Additionally, if this literal string ever gets wrapped in `_(...)`, then the above change is even more undesirable due to the extra burden it places on translators.

Previous: Junio C HamanoNext: AreaZR via GitGitGadget
Message 3 of 6 in “git: replace strbuf_addstr with strbuf_addch for all strings of length 2”
  1. git: replace strbuf_addstr with strbuf_addch for all strings of length 2Rose via GitGitGadget, Jan 17, 2023
  2. Junio C HamanoJan 18, 2023
  3. Eric SunshineJan 18, 2023
  4. git: replace strbuf_addstr with strbuf_addch for all strings of length 2AreaZR via GitGitGadget, Dec 18, 2024
  5. git: replace strbuf_addstr with strbuf_addch for all strings of length 2AreaZR via GitGitGadget, Dec 18, 2024
  6. git: replace strbuf_addstr with strbuf_addch for all strings of length 2AreaZR via GitGitGadget, Dec 18, 2024

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.