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

Re: [PATCH v2] builtin-branch: improve output when displaying remote branches

From
Jay Soffian <jaysoffian@gmail.com>
Date
Feb 13, 2009, 06:45 UTC
Message-ID
<76718490902122245q6f530bb5sc85a65e065966eda@mail.gmail.com>
In-Reply-To
<7v7i3ug7y9.fsf@gitster.siamese.dyndns.org>
On Fri, Feb 13, 2009 at 1:35 AM, Junio C Hamano <gitster@pobox.com> wrote:
Show 68 quoted lines
> Jay Soffian <jaysoffian@gmail.com> writes:
>
>> +     if (prefix && !prefixcmp(dst, prefix))
>> +             return xstrdup(skip_prefix(dst, prefix));
>> +     else
>> +             return xstrdup(dst);
>> +}
>
> I wonder modern compilers are clever enough to optimze the above to
> something more like:
>
>        pfxlen = prefix ? strlen(prefix) : 0;
>        if (pfxlen && !strncmp(dst, prefix, pfxlen))
>                return xstrdup(dst + pfxlen);
>        else
>                return xstrdup(dst);
>
> given that skip_prefix is an inline function but prefixcmp is not
> (anymore), perhaps not.
>
>>  static int append_ref(const char *refname, const unsigned char *sha1, int flags, void *cb_data)
>>  {
>>       struct ref_list *ref_list = (struct ref_list*)(cb_data);
>>       struct ref_item *newitem;
>>       struct commit *commit;
>>       int kind;
>> -     int len;
>> +     const char *prefix, *orig_refname = refname;
>>
>>       /* Detect kind */
>>       if (!prefixcmp(refname, "refs/heads/")) {
>>               kind = REF_LOCAL_BRANCH;
>>               refname += 11;
>> +             prefix = "refs/heads/";
>>       } else if (!prefixcmp(refname, "refs/remotes/")) {
>>               kind = REF_REMOTE_BRANCH;
>>               refname += 13;
>> +             prefix = "refs/remotes/";
>>       } else
>>               return 0;
>
> Once you start making each case arm do more things, it might make sense to
> rewrite the above unrolled loop into something like this:
>
>        static struct {
>                int kind;
>                const char *prefix;
>                int pfxlen;
>        } ref_kind[] = {
>                { REF_LOCAL_BRANCH, "refs/heads/", 11 },
>                { REF_REMOTE_BRANCH, "refs/remotes/", 13 },
>        };
>
>        for (i = 0; i < ARRAY_SIZE(ref_kind); i++) {
>                prefix = ref_kind[i].prefix;
>                if (strncmp(refname, prefix, ref_kind[i].pfxlen))
>                        continue;
>                kind = ref_kind[i].kind;
>                refname += ref_kind[i].pfxlen;
>                break;
>        }
>        if (ARRAY_SIZE(ref_kind) <= i)
>                return 0;
>
> Then we can later add new elements more easily, e.g.
>
>                { REF_TOPGIT_BASE, "refs/top-base/", 14 },
> ;-)

This strikes me as premature optimization. We're just emitting a few branch names here. I'm beginning to lose my motivation to keep working on this patch. I just wanted to improve the UI slightly. :-(

j.
Previous: Junio C HamanoNext: Junio C Hamano
Message 8 of 14 in “builtin-branch: improve output when displaying remote branches”
  1. builtin-branch: improve output when displaying remote branchesJay Soffian, Feb 10, 2009
  2. Jay SoffianFeb 11, 2009
  3. Junio C HamanoFeb 12, 2009
  4. Jay SoffianFeb 12, 2009
  5. Junio C HamanoFeb 12, 2009
  6. builtin-branch: improve output when displaying remote branchesJay Soffian, Feb 13, 2009
  7. Junio C HamanoFeb 13, 2009
  8. Jay SoffianFeb 13, 2009
  9. Junio C HamanoFeb 13, 2009
  10. Jay SoffianFeb 13, 2009
  11. builtin-branch: improve output when displaying remote branchesJay Soffian, Feb 13, 2009
  12. martin f krafftFeb 13, 2009
  13. Junio C HamanoFeb 13, 2009
  14. Johannes SixtFeb 13, 2009

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.