Re: [PATCH v2 05/10] log-tree: use ref_namespaces instead of if/else-if

[Junio C Hamano <gitster@pobox.com>]

"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Derrick Stolee <derrickstolee@github.com>
>
> The add_ref_decoration() method uses an if/else-if chain to determine if
> a ref matches a known ref namespace that has a special decoration
> category. That decoration type is later used to assign a color when
> writing to stdout.
>
> The newly-added ref_namespaces array contains all namespaces, along
> with information about their decoration type. Check this array instead
> of this if/else-if chain. This reduces our dependency on string literals
> being embedded in the decoration logic.
>
> Signed-off-by: Derrick Stolee <derrickstolee@github.com>
> ---
>  log-tree.c | 26 ++++++++++++++++----------
>  1 file changed, 16 insertions(+), 10 deletions(-)
>
> diff --git a/log-tree.c b/log-tree.c
> index bb80f1a94d2..6075bdd334e 100644
> --- a/log-tree.c
> +++ b/log-tree.c
> @@ -137,6 +137,7 @@ static int ref_filter_match(const char *refname,
>  static int add_ref_decoration(const char *refname, const struct object_id *oid,
>  			      int flags, void *cb_data)
>  {
> +	int i;
>  	struct object *obj;
>  	enum object_type objtype;
>  	enum decoration_type deco_type = DECORATION_NONE;
> @@ -167,16 +168,21 @@ static int add_ref_decoration(const char *refname, const struct object_id *oid,
>  		return 0;
>  	obj = lookup_object_by_type(the_repository, oid, objtype);
>  
> -	if (starts_with(refname, "refs/heads/"))
> -		deco_type = DECORATION_REF_LOCAL;
> -	else if (starts_with(refname, "refs/remotes/"))
> -		deco_type = DECORATION_REF_REMOTE;
> -	else if (starts_with(refname, "refs/tags/"))
> -		deco_type = DECORATION_REF_TAG;
> -	else if (!strcmp(refname, "refs/stash"))
> -		deco_type = DECORATION_REF_STASH;
> -	else if (!strcmp(refname, "HEAD"))
> -		deco_type = DECORATION_REF_HEAD;
> +	for (i = 0; i < NAMESPACE__COUNT; i++) {
> +		struct ref_namespace_info *info = &ref_namespaces[i];
> +
> +		if (!info->decoration)
> +			continue;
> +		if (info->exact) {
> +			if (!strcmp(refname, info->ref)) {
> +				deco_type = info->decoration;
> +				break;
> +			}
> +		} else if (starts_with(refname, info->ref)) {
> +			deco_type = info->decoration;
> +			break;
> +		}
> +	}

Very nice.  The double-dash in the NAMESPACE__COUNT constant somehow
looks strange.  As we scan through ref_namespace[] array densely,

	for (i = 0; i < ARRAY_SIZE(ref_namespace); i++)
		...

without having to use the constant would probably be more in line
with the way how the rest of the codebase works.