"John Cai via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
> +static int ref_matched(const char *path,
> + const struct string_list *l,
> + const struct string_list *hidden_refs)
> {
> const char *stripped_path = strip_namespace(path);
> struct string_list_item *item;
>
> - for_each_string_list_item(item, &exclusions->excluded_refs) {
> + for_each_string_list_item(item, l) {
> if (!wildmatch(item->string, path, 0))
> return 1;
> }
>
> - if (ref_is_hidden(stripped_path, path, &exclusions->hidden_refs))
> + if (ref_is_hidden(stripped_path, path, hidden_refs))
> return 1;
>
> return 0;
> }
>
> -void init_ref_exclusions(struct ref_exclusions *exclusions)
> +int ref_excluded(const struct ref_visibility *visibility, const char *path)
> {
> - struct ref_exclusions blank = REF_EXCLUSIONS_INIT;
> - memcpy(exclusions, &blank, sizeof(*exclusions));
> + return ref_matched(path, &visibility->excluded_refs, &visibility->hidden_refs);
> }
>
> -void clear_ref_exclusions(struct ref_exclusions *exclusions)
> +int ref_included(const struct ref_visibility *visibility, const char *path)
> {
> - string_list_clear(&exclusions->excluded_refs, 0);
> - string_list_clear(&exclusions->hidden_refs, 0);
> - exclusions->hidden_refs_configured = 0;
> + return ref_matched(path, &visibility->included_refs, &visibility->hidden_refs);
> }
It is unexpected that we do "hidden" inside ref_matched(). I would
have expected that no matter what exclusion or inclusion patterns
say, hidden things are to be kept hidden. I.e. I expected
- ref_matched(), which takes a path and a list of patterns, checks
if the path matches any of the given patterns;
- ref_excluded(), whcih takes a path and a visibility, asks
ref_matched() if the path matches visibility->excluded_refs and
relays its answer to the caller.
- ref_included(), which takes a path and a visibility, asks
ref_matched() if the path matches visibility->included_refs and
relays its answer to the caller.
- ref_visibility(), which takes a path and a visibility, goes
through the following sequence:
- if ref_is_hidden() says that the path is hidden, it is not
visible;
- if ref_excluded() says the path is excluded, it is not visible;
- if ref_included() says the path is included, it is visible;
- if none of the above triggers, the fate of the path is
determined by some default logic.
or something along that line. That would also allow us to avoid
having to call ref_is_hidden() more than once when we need to check
both inclusion and exclusion list.
But perhaps I am missing some requirements to be taken into
account, so let me read on.
To be honest, I didn't expect the "exclusions can be extended",
which I alluded to as a future, possible, follow-on work, to be
included as a part of this topic. I appreciate your going an extra
mile, but I am not sure if it was a good change in the context of
this series. With this patch, it is not trivial to even validate
that there is no behaviour change to any users of "struct
ref_exclusions" when the included_refs list is empty, which is an
absolute must to avoid regressions.
