Re: [PATCH v2 1/2] var: do not print usage() with a correct invocation

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Fri, Nov 25 2022, Sean Allred via GitGitGadget wrote:

> From: Sean Allred <allred.sean@gmail.com>
>
> Before, git-var could print usage() even if the command was invoked
> correctly with a variable defined in git_vars -- provided that its
> read() function returned NULL.
>
> Now, we only print usage() only if it was called with a logical

"we only ... only if", drop/combine some "only"?

> variable that wasn't defined -- regardless of read().
>
> Since we now know the variable is valid when we call read_var(), we
> can avoid printing usage() here (and exiting with code 129) and
> instead exit quietly with code 1. While exiting with a different code
> can be a breaking change, it's far better than changing the exit
> status more generally from 'failure' to 'success'.

I honestly don't still don't grok what was different here before/after,
whatever we are now/should be doing here, a test as part of this change
asserting the new behavior would be really useful.

> -static const char *read_var(const char *var)
> +static const struct git_var *get_git_var(const char *var)
>  {
>  	struct git_var *ptr;
> -	const char *val;
> -	val = NULL;
>  	for (ptr = git_vars; ptr->read; ptr++) {
>  		if (strcmp(var, ptr->name) == 0) {
> -			val = ptr->read(IDENT_STRICT);
> -			break;
> +			return ptr;
>  		}

>  {
> +	const struct git_var *git_var = NULL;

This assignment to "NULL" can be dropped, i.e....

>  	const char *val = NULL;
>  	if (argc != 2)
>  		usage(var_usage);
> @@ -91,10 +89,15 @@ int cmd_var(int argc, const char **argv, const char *prefix)
>  		return 0;
>  	}
>  	git_config(git_default_config, NULL);
> -	val = read_var(argv[1]);
> -	if (!val)
> +
> +	git_var = get_git_var(argv[1]);

...we first assign to it here, and if we use it uninit'd before the
compiler will tell us.