diff mbox series

[v4,1/8] help.c: BUG() out if "help --guides" can't remove "git" prefixes

Message ID patch-v4-1.8-4428f0a6fb1-20220718T132911Z-avarab@gmail.com (mailing list archive)
State New, archived
Headers show
Series git doc + "git help": move "format" docs from technical/* | expand

Commit Message

Ævar Arnfjörð Bjarmason July 18, 2022, 1:29 p.m. UTC
Adjust code added in 929d9192828 (git docs: split "User-facing file
formats" off from "Guides", 2021-06-04) to be more strict about the
prefix trimming of the "guides" category.

There are no guides in the command-list.txt that don't start with
"git", and we're unlikely to ever add any, if we do we can remove this
BUG() invocation, but in the meantime this makes the intent more
clear.

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@gmail.com>
---
 help.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

Comments

Junio C Hamano July 18, 2022, 5:03 p.m. UTC | #1
Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> Adjust code added in 929d9192828 (git docs: split "User-facing file
> formats" off from "Guides", 2021-06-04) to be more strict about the
> prefix trimming of the "guides" category.
>
> There are no guides in the command-list.txt that don't start with
> "git", and we're unlikely to ever add any, if we do we can remove this
> BUG() invocation, but in the meantime this makes the intent more
> clear.

The observation is good.

>  	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:
> +		if (!skip_prefix(name, "git", &new_name))
> +			BUG("category #%d but no 'git' prefix?", category);
>  		return new_name;
> +	}
>  	return name;
> -
>  }

If we were to use "switch", can we have the opening brace on the
same line, like all other switch statements?

I suspect that we should avoid losing information, perhaps the most
straight-forward implementation would be without "switch", like so

	if (category == CAT_guide) {
		if (skip_prefix(name, "git", &new_name))
			return new_name;
		BUG("CAT_guide category but no 'git' prefix? '%s'", name);
	}
	return name;

even if we plan to add more categories that would remove the prefix
and we insist the prefix to be there?

>  static void extract_cmds(struct cmdname_help **p_cmds, uint32_t mask)
diff mbox series

Patch

diff --git a/help.c b/help.c
index 41c41c2aa11..24ac50f62fe 100644
--- a/help.c
+++ b/help.c
@@ -47,10 +47,14 @@  static const char *drop_prefix(const char *name, uint32_t category)
 
 	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:
+		if (!skip_prefix(name, "git", &new_name))
+			BUG("category #%d but no 'git' prefix?", category);
 		return new_name;
+	}
 	return name;
-
 }
 
 static void extract_cmds(struct cmdname_help **p_cmds, uint32_t mask)