Re: [PATCH v2 05/10] log-tree: use ref_namespaces instead of if/else-if
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Aug 3, 2022, 06:31 UTC
- Message-ID
- <xmqqv8r9dei7.fsf@gitster.g>
- In-Reply-To
- <53b15a0b7932f892505d07a509909b62c473037e.1659122979.git.gitgitgadget@gmail.com>
"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:
Show 58 quoted lines
> 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.