* [PATCH 0/3] Improve consistency of git-var
@ 2022-11-24 20:22 Sean Allred via GitGitGadget
2022-11-24 20:22 ` [PATCH 1/3] var: do not print usage() with a correct invocation Sean Allred via GitGitGadget
` (3 more replies)
0 siblings, 4 replies; 15+ messages in thread
From: Sean Allred via GitGitGadget @ 2022-11-24 20:22 UTC (permalink / raw)
To: git; +Cc: Sean Allred
This patch series makes a few distinct improvements to git-var to support
the change to git_editor() prompted here
[https://lore.kernel.org/git/xmqq1qpwwwxg.fsf@gitster.g/] and ultimately
support that patch to introduce GIT_SEQUENCE_EDITOR as a handled logical
variable.
We first have to pull apart the errors of 'the given logical variable is
unknown/meaningless' and 'the given logical variable is known, but its value
is undefined'. For example, if GIT_EDITOR (and its fallbacks) was completely
unset, git var GIT_EDITOR would end up inappropriately printing a usage
message. This is fixed in var.c by returning the git_var struct itself in
the search on git_vars (to see if the variable is known) and then calling
git_var->read() -- allowing us to handle the cases of 'git_var is null' and
'read() returned null' separately.
After this is done, we're able to remove the handling in var.c:editor()
that's been duplicated in editor.c -- allowing editor() to return NULL and
follow the logic prepared above.
Sean Allred (3):
var: do not print usage() with a correct invocation
var: remove read_var
var: allow GIT_EDITOR to return null
Documentation/git-var.txt | 3 +-
builtin/var.c | 26 +++++++--------
t/t0007-git-var.sh | 69 +++++++++++++++++++++++++++++++++++++++
3 files changed, 83 insertions(+), 15 deletions(-)
base-commit: a0789512c5a4ae7da935cd2e419f253cb3cb4ce7
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1434%2Fvermiculus%2Fsa%2Fvar-improvements-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1434/vermiculus/sa/var-improvements-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1434
--
gitgitgadget
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 1/3] var: do not print usage() with a correct invocation
2022-11-24 20:22 [PATCH 0/3] Improve consistency of git-var Sean Allred via GitGitGadget
@ 2022-11-24 20:22 ` Sean Allred via GitGitGadget
2022-11-24 20:22 ` [PATCH 2/3] var: remove read_var Sean Allred via GitGitGadget
` (2 subsequent siblings)
3 siblings, 0 replies; 15+ messages in thread
From: Sean Allred via GitGitGadget @ 2022-11-24 20:22 UTC (permalink / raw)
To: git; +Cc: Sean Allred, Sean Allred
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
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'.
Signed-off-by: Sean Allred <allred.sean@gmail.com>
---
Documentation/git-var.txt | 3 ++-
builtin/var.c | 19 ++++++++++++++++++-
2 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-var.txt b/Documentation/git-var.txt
index 6aa521fab23..0ab5bfa7d72 100644
--- a/Documentation/git-var.txt
+++ b/Documentation/git-var.txt
@@ -13,7 +13,8 @@ SYNOPSIS
DESCRIPTION
-----------
-Prints a Git logical variable.
+Prints a Git logical variable. Exits with code 1 if the variable has
+no value.
OPTIONS
-------
diff --git a/builtin/var.c b/builtin/var.c
index 491db274292..776f1778ae1 100644
--- a/builtin/var.c
+++ b/builtin/var.c
@@ -56,6 +56,17 @@ static void list_vars(void)
printf("%s=%s\n", ptr->name, val);
}
+static const struct git_var *get_git_var(const char *var)
+{
+ struct git_var *ptr;
+ for (ptr = git_vars; ptr->read; ptr++) {
+ if (strcmp(var, ptr->name) == 0) {
+ return ptr;
+ }
+ }
+ return NULL;
+}
+
static const char *read_var(const char *var)
{
struct git_var *ptr;
@@ -81,6 +92,7 @@ static int show_config(const char *var, const char *value, void *cb)
int cmd_var(int argc, const char **argv, const char *prefix)
{
+ const struct git_var *git_var = NULL;
const char *val = NULL;
if (argc != 2)
usage(var_usage);
@@ -91,9 +103,14 @@ int cmd_var(int argc, const char **argv, const char *prefix)
return 0;
}
git_config(git_default_config, NULL);
+
+ git_var = get_git_var(argv[1]);
+ if (!git_var)
+ usage(var_usage);
+
val = read_var(argv[1]);
if (!val)
- usage(var_usage);
+ return 1;
printf("%s\n", val);
--
gitgitgadget
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 2/3] var: remove read_var
2022-11-24 20:22 [PATCH 0/3] Improve consistency of git-var Sean Allred via GitGitGadget
2022-11-24 20:22 ` [PATCH 1/3] var: do not print usage() with a correct invocation Sean Allred via GitGitGadget
@ 2022-11-24 20:22 ` Sean Allred via GitGitGadget
2022-11-25 5:48 ` Junio C Hamano
2022-11-24 20:22 ` [PATCH 3/3] var: allow GIT_EDITOR to return null Sean Allred via GitGitGadget
2022-11-25 16:52 ` [PATCH v2 0/2] Improve consistency of git-var Sean Allred via GitGitGadget
3 siblings, 1 reply; 15+ messages in thread
From: Sean Allred via GitGitGadget @ 2022-11-24 20:22 UTC (permalink / raw)
To: git; +Cc: Sean Allred, Sean Allred
From: Sean Allred <allred.sean@gmail.com>
With our target git_var value now available, we no longer need to call
into read_var() to find its read() function again. This does avoid a
second loop through git_vars, but mostly it just removes a lot of
duplicated logic.
Signed-off-by: Sean Allred <allred.sean@gmail.com>
---
builtin/var.c | 16 +---------------
1 file changed, 1 insertion(+), 15 deletions(-)
diff --git a/builtin/var.c b/builtin/var.c
index 776f1778ae1..e215cd3b0c0 100644
--- a/builtin/var.c
+++ b/builtin/var.c
@@ -67,20 +67,6 @@ static const struct git_var *get_git_var(const char *var)
return NULL;
}
-static const char *read_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 val;
-}
-
static int show_config(const char *var, const char *value, void *cb)
{
if (value)
@@ -108,7 +94,7 @@ int cmd_var(int argc, const char **argv, const char *prefix)
if (!git_var)
usage(var_usage);
- val = read_var(argv[1]);
+ val = git_var->read(IDENT_STRICT);
if (!val)
return 1;
--
gitgitgadget
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 3/3] var: allow GIT_EDITOR to return null
2022-11-24 20:22 [PATCH 0/3] Improve consistency of git-var Sean Allred via GitGitGadget
2022-11-24 20:22 ` [PATCH 1/3] var: do not print usage() with a correct invocation Sean Allred via GitGitGadget
2022-11-24 20:22 ` [PATCH 2/3] var: remove read_var Sean Allred via GitGitGadget
@ 2022-11-24 20:22 ` Sean Allred via GitGitGadget
2022-11-25 16:52 ` [PATCH v2 0/2] Improve consistency of git-var Sean Allred via GitGitGadget
3 siblings, 0 replies; 15+ messages in thread
From: Sean Allred via GitGitGadget @ 2022-11-24 20:22 UTC (permalink / raw)
To: git; +Cc: Sean Allred, Sean Allred
From: Sean Allred <allred.sean@gmail.com>
The handling to die early when there is no EDITOR is valuable when
used in normal code (i.e., editor.c). In git-var, where
null/empty-string is a perfectly valid value to return, it doesn't
make as much sense.
Remove this handling from `git var GIT_EDITOR` so that it does not
fail so noisily when there is no defined editor.
Signed-off-by: Sean Allred <allred.sean@gmail.com>
---
builtin/var.c | 7 +----
t/t0007-git-var.sh | 69 ++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 70 insertions(+), 6 deletions(-)
diff --git a/builtin/var.c b/builtin/var.c
index e215cd3b0c0..77e9ef3081a 100644
--- a/builtin/var.c
+++ b/builtin/var.c
@@ -11,12 +11,7 @@ static const char var_usage[] = "git var (-l | <variable>)";
static const char *editor(int flag)
{
- const char *pgm = git_editor();
-
- if (!pgm && flag & IDENT_STRICT)
- die("Terminal is dumb, but EDITOR unset");
-
- return pgm;
+ return git_editor();
}
static const char *pager(int flag)
diff --git a/t/t0007-git-var.sh b/t/t0007-git-var.sh
index e56f4b9ac59..bdef271c92a 100755
--- a/t/t0007-git-var.sh
+++ b/t/t0007-git-var.sh
@@ -47,6 +47,75 @@ test_expect_success 'get GIT_DEFAULT_BRANCH with configuration' '
)
'
+test_expect_success 'get GIT_EDITOR without configuration' '
+ (
+ sane_unset GIT_EDITOR &&
+ sane_unset VISUAL &&
+ sane_unset EDITOR &&
+ >expect &&
+ ! git var GIT_EDITOR >actual &&
+ test_cmp expect actual
+ )
+'
+
+test_expect_success 'get GIT_EDITOR with configuration' '
+ test_config core.editor foo &&
+ (
+ sane_unset GIT_EDITOR &&
+ sane_unset VISUAL &&
+ sane_unset EDITOR &&
+ echo foo >expect &&
+ git var GIT_EDITOR >actual &&
+ test_cmp expect actual
+ )
+'
+
+test_expect_success 'get GIT_EDITOR with environment variable GIT_EDITOR' '
+ (
+ sane_unset GIT_EDITOR &&
+ sane_unset VISUAL &&
+ sane_unset EDITOR &&
+ echo bar >expect &&
+ GIT_EDITOR=bar git var GIT_EDITOR >actual &&
+ test_cmp expect actual
+ )
+'
+
+test_expect_success 'get GIT_EDITOR with environment variable EDITOR' '
+ (
+ sane_unset GIT_EDITOR &&
+ sane_unset VISUAL &&
+ sane_unset EDITOR &&
+ echo bar >expect &&
+ EDITOR=bar git var GIT_EDITOR >actual &&
+ test_cmp expect actual
+ )
+'
+
+test_expect_success 'get GIT_EDITOR with configuration and environment variable GIT_EDITOR' '
+ test_config core.editor foo &&
+ (
+ sane_unset GIT_EDITOR &&
+ sane_unset VISUAL &&
+ sane_unset EDITOR &&
+ echo bar >expect &&
+ GIT_EDITOR=bar git var GIT_EDITOR >actual &&
+ test_cmp expect actual
+ )
+'
+
+test_expect_success 'get GIT_EDITOR with configuration and environment variable EDITOR' '
+ test_config core.editor foo &&
+ (
+ sane_unset GIT_EDITOR &&
+ sane_unset VISUAL &&
+ sane_unset EDITOR &&
+ echo foo >expect &&
+ EDITOR=bar git var GIT_EDITOR >actual &&
+ test_cmp expect actual
+ )
+'
+
# For git var -l, we check only a representative variable;
# testing the whole output would make our test too brittle with
# respect to unrelated changes in the test suite's environment.
--
gitgitgadget
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 2/3] var: remove read_var
2022-11-24 20:22 ` [PATCH 2/3] var: remove read_var Sean Allred via GitGitGadget
@ 2022-11-25 5:48 ` Junio C Hamano
0 siblings, 0 replies; 15+ messages in thread
From: Junio C Hamano @ 2022-11-25 5:48 UTC (permalink / raw)
To: Sean Allred via GitGitGadget; +Cc: git, Sean Allred, Sean Allred
"Sean Allred via GitGitGadget" <gitgitgadget@gmail.com> writes:
> From: Sean Allred <allred.sean@gmail.com>
>
> With our target git_var value now available, we no longer need to call
> into read_var() to find its read() function again. This does avoid a
> second loop through git_vars, but mostly it just removes a lot of
> duplicated logic.
If I were doing this series, I would probably have written a single
patch for the steps 1 & 2 from the beginning. That way, reviewers
can clearly see what the differences in behaviour between
get_git_var() and read_var() in that patch to see that the single
step is a strict improvement.
Other than that, both patches 1 & 2 look good.
Thanks.
> Signed-off-by: Sean Allred <allred.sean@gmail.com>
> ---
> builtin/var.c | 16 +---------------
> 1 file changed, 1 insertion(+), 15 deletions(-)
>
> diff --git a/builtin/var.c b/builtin/var.c
> index 776f1778ae1..e215cd3b0c0 100644
> --- a/builtin/var.c
> +++ b/builtin/var.c
> @@ -67,20 +67,6 @@ static const struct git_var *get_git_var(const char *var)
> return NULL;
> }
>
> -static const char *read_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 val;
> -}
> -
> static int show_config(const char *var, const char *value, void *cb)
> {
> if (value)
> @@ -108,7 +94,7 @@ int cmd_var(int argc, const char **argv, const char *prefix)
> if (!git_var)
> usage(var_usage);
>
> - val = read_var(argv[1]);
> + val = git_var->read(IDENT_STRICT);
> if (!val)
> return 1;
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 0/2] Improve consistency of git-var
2022-11-24 20:22 [PATCH 0/3] Improve consistency of git-var Sean Allred via GitGitGadget
` (2 preceding siblings ...)
2022-11-24 20:22 ` [PATCH 3/3] var: allow GIT_EDITOR to return null Sean Allred via GitGitGadget
@ 2022-11-25 16:52 ` Sean Allred via GitGitGadget
2022-11-25 16:52 ` [PATCH v2 1/2] var: do not print usage() with a correct invocation Sean Allred via GitGitGadget
` (2 more replies)
3 siblings, 3 replies; 15+ messages in thread
From: Sean Allred via GitGitGadget @ 2022-11-25 16:52 UTC (permalink / raw)
To: git; +Cc: Sean Allred
This patch series makes a few distinct improvements to git-var to support
the change to git_editor() prompted [here][1] and ultimately support that
patch to introduce GIT_SEQUENCE_EDITOR as a handled logical variable.
Changes since v1:
* Fix a whitespace issue in var.c:editor() (where I have my editor
configured to use spaces instead of tabs; whoops)
* Squash this down to two patches as suggested. I typically organize my
commits to make it clear they don't actively break something, but I can
certainly see the value in organizing them differently when there is
already an extremely robust body of automated tests like there is for
Git.
* Rebased on current main; no conflicts.
Sean Allred (2):
var: do not print usage() with a correct invocation
var: allow GIT_EDITOR to return null
Documentation/git-var.txt | 3 +-
builtin/var.c | 26 +++++++--------
t/t0007-git-var.sh | 69 +++++++++++++++++++++++++++++++++++++++
3 files changed, 83 insertions(+), 15 deletions(-)
base-commit: c000d916380bb59db69c78546928eadd076b9c7d
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1434%2Fvermiculus%2Fsa%2Fvar-improvements-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1434/vermiculus/sa/var-improvements-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1434
Range-diff vs v1:
1: d5f571f0bb3 ! 1: a7ff842a3e8 var: do not print usage() with a correct invocation
@@ builtin/var.c: static void list_vars(void)
printf("%s=%s\n", ptr->name, val);
}
+-static const char *read_var(const char *var)
+static const struct git_var *get_git_var(const char *var)
-+{
-+ struct git_var *ptr;
-+ for (ptr = git_vars; ptr->read; ptr++) {
-+ if (strcmp(var, ptr->name) == 0) {
-+ return ptr;
-+ }
-+ }
-+ return NULL;
-+}
-+
- static const char *read_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;
+ }
+ }
+- return val;
++ return NULL;
+ }
+
+ static int show_config(const char *var, const char *value, void *cb)
@@ builtin/var.c: static int show_config(const char *var, const char *value, void *cb)
int cmd_var(int argc, const char **argv, const char *prefix)
@@ builtin/var.c: 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]);
+ if (!git_var)
-+ usage(var_usage);
-+
- val = read_var(argv[1]);
- if (!val)
-- usage(var_usage);
-+ return 1;
+ usage(var_usage);
++ val = git_var->read(IDENT_STRICT);
++ if (!val)
++ return 1;
++
printf("%s\n", val);
+ return 0;
2: 905b109b458 < -: ----------- var: remove read_var
3: 8d49a718038 ! 2: 427cb7b55ac var: allow GIT_EDITOR to return null
@@ builtin/var.c: static const char var_usage[] = "git var (-l | <variable>)";
- die("Terminal is dumb, but EDITOR unset");
-
- return pgm;
-+ return git_editor();
++ return git_editor();
}
static const char *pager(int flag)
--
gitgitgadget
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 1/2] var: do not print usage() with a correct invocation
2022-11-25 16:52 ` [PATCH v2 0/2] Improve consistency of git-var Sean Allred via GitGitGadget
@ 2022-11-25 16:52 ` Sean Allred via GitGitGadget
2022-11-25 22:45 ` Ævar Arnfjörð Bjarmason
2022-11-25 16:52 ` [PATCH v2 2/2] var: allow GIT_EDITOR to return null Sean Allred via GitGitGadget
2022-11-26 14:17 ` [PATCH v3 0/2] Improve consistency of git-var Sean Allred via GitGitGadget
2 siblings, 1 reply; 15+ messages in thread
From: Sean Allred via GitGitGadget @ 2022-11-25 16:52 UTC (permalink / raw)
To: git; +Cc: Sean Allred, Sean Allred
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
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'.
Signed-off-by: Sean Allred <allred.sean@gmail.com>
---
Documentation/git-var.txt | 3 ++-
builtin/var.c | 19 +++++++++++--------
2 files changed, 13 insertions(+), 9 deletions(-)
diff --git a/Documentation/git-var.txt b/Documentation/git-var.txt
index 6aa521fab23..0ab5bfa7d72 100644
--- a/Documentation/git-var.txt
+++ b/Documentation/git-var.txt
@@ -13,7 +13,8 @@ SYNOPSIS
DESCRIPTION
-----------
-Prints a Git logical variable.
+Prints a Git logical variable. Exits with code 1 if the variable has
+no value.
OPTIONS
-------
diff --git a/builtin/var.c b/builtin/var.c
index 491db274292..e215cd3b0c0 100644
--- a/builtin/var.c
+++ b/builtin/var.c
@@ -56,18 +56,15 @@ static void list_vars(void)
printf("%s=%s\n", ptr->name, val);
}
-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;
}
}
- return val;
+ return NULL;
}
static int show_config(const char *var, const char *value, void *cb)
@@ -81,6 +78,7 @@ static int show_config(const char *var, const char *value, void *cb)
int cmd_var(int argc, const char **argv, const char *prefix)
{
+ const struct git_var *git_var = NULL;
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]);
+ if (!git_var)
usage(var_usage);
+ val = git_var->read(IDENT_STRICT);
+ if (!val)
+ return 1;
+
printf("%s\n", val);
return 0;
--
gitgitgadget
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 2/2] var: allow GIT_EDITOR to return null
2022-11-25 16:52 ` [PATCH v2 0/2] Improve consistency of git-var Sean Allred via GitGitGadget
2022-11-25 16:52 ` [PATCH v2 1/2] var: do not print usage() with a correct invocation Sean Allred via GitGitGadget
@ 2022-11-25 16:52 ` Sean Allred via GitGitGadget
2022-11-25 22:48 ` Ævar Arnfjörð Bjarmason
2022-11-26 14:17 ` [PATCH v3 0/2] Improve consistency of git-var Sean Allred via GitGitGadget
2 siblings, 1 reply; 15+ messages in thread
From: Sean Allred via GitGitGadget @ 2022-11-25 16:52 UTC (permalink / raw)
To: git; +Cc: Sean Allred, Sean Allred
From: Sean Allred <allred.sean@gmail.com>
The handling to die early when there is no EDITOR is valuable when
used in normal code (i.e., editor.c). In git-var, where
null/empty-string is a perfectly valid value to return, it doesn't
make as much sense.
Remove this handling from `git var GIT_EDITOR` so that it does not
fail so noisily when there is no defined editor.
Signed-off-by: Sean Allred <allred.sean@gmail.com>
---
builtin/var.c | 7 +----
t/t0007-git-var.sh | 69 ++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 70 insertions(+), 6 deletions(-)
diff --git a/builtin/var.c b/builtin/var.c
index e215cd3b0c0..5678ec68bfe 100644
--- a/builtin/var.c
+++ b/builtin/var.c
@@ -11,12 +11,7 @@ static const char var_usage[] = "git var (-l | <variable>)";
static const char *editor(int flag)
{
- const char *pgm = git_editor();
-
- if (!pgm && flag & IDENT_STRICT)
- die("Terminal is dumb, but EDITOR unset");
-
- return pgm;
+ return git_editor();
}
static const char *pager(int flag)
diff --git a/t/t0007-git-var.sh b/t/t0007-git-var.sh
index e56f4b9ac59..bdef271c92a 100755
--- a/t/t0007-git-var.sh
+++ b/t/t0007-git-var.sh
@@ -47,6 +47,75 @@ test_expect_success 'get GIT_DEFAULT_BRANCH with configuration' '
)
'
+test_expect_success 'get GIT_EDITOR without configuration' '
+ (
+ sane_unset GIT_EDITOR &&
+ sane_unset VISUAL &&
+ sane_unset EDITOR &&
+ >expect &&
+ ! git var GIT_EDITOR >actual &&
+ test_cmp expect actual
+ )
+'
+
+test_expect_success 'get GIT_EDITOR with configuration' '
+ test_config core.editor foo &&
+ (
+ sane_unset GIT_EDITOR &&
+ sane_unset VISUAL &&
+ sane_unset EDITOR &&
+ echo foo >expect &&
+ git var GIT_EDITOR >actual &&
+ test_cmp expect actual
+ )
+'
+
+test_expect_success 'get GIT_EDITOR with environment variable GIT_EDITOR' '
+ (
+ sane_unset GIT_EDITOR &&
+ sane_unset VISUAL &&
+ sane_unset EDITOR &&
+ echo bar >expect &&
+ GIT_EDITOR=bar git var GIT_EDITOR >actual &&
+ test_cmp expect actual
+ )
+'
+
+test_expect_success 'get GIT_EDITOR with environment variable EDITOR' '
+ (
+ sane_unset GIT_EDITOR &&
+ sane_unset VISUAL &&
+ sane_unset EDITOR &&
+ echo bar >expect &&
+ EDITOR=bar git var GIT_EDITOR >actual &&
+ test_cmp expect actual
+ )
+'
+
+test_expect_success 'get GIT_EDITOR with configuration and environment variable GIT_EDITOR' '
+ test_config core.editor foo &&
+ (
+ sane_unset GIT_EDITOR &&
+ sane_unset VISUAL &&
+ sane_unset EDITOR &&
+ echo bar >expect &&
+ GIT_EDITOR=bar git var GIT_EDITOR >actual &&
+ test_cmp expect actual
+ )
+'
+
+test_expect_success 'get GIT_EDITOR with configuration and environment variable EDITOR' '
+ test_config core.editor foo &&
+ (
+ sane_unset GIT_EDITOR &&
+ sane_unset VISUAL &&
+ sane_unset EDITOR &&
+ echo foo >expect &&
+ EDITOR=bar git var GIT_EDITOR >actual &&
+ test_cmp expect actual
+ )
+'
+
# For git var -l, we check only a representative variable;
# testing the whole output would make our test too brittle with
# respect to unrelated changes in the test suite's environment.
--
gitgitgadget
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/2] var: do not print usage() with a correct invocation
2022-11-25 16:52 ` [PATCH v2 1/2] var: do not print usage() with a correct invocation Sean Allred via GitGitGadget
@ 2022-11-25 22:45 ` Ævar Arnfjörð Bjarmason
2022-11-26 13:19 ` Sean Allred
0 siblings, 1 reply; 15+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-11-25 22:45 UTC (permalink / raw)
To: Sean Allred via GitGitGadget; +Cc: git, Sean Allred, Sean Allred
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.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 2/2] var: allow GIT_EDITOR to return null
2022-11-25 16:52 ` [PATCH v2 2/2] var: allow GIT_EDITOR to return null Sean Allred via GitGitGadget
@ 2022-11-25 22:48 ` Ævar Arnfjörð Bjarmason
2022-11-26 13:54 ` Sean Allred
0 siblings, 1 reply; 15+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-11-25 22:48 UTC (permalink / raw)
To: Sean Allred via GitGitGadget; +Cc: git, Sean Allred, Sean Allred
On Fri, Nov 25 2022, Sean Allred via GitGitGadget wrote:
> From: Sean Allred <allred.sean@gmail.com>
> +test_expect_success 'get GIT_EDITOR without configuration' '
> + (
> + sane_unset GIT_EDITOR &&
> + sane_unset VISUAL &&
> + sane_unset EDITOR &&
> + >expect &&
> + ! git var GIT_EDITOR >actual &&
Negate git with "test_must_fail", not "!", this would e.g. hide
segfaults. See t/README's discussion about it.
> + test_cmp expect actual
Looks like this should be:
test_must_fail git ... >out &&
test_must_be_empty out
> +test_expect_success 'get GIT_EDITOR with configuration and environment variable EDITOR' '
> + test_config core.editor foo &&
> + (
> + sane_unset GIT_EDITOR &&
> + sane_unset VISUAL &&
> + sane_unset EDITOR &&
> + echo foo >expect &&
> + EDITOR=bar git var GIT_EDITOR >actual &&
> + test_cmp expect actual
> + )
Perhaps these can all be factored into a helper to hide this repetition
in a function, but maybe not. E.g:
test_git_var () {
cat >expect &&
(
[...common part of subshell ...]
"$@" >actual &&
test_cmp expect actual
)
}
(untested)
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/2] var: do not print usage() with a correct invocation
2022-11-25 22:45 ` Ævar Arnfjörð Bjarmason
@ 2022-11-26 13:19 ` Sean Allred
0 siblings, 0 replies; 15+ messages in thread
From: Sean Allred @ 2022-11-26 13:19 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: Sean Allred via GitGitGadget, git, Sean Allred
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
> 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.
Sadly I don't think there are any logical variables that could be tested
for this behavior until the second patch in the series (where quite a
few tests are added). I did some brief testing with GIT_COMMITTER_IDENT
as the most obvious candidate, but it will still die early if
GIT_COMMITTER_NAME is unset, so it's not a good test case.
If you've got a test case that'll work before the second patch, I'd be
happy to include it here.
>> {
>> + 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.
Nice catch! I've removed the premature assignment to both `git_var` and
`val`. I've updated my branch with this change; I'll send out a v3 later
today.
--
Sean Allred
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 2/2] var: allow GIT_EDITOR to return null
2022-11-25 22:48 ` Ævar Arnfjörð Bjarmason
@ 2022-11-26 13:54 ` Sean Allred
0 siblings, 0 replies; 15+ messages in thread
From: Sean Allred @ 2022-11-26 13:54 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: Sean Allred via GitGitGadget, git, Sean Allred
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
> Negate git with "test_must_fail", not "!", this would e.g. hide
> segfaults. See t/README's discussion about it.
>
>> + test_cmp expect actual
>
> Looks like this should be:
>
> test_must_fail git ... >out &&
> test_must_be_empty out
Nice! I don't know why I didn't look for t/README, but I also found
test_expect_code, which seems to be even more specific as to what is
being expected. I assume it has the same segfault detection.
This has now been incorporated in my branch; I'll submit it in v3 later
today.
>> +test_expect_success 'get GIT_EDITOR with configuration and environment variable EDITOR' '
>> + test_config core.editor foo &&
>> + (
>> + sane_unset GIT_EDITOR &&
>> + sane_unset VISUAL &&
>> + sane_unset EDITOR &&
>> + echo foo >expect &&
>> + EDITOR=bar git var GIT_EDITOR >actual &&
>> + test_cmp expect actual
>> + )
>
> Perhaps these can all be factored into a helper to hide this repetition
> in a function, but maybe not. E.g:
>
> test_git_var () {
> cat >expect &&
> (
> [...common part of subshell ...]
> "$@" >actual &&
> test_cmp expect actual
> )
> }
>
> (untested)
In all honesty, I think too much abstraction would do more harm than
good here. I definitely share the instinct to factor out the common
pieces, but in other codebases I've worked in, that tends to stifle
future changes in the tests themselves.
That said, I can't realistically imagine a world where a
'sane_unset_all_editors' would stifle code changes -- and I think that
accounts for the lion's share of the repetition. I've incorporated such
a helper in my branch now.
If you're not convinced there should be further abstraction, I'd rather
leave things 'stupid simple' -- but if you think this would block merge,
I'd be happy to take a crack at further factoring out what I can.
--
Sean Allred
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v3 0/2] Improve consistency of git-var
2022-11-25 16:52 ` [PATCH v2 0/2] Improve consistency of git-var Sean Allred via GitGitGadget
2022-11-25 16:52 ` [PATCH v2 1/2] var: do not print usage() with a correct invocation Sean Allred via GitGitGadget
2022-11-25 16:52 ` [PATCH v2 2/2] var: allow GIT_EDITOR to return null Sean Allred via GitGitGadget
@ 2022-11-26 14:17 ` Sean Allred via GitGitGadget
2022-11-26 14:17 ` [PATCH v3 1/2] var: do not print usage() with a correct invocation Sean Allred via GitGitGadget
2022-11-26 14:17 ` [PATCH v3 2/2] var: allow GIT_EDITOR to return null Sean Allred via GitGitGadget
2 siblings, 2 replies; 15+ messages in thread
From: Sean Allred via GitGitGadget @ 2022-11-26 14:17 UTC (permalink / raw)
To: git; +Cc: Sean Allred
This patch series makes a few distinct improvements to git-var to support
the change to git_editor() prompted [here][1] and ultimately support that
patch to introduce GIT_SEQUENCE_EDITOR as a handled logical variable.
Changes since v2:
* Nix premature assignment of git_var and val, preferring to let the
compiler tell us when they're being used before init.
* Factor out sane_unset_all_editors for tests to reduce duplication
* Use more specific test_* helper functions
Sean Allred (2):
var: do not print usage() with a correct invocation
var: allow GIT_EDITOR to return null
Documentation/git-var.txt | 3 +-
builtin/var.c | 29 +++++++++---------
t/t0007-git-var.sh | 62 +++++++++++++++++++++++++++++++++++++++
3 files changed, 78 insertions(+), 16 deletions(-)
base-commit: c000d916380bb59db69c78546928eadd076b9c7d
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1434%2Fvermiculus%2Fsa%2Fvar-improvements-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1434/vermiculus/sa/var-improvements-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/1434
Range-diff vs v2:
1: a7ff842a3e8 ! 1: 889fdf877a1 var: do not print usage() with a correct invocation
@@ builtin/var.c: static int show_config(const char *var, const char *value, void *
int cmd_var(int argc, const char **argv, const char *prefix)
{
-+ const struct git_var *git_var = NULL;
- const char *val = NULL;
+- const char *val = NULL;
++ const struct git_var *git_var;
++ const char *val;
++
if (argc != 2)
usage(var_usage);
+
@@ builtin/var.c: int cmd_var(int argc, const char **argv, const char *prefix)
return 0;
}
2: 427cb7b55ac ! 2: 3d8bf3662fe var: allow GIT_EDITOR to return null
@@ builtin/var.c: static const char var_usage[] = "git var (-l | <variable>)";
static const char *pager(int flag)
## t/t0007-git-var.sh ##
+@@ t/t0007-git-var.sh: test_description='basic sanity checks for git var'
+ TEST_PASSES_SANITIZE_LEAK=true
+ . ./test-lib.sh
+
++sane_unset_all_editors () {
++ sane_unset GIT_EDITOR &&
++ sane_unset VISUAL &&
++ sane_unset EDITOR
++}
++
+ test_expect_success 'get GIT_AUTHOR_IDENT' '
+ test_tick &&
+ echo "$GIT_AUTHOR_NAME <$GIT_AUTHOR_EMAIL> $GIT_AUTHOR_DATE" >expect &&
@@ t/t0007-git-var.sh: test_expect_success 'get GIT_DEFAULT_BRANCH with configuration' '
)
'
+test_expect_success 'get GIT_EDITOR without configuration' '
+ (
-+ sane_unset GIT_EDITOR &&
-+ sane_unset VISUAL &&
-+ sane_unset EDITOR &&
-+ >expect &&
-+ ! git var GIT_EDITOR >actual &&
-+ test_cmp expect actual
++ sane_unset_all_editors &&
++ test_expect_code 1 git var GIT_EDITOR >out &&
++ test_must_be_empty out
+ )
+'
+
+test_expect_success 'get GIT_EDITOR with configuration' '
+ test_config core.editor foo &&
+ (
-+ sane_unset GIT_EDITOR &&
-+ sane_unset VISUAL &&
-+ sane_unset EDITOR &&
++ sane_unset_all_editors &&
+ echo foo >expect &&
+ git var GIT_EDITOR >actual &&
+ test_cmp expect actual
@@ t/t0007-git-var.sh: test_expect_success 'get GIT_DEFAULT_BRANCH with configurati
+
+test_expect_success 'get GIT_EDITOR with environment variable GIT_EDITOR' '
+ (
-+ sane_unset GIT_EDITOR &&
-+ sane_unset VISUAL &&
-+ sane_unset EDITOR &&
++ sane_unset_all_editors &&
+ echo bar >expect &&
+ GIT_EDITOR=bar git var GIT_EDITOR >actual &&
+ test_cmp expect actual
@@ t/t0007-git-var.sh: test_expect_success 'get GIT_DEFAULT_BRANCH with configurati
+
+test_expect_success 'get GIT_EDITOR with environment variable EDITOR' '
+ (
-+ sane_unset GIT_EDITOR &&
-+ sane_unset VISUAL &&
-+ sane_unset EDITOR &&
++ sane_unset_all_editors &&
+ echo bar >expect &&
+ EDITOR=bar git var GIT_EDITOR >actual &&
+ test_cmp expect actual
@@ t/t0007-git-var.sh: test_expect_success 'get GIT_DEFAULT_BRANCH with configurati
+test_expect_success 'get GIT_EDITOR with configuration and environment variable GIT_EDITOR' '
+ test_config core.editor foo &&
+ (
-+ sane_unset GIT_EDITOR &&
-+ sane_unset VISUAL &&
-+ sane_unset EDITOR &&
++ sane_unset_all_editors &&
+ echo bar >expect &&
+ GIT_EDITOR=bar git var GIT_EDITOR >actual &&
+ test_cmp expect actual
@@ t/t0007-git-var.sh: test_expect_success 'get GIT_DEFAULT_BRANCH with configurati
+test_expect_success 'get GIT_EDITOR with configuration and environment variable EDITOR' '
+ test_config core.editor foo &&
+ (
-+ sane_unset GIT_EDITOR &&
-+ sane_unset VISUAL &&
-+ sane_unset EDITOR &&
++ sane_unset_all_editors &&
+ echo foo >expect &&
+ EDITOR=bar git var GIT_EDITOR >actual &&
+ test_cmp expect actual
--
gitgitgadget
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v3 1/2] var: do not print usage() with a correct invocation
2022-11-26 14:17 ` [PATCH v3 0/2] Improve consistency of git-var Sean Allred via GitGitGadget
@ 2022-11-26 14:17 ` Sean Allred via GitGitGadget
2022-11-26 14:17 ` [PATCH v3 2/2] var: allow GIT_EDITOR to return null Sean Allred via GitGitGadget
1 sibling, 0 replies; 15+ messages in thread
From: Sean Allred via GitGitGadget @ 2022-11-26 14:17 UTC (permalink / raw)
To: git; +Cc: Sean Allred, Sean Allred
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
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'.
Signed-off-by: Sean Allred <allred.sean@gmail.com>
---
Documentation/git-var.txt | 3 ++-
builtin/var.c | 22 +++++++++++++---------
2 files changed, 15 insertions(+), 10 deletions(-)
diff --git a/Documentation/git-var.txt b/Documentation/git-var.txt
index 6aa521fab23..0ab5bfa7d72 100644
--- a/Documentation/git-var.txt
+++ b/Documentation/git-var.txt
@@ -13,7 +13,8 @@ SYNOPSIS
DESCRIPTION
-----------
-Prints a Git logical variable.
+Prints a Git logical variable. Exits with code 1 if the variable has
+no value.
OPTIONS
-------
diff --git a/builtin/var.c b/builtin/var.c
index 491db274292..5cbe32ec890 100644
--- a/builtin/var.c
+++ b/builtin/var.c
@@ -56,18 +56,15 @@ static void list_vars(void)
printf("%s=%s\n", ptr->name, val);
}
-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;
}
}
- return val;
+ return NULL;
}
static int show_config(const char *var, const char *value, void *cb)
@@ -81,7 +78,9 @@ static int show_config(const char *var, const char *value, void *cb)
int cmd_var(int argc, const char **argv, const char *prefix)
{
- const char *val = NULL;
+ const struct git_var *git_var;
+ const char *val;
+
if (argc != 2)
usage(var_usage);
@@ -91,10 +90,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]);
+ if (!git_var)
usage(var_usage);
+ val = git_var->read(IDENT_STRICT);
+ if (!val)
+ return 1;
+
printf("%s\n", val);
return 0;
--
gitgitgadget
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v3 2/2] var: allow GIT_EDITOR to return null
2022-11-26 14:17 ` [PATCH v3 0/2] Improve consistency of git-var Sean Allred via GitGitGadget
2022-11-26 14:17 ` [PATCH v3 1/2] var: do not print usage() with a correct invocation Sean Allred via GitGitGadget
@ 2022-11-26 14:17 ` Sean Allred via GitGitGadget
1 sibling, 0 replies; 15+ messages in thread
From: Sean Allred via GitGitGadget @ 2022-11-26 14:17 UTC (permalink / raw)
To: git; +Cc: Sean Allred, Sean Allred
From: Sean Allred <allred.sean@gmail.com>
The handling to die early when there is no EDITOR is valuable when
used in normal code (i.e., editor.c). In git-var, where
null/empty-string is a perfectly valid value to return, it doesn't
make as much sense.
Remove this handling from `git var GIT_EDITOR` so that it does not
fail so noisily when there is no defined editor.
Signed-off-by: Sean Allred <allred.sean@gmail.com>
---
builtin/var.c | 7 +-----
t/t0007-git-var.sh | 62 ++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 63 insertions(+), 6 deletions(-)
diff --git a/builtin/var.c b/builtin/var.c
index 5cbe32ec890..a1a2522126f 100644
--- a/builtin/var.c
+++ b/builtin/var.c
@@ -11,12 +11,7 @@ static const char var_usage[] = "git var (-l | <variable>)";
static const char *editor(int flag)
{
- const char *pgm = git_editor();
-
- if (!pgm && flag & IDENT_STRICT)
- die("Terminal is dumb, but EDITOR unset");
-
- return pgm;
+ return git_editor();
}
static const char *pager(int flag)
diff --git a/t/t0007-git-var.sh b/t/t0007-git-var.sh
index e56f4b9ac59..433d242897c 100755
--- a/t/t0007-git-var.sh
+++ b/t/t0007-git-var.sh
@@ -5,6 +5,12 @@ test_description='basic sanity checks for git var'
TEST_PASSES_SANITIZE_LEAK=true
. ./test-lib.sh
+sane_unset_all_editors () {
+ sane_unset GIT_EDITOR &&
+ sane_unset VISUAL &&
+ sane_unset EDITOR
+}
+
test_expect_success 'get GIT_AUTHOR_IDENT' '
test_tick &&
echo "$GIT_AUTHOR_NAME <$GIT_AUTHOR_EMAIL> $GIT_AUTHOR_DATE" >expect &&
@@ -47,6 +53,62 @@ test_expect_success 'get GIT_DEFAULT_BRANCH with configuration' '
)
'
+test_expect_success 'get GIT_EDITOR without configuration' '
+ (
+ sane_unset_all_editors &&
+ test_expect_code 1 git var GIT_EDITOR >out &&
+ test_must_be_empty out
+ )
+'
+
+test_expect_success 'get GIT_EDITOR with configuration' '
+ test_config core.editor foo &&
+ (
+ sane_unset_all_editors &&
+ echo foo >expect &&
+ git var GIT_EDITOR >actual &&
+ test_cmp expect actual
+ )
+'
+
+test_expect_success 'get GIT_EDITOR with environment variable GIT_EDITOR' '
+ (
+ sane_unset_all_editors &&
+ echo bar >expect &&
+ GIT_EDITOR=bar git var GIT_EDITOR >actual &&
+ test_cmp expect actual
+ )
+'
+
+test_expect_success 'get GIT_EDITOR with environment variable EDITOR' '
+ (
+ sane_unset_all_editors &&
+ echo bar >expect &&
+ EDITOR=bar git var GIT_EDITOR >actual &&
+ test_cmp expect actual
+ )
+'
+
+test_expect_success 'get GIT_EDITOR with configuration and environment variable GIT_EDITOR' '
+ test_config core.editor foo &&
+ (
+ sane_unset_all_editors &&
+ echo bar >expect &&
+ GIT_EDITOR=bar git var GIT_EDITOR >actual &&
+ test_cmp expect actual
+ )
+'
+
+test_expect_success 'get GIT_EDITOR with configuration and environment variable EDITOR' '
+ test_config core.editor foo &&
+ (
+ sane_unset_all_editors &&
+ echo foo >expect &&
+ EDITOR=bar git var GIT_EDITOR >actual &&
+ test_cmp expect actual
+ )
+'
+
# For git var -l, we check only a representative variable;
# testing the whole output would make our test too brittle with
# respect to unrelated changes in the test suite's environment.
--
gitgitgadget
^ permalink raw reply related [flat|nested] 15+ messages in thread
end of thread, other threads:[~2022-11-26 14:18 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-24 20:22 [PATCH 0/3] Improve consistency of git-var Sean Allred via GitGitGadget
2022-11-24 20:22 ` [PATCH 1/3] var: do not print usage() with a correct invocation Sean Allred via GitGitGadget
2022-11-24 20:22 ` [PATCH 2/3] var: remove read_var Sean Allred via GitGitGadget
2022-11-25 5:48 ` Junio C Hamano
2022-11-24 20:22 ` [PATCH 3/3] var: allow GIT_EDITOR to return null Sean Allred via GitGitGadget
2022-11-25 16:52 ` [PATCH v2 0/2] Improve consistency of git-var Sean Allred via GitGitGadget
2022-11-25 16:52 ` [PATCH v2 1/2] var: do not print usage() with a correct invocation Sean Allred via GitGitGadget
2022-11-25 22:45 ` Ævar Arnfjörð Bjarmason
2022-11-26 13:19 ` Sean Allred
2022-11-25 16:52 ` [PATCH v2 2/2] var: allow GIT_EDITOR to return null Sean Allred via GitGitGadget
2022-11-25 22:48 ` Ævar Arnfjörð Bjarmason
2022-11-26 13:54 ` Sean Allred
2022-11-26 14:17 ` [PATCH v3 0/2] Improve consistency of git-var Sean Allred via GitGitGadget
2022-11-26 14:17 ` [PATCH v3 1/2] var: do not print usage() with a correct invocation Sean Allred via GitGitGadget
2022-11-26 14:17 ` [PATCH v3 2/2] var: allow GIT_EDITOR to return null Sean Allred via GitGitGadget
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).