Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes:
> Refactor the drop_prefix() function in in help.c to make it easier to
> strip prefixes from categories that aren't "CAT_guide". There are no
> functional changes here, by doing this we make a subsequent functional
> change's diff smaller.
>
> As before we first try to strip "git-" unconditionally, if that works
> we'll return the stripped string. Then we'll strip "git" if the
> command is in "CAT_guide".
OK. From the code structure's point of view, it somehow not exactly
satisfactory that we still need two "skip and then if skipped yield
the remainder" in this function. Especially because we only strip
once.
> This means that we'd in principle strip "git-gitfoo" down to "foo" if
> it's in CAT_guide. That doesn't make much sense, and we don't have
> such an entry in command-list.txt, but let's preserve that behavior
> for now.
I am not sure if that is what the code means.
"git-gitfoo" will become "gitfoo" regardless of what category we are
calling drop_prefix() for, because we will return the resulting name
without falling through to the new switch statement, if the first
"strip 'git-'" succeeds, no?
> While we're at it remove a stray newline that had been added after the
> "return name;" statement.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx>
> ---
> help.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/help.c b/help.c
> index 41c41c2aa11..7e594d291b0 100644
> --- a/help.c
> +++ b/help.c
> @@ -44,13 +44,19 @@ static struct category_description main_categories[] = {
> static const char *drop_prefix(const char *name, uint32_t category)
> {
> const char *new_name;
> + const char *prefix = NULL;
>
> if (skip_prefix(name, "git-", &new_name))
> return new_name;
> - if (category == CAT_guide && skip_prefix(name, "git", &new_name))
> + switch (category) {
> + case CAT_guide:
> + prefix = "git";
> + break;
> + }
> + if (prefix && skip_prefix(name, prefix, &new_name))
> return new_name;
> - return name;
>
> + return name;
> }
The diff algorighm made an interesting choice as to which line to
consider common here. I would have expected to see
return new_name;
+
return name;
-
}
especially after reading the last paragraph of the proposed log
message.
