Re: [PATCH v3 08/12] submodule--helper: report "submodule" as our name in some "-h" output

From: Glen Choo <chooglen@google.com>
Date: Jun 22, 2022, 18:28 UTC
Message-ID: <kl6l4k0ck0y7.fsf@chooglen-macbookpro.roam.corp.google.com>
In-Reply-To: <patch-v3-08.12-bb1310b9abe-20220622T142012Z-avarab@gmail.com>

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

Show 27 quoted lines

> Change the user-facing "git submodule--helper" commands so that
> they'll report their name as being "git submodule". To a user these
> commands are internal implementation details, and it doesn't make
> sense to emit usage about an internal helper when "git submodule" is
> invoked with invalid options.
>
> Before this we'd emit e.g.:
>
> 	$ git submodule absorbgitdirs --blah
> 	error: unknown option `blah'
> 	usage: git submodule--helper absorbgitdirs [<options>] [<path>...]
> 	[...]
> And:
>
> 	$ git submodule set-url -- --
> 	usage: git submodule--helper set-url [--quiet] <path> <newurl>
> 	[...]
>
> Now we'll start with "usage: git submodule [...]" in both of those
> cases. This change does not alter the "list", "name", "clone",
> "config" and "create-branch" commands, those are internal-only (as an
> aside; their usage info should probably invoke BUG(...)). This only
> changes the user-facing commands.
>
> The "status", "deinit" and "update" commands are not included in this
> change, because their usage information already used "submodule"
> rather than "submodule--helper".

I enumerated over all of the commands just to be sure:

- The commands named here and the changed commands cover all of the uses
  of the usage_*/parse_options API.

- We still mention "submodule--helper" in two internal facing commands
  ("is-active" and "push-check"), but those call die() (should be BUG()
  I assume?) instead of usage_*().

- The other two commands ("resolve-relative-url-test" and "check-name")
  are internal facing commands that never print usage information.

So this covers all of the cases. Great!

Show 9 quoted lines

> I don't think it's currently possible to emit some of this usage
> information in practice, as git-submodule.sh will catch unknown
> options, and e.g. it doesn't seem to be possible to get "add" to emit
> its usage information from "submodule--helper".
>
> Though that change may be superfluous now, it's also harmless, and
> will allow us to eventually dispatch further into "git
> submodule--helper" from git-submodule.sh, while emitting the correct
> usage output.

Makes sense.

Show 86 quoted lines

>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>  builtin/submodule--helper.c | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index a298d8d0c12..6af09d78d4b 100644
> --- a/builtin/submodule--helper.c
> +++ b/builtin/submodule--helper.c
> @@ -444,7 +444,7 @@ static int module_foreach(int argc, const char **argv, const char *prefix)
>  	};
>  
>  	const char *const git_submodule_helper_usage[] = {
> -		N_("git submodule--helper foreach [--quiet] [--recursive] [--] <command>"),
> +		N_("git submodule foreach [--quiet] [--recursive] [--] <command>"),
>  		NULL
>  	};
>  
> @@ -582,7 +582,7 @@ static int module_init(int argc, const char **argv, const char *prefix)
>  	};
>  
>  	const char *const git_submodule_helper_usage[] = {
> -		N_("git submodule--helper init [<options>] [<path>]"),
> +		N_("git submodule init [<options>] [<path>]"),
>  		NULL
>  	};
>  
> @@ -1185,7 +1185,7 @@ static int module_summary(int argc, const char **argv, const char *prefix)
>  	};
>  
>  	const char *const git_submodule_helper_usage[] = {
> -		N_("git submodule--helper summary [<options>] [<commit>] [--] [<path>]"),
> +		N_("git submodule summary [<options>] [<commit>] [--] [<path>]"),
>  		NULL
>  	};
>  
> @@ -1349,7 +1349,7 @@ static int module_sync(int argc, const char **argv, const char *prefix)
>  	};
>  
>  	const char *const git_submodule_helper_usage[] = {
> -		N_("git submodule--helper sync [--quiet] [--recursive] [<path>]"),
> +		N_("git submodule sync [--quiet] [--recursive] [<path>]"),
>  		NULL
>  	};
>  
> @@ -2788,7 +2788,7 @@ static int absorb_git_dirs(int argc, const char **argv, const char *prefix)
>  	};
>  
>  	const char *const git_submodule_helper_usage[] = {
> -		N_("git submodule--helper absorbgitdirs [<options>] [<path>...]"),
> +		N_("git submodule absorbgitdirs [<options>] [<path>...]"),
>  		NULL
>  	};
>  
> @@ -2893,7 +2893,7 @@ static int module_set_url(int argc, const char **argv, const char *prefix)
>  		OPT_END()
>  	};
>  	const char *const usage[] = {
> -		N_("git submodule--helper set-url [--quiet] <path> <newurl>"),
> +		N_("git submodule set-url [--quiet] <path> <newurl>"),
>  		NULL
>  	};
>  
> @@ -2932,8 +2932,8 @@ static int module_set_branch(int argc, const char **argv, const char *prefix)
>  		OPT_END()
>  	};
>  	const char *const usage[] = {
> -		N_("git submodule--helper set-branch [-q|--quiet] (-d|--default) <path>"),
> -		N_("git submodule--helper set-branch [-q|--quiet] (-b|--branch) <branch> <path>"),
> +		N_("git submodule set-branch [-q|--quiet] (-d|--default) <path>"),
> +		N_("git submodule set-branch [-q|--quiet] (-b|--branch) <branch> <path>"),
>  		NULL
>  	};
>  
> @@ -3277,7 +3277,7 @@ static int module_add(int argc, const char **argv, const char *prefix)
>  	};
>  
>  	const char *const usage[] = {
> -		N_("git submodule--helper add [<options>] [--] <repository> [<path>]"),
> +		N_("git submodule add [<options>] [--] <repository> [<path>]"),
>  		NULL
>  	};
>  
> -- 
> 2.36.1.1239.gfba91521d90