Johan Herland <johan@xxxxxxxxxxx> writes:
> This patch removes the only remaining user of ref_rev_parse_rules.
> It has now been fully replaced by ref_expand_rules. Hence this patch
> also removes ref_rev_parse_rules.
Yeah, I was wondering when you would do this while reading [4/7], as
removing the parse_rules[] would break shortener side, and leaving
it in for long would risk allowing parse_rules[] and expand_rules[]
to drift apart.
> -const struct ref_expand_rule ref_expand_rules[] = {
> - { ref_expand_txtly, "%.*s" },
> - { ref_expand_txtly, "refs/%.*s" },
> - { ref_expand_txtly, "refs/tags/%.*s" },
> - { ref_expand_txtly, "refs/heads/%.*s" },
> - { ref_expand_txtly, "refs/remotes/%.*s" },
> - { ref_expand_txtly, "refs/remotes/%.*s/HEAD" },
> - { NULL, NULL }
> -};
I wonder if you planned the previous step a bit better, this removal
of a large block of text could have come next to the replacement of
it we see after the addition of ref_shorten_txtly() function.
> +static char *ref_shorten_txtly(const struct ref_expand_rule *rule,
> + const char *refname)
> +{
> +...
> +}
>
> -static const char *ref_rev_parse_rules[] = {
> - "%.*s",
> - "refs/%.*s",
> - "refs/tags/%.*s",
> - "refs/heads/%.*s",
> - "refs/remotes/%.*s",
> - "refs/remotes/%.*s/HEAD",
> - NULL
> +const struct ref_expand_rule ref_expand_rules[] = {
> + { ref_expand_txtly, NULL, "%.*s" },
> + { ref_expand_txtly, ref_shorten_txtly, "refs/%.*s" },
> + { ref_expand_txtly, ref_shorten_txtly, "refs/tags/%.*s" },
> + { ref_expand_txtly, ref_shorten_txtly, "refs/heads/%.*s" },
> + { ref_expand_txtly, ref_shorten_txtly, "refs/remotes/%.*s" },
> + { ref_expand_txtly, ref_shorten_txtly, "refs/remotes/%.*s/HEAD" },
> + { NULL, NULL, NULL }
> };
>
> int refname_match(const char *abbrev_name, const char *full_name,
> char *shorten_unambiguous_ref(const char *refname, int strict)
> {
> int i;
> char *short_name;
>
> - /* buffer for scanf result, at most refname must fit */
> - short_name = xstrdup(refname);
> -
> - /* skip first rule, it will always match */
> - for (i = ARRAY_SIZE(ref_rev_parse_rules) - 1; i > 0 ; --i) {
> + for (i = ARRAY_SIZE(ref_expand_rules) - 1; i >= 0 ; --i) {
> ...
> /*
> * in strict mode, all (except the matched one) rules
> * must fail to resolve to a valid non-ambiguous ref
> */
> if (strict)
> - rules_to_fail = ARRAY_SIZE(ref_rev_parse_rules);
> + rules_to_fail = ARRAY_SIZE(ref_expand_rules);
This part obviously depends on 1/7; do we still have an off-by-one
change from the original, or did I miscount when I reviewed 1/7?
Again, the overall strategy to refactor sounds sound.
It may be a lot simpler if you have ref_expand/shorten_append() and
ref_expand/shortn_append_with_HEAD() built-in helper functions.
Then you can perform the expansion and contraction without "%.*s" at
all.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html