Re: [PATCH 04/10] branch: roll show_detached HEAD into regular ref_list

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Aug 11, 2015, 02:41 UTC
Message-ID: <CAPig+cRvdpv_NO8jCrWmFfpuQ+mcxaC5P_oHVrU+tQoH9DKnkg@mail.gmail.com>
In-Reply-To: <1438693282-15516-4-git-send-email-Karthik.188@gmail.com>

On Tue, Aug 4, 2015 at 9:01 AM, Karthik Nayak <karthik.188@gmail.com> wrote:

Show 49 quoted lines

> Remove show_detached() and make detached HEAD to be rolled into
> regular ref_list by adding REF_DETACHED_HEAD as a kind of branch and
> supporting the same in append_ref(). This eliminates the need for an
> extra function and helps in easier porting of branch.c to use
> ref-filter APIs.
>
> Before show_detached() used to check if the HEAD branch satisfies the
> '--contains' option, now that is taken care by append_ref().
>
> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
> ---
> diff --git a/builtin/branch.c b/builtin/branch.c
> index 65f6d0d..81815c9 100644
> --- a/builtin/branch.c
> +++ b/builtin/branch.c
> @@ -535,6 +540,7 @@ static void print_ref_item(struct ref_item *item, int maxwidth, int verbose,
>         int color;
>         struct strbuf out = STRBUF_INIT, name = STRBUF_INIT;
>         const char *prefix = "";
> +       const char *desc = item->name;
>
>         if (item->ignore)
>                 return;
> @@ -547,6 +553,10 @@ static void print_ref_item(struct ref_item *item, int maxwidth, int verbose,
>                 color = BRANCH_COLOR_REMOTE;
>                 prefix = remote_prefix;
>                 break;
> +       case REF_DETACHED_HEAD:
> +               color = BRANCH_COLOR_CURRENT;
> +               desc = get_head_description();
> +               break;
>         default:
>                 color = BRANCH_COLOR_PLAIN;
>                 break;
> @@ -558,7 +568,7 @@ static void print_ref_item(struct ref_item *item, int maxwidth, int verbose,
>                 color = BRANCH_COLOR_CURRENT;
>         }
>
> -       strbuf_addf(&name, "%s%s", prefix, item->name);
> +       strbuf_addf(&name, "%s%s", prefix, desc);
>         if (verbose) {
>                 int utf8_compensation = strlen(name.buf) - utf8_strwidth(name.buf);
>                 strbuf_addf(&out, "%c %s%-*s%s", c, branch_get_color(color),
> @@ -581,6 +591,8 @@ static void print_ref_item(struct ref_item *item, int maxwidth, int verbose,
>         }
>         strbuf_release(&name);
>         strbuf_release(&out);
> +       if (item->kind == REF_DETACHED_HEAD)
> +               free((void *)desc);

This would be cleaner, and more easily extended to other cases if you used a 'to_free' variable. For instance, something like this:

    const char *desc = item->name;
    char *to_free = NULL;
    ...
    case REF_DETACHED_HEAD:
        ...
        desc = to_free = get_head_description();
        break;
    ...
    strbuf_addf(&name, "%s%s", prefix, desc);
    ...
    free(to_free);

Note that it's safe to free 'to_free' when it's NULL, so you don't need to protect the free() with that ugly specialized 'if' which checks for REF_DETACHED_HEAD. You can also do away with the "cast to non-const" when freeing.

Show 7 quoted lines

>  }
> @@ -642,7 +638,14 @@ static int print_ref_list(int kinds, int detached, int verbose, int abbrev, stru
>         cb.ref_list = &ref_list;
>         cb.pattern = pattern;
>         cb.ret = 0;
> +       /*
> +        * First we obtain all regular branch refs and then if the

s/then//

Show 6 quoted lines

> +        * HEAD is detached then we insert that ref to the end of the
> +        * ref_fist so that it can be printed first.
> +        */
>         for_each_rawref(append_ref, &cb);
> +       if (detached)
> +               head_ref(append_ref, &cb);