"Derrick Stolee via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
> From: Derrick Stolee <derrickstolee@xxxxxxxxxx>
>
> 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@xxxxxxxxxx>
> ---
> 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.
