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.