Re: [PATCH v4 3/8] branch: roll show_detached HEAD into regular ref_list

[Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>]

Karthik Nayak <karthik.188@gmail.com> writes:

> @@ -679,15 +682,20 @@ static int print_ref_list(int kinds, int detached, int verbose, int abbrev, stru
>  	if (verbose)
>  		maxwidth = calc_maxwidth(&ref_list, strlen(remote_prefix));
>  
> -	qsort(ref_list.list, ref_list.index, sizeof(struct ref_item), ref_cmp);
> +	index = ref_list.index;
> +
> +	/* Print detached HEAD before sorting and printing the rest */
> +	if (detached && (ref_list.list[index - 1].kind == REF_DETACHED_HEAD) &&
> +	    !strcmp(ref_list.list[index - 1].name, head)) {
> +		print_ref_item(&ref_list.list[index - 1], maxwidth, verbose, abbrev,
> +			       1, remote_prefix);
> +		index -= 1;
> +	}

I think Eric already mentionned it, but I don't remember the conclusion
and can't find it in the archives. Wouldn't it be cleaner to actually
remove the detached head from the array (do "ref_list.index -= 1"
instead of "index -= 1", and possibly free() what needs to be freed?

If you did so, you wouldn't have any possible confusion between the
local variable "index" and ref_list.index in the code below:

> -	detached = (detached && (kinds & REF_LOCAL_BRANCH));
> -	if (detached && match_patterns(pattern, "HEAD"))
> -		show_detached(&ref_list, maxwidth);
> +	qsort(ref_list.list, index, sizeof(struct ref_item), ref_cmp);
>  
> -	for (i = 0; i < ref_list.index; i++) {
> -		int current = !detached &&
> -			(ref_list.list[i].kind == REF_LOCAL_BRANCH) &&
> +	for (i = 0; i < index; i++) {
> +		int current = !detached && (ref_list.list[i].kind == REF_LOCAL_BRANCH) &&
>  			!strcmp(ref_list.list[i].name, head);
>  		print_ref_item(&ref_list.list[i], maxwidth, verbose,
>  			       abbrev, current, remote_prefix);

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/