Re: [PATCH v2 1/2] var: do not print usage() with a correct invocation
From: Ævar Arnfjörð Bjarmason <hidden>
Date: 2022-11-25 22:48:05
On Fri, Nov 25 2022, Sean Allred via GitGitGadget wrote:
From: Sean Allred <redacted> 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....
quoted hunk ↗ jump to hunk
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.