Re: [PATCH v1 2/2] log: add option to choose which refs to decorate

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

Rafael Ascensão <rafa.almas@gmail.com> writes:

> I agree that describe should receive the "normalize" treatment. However,
> and following the same reasoning, why should describe users adopt the
> rules imposed by --glob? I could argue they're also used to the way it
> works now.
>
> That being said, the suggestion I mentioned earlier would allow to keep
> both current behaviors consistent at the expense of the extra call to
> refs.c::ref_exists().

In any case, updating the "describe" for consistency is something we
can and should leave for later, to be done as a separate topic.

While I agree with you that the consistent behaviour between
commands is desirable, and also I agree with you that given a
pattern $X that does not have any glob char, trying to match $X when
a ref whose name exactly is $X exists and trying to match $X/*
otherwise would give us a consistent semantics without hurting any
existing uses, I do not think you need to pay any extra expense of
calling ref_exists() at all to achieve that.

That is because when $X exists, you already know $X/otherthing does
not exist.  And when $X does not exist, $X/otherthing might.  So a
naive implementation would be just to add two patterns $X and $X/*
to the filter list and be done with it.  If you exactly have
refs/heads/master, even with the naive logic may throw both
refs/heads/master and refs/heads/master/* to the filter list,
nothing will match the latter to contaminate your result (and vice
versa).

A bit more clever implementation "just throw in two items" would go
like this.  It is not all that involved:

 - In load_ref_decorations(), before running add_ref_decoration for
   each ref and head ref, iterate over the elements in the refname
   filter list.  For each element:

   - if item->string has a trailing '/', trim that.

   - store NULL in the item->util field for item whose string field
     has a glob char.

   - store something non-NULL (e.g. item->string) for item whose
     string field does not have a glob char.

 - In add_ref_decoration(), where your previous round iterates over
   filter->{include,exclude}, get rid of normalize_glob_ref() and
   use of real_pattern.  Instead do something like:

	matched = 0;
	if (item->util == NULL) {
		if (!wildmatch(item->string, refname, 0))
                	matched = 1;
	} else {
		const char *rest;
		if (skip_prefix(refname, item->string, &rest) &&
                    (!*rest || *rest == '/'))
			matched = 1;
	}
	if (matched)
		...

   Of course, you would probably want to encapsulate the logic to
   set matched = 1/0 in a helper function, e.g.

	static int match_ref_pattern(const char *refname,
				     const struct string_list_item *item) {
		int matched = 0;
		... do either wildmatch or head match with tail validation
		... depending on the item->util's NULLness (see above)
		return matched;
	}

   and call that from the two loops for exclude and include list.

Hmm?