* [PATCH] Remove --no-gui option from difftool usage string @ 2017-02-04 2:56 Denton Liu 2017-02-04 5:58 ` Jacob Keller 2017-02-05 20:17 ` [BUG] was: " David Aguilar 0 siblings, 2 replies; 12+ messages in thread From: Denton Liu @ 2017-02-04 2:56 UTC (permalink / raw) To: git The --no-gui option not documented in the manpage, nor is it actually used in the source code. This change removes it from the usage help that's printed. Signed-off-by: Denton Liu <liu.denton@gmail.com> --- git-difftool.perl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/git-difftool.perl b/git-difftool.perl index a5790d03a..657d5622d 100755 --- a/git-difftool.perl +++ b/git-difftool.perl @@ -29,8 +29,8 @@ sub usage print << 'USAGE'; usage: git difftool [-t|--tool=<tool>] [--tool-help] [-x|--extcmd=<cmd>] - [-g|--gui] [--no-gui] - [--prompt] [-y|--no-prompt] + [-g|--gui] [--prompt] + [-y|--no-prompt] [-d|--dir-diff] ['git diff' options] USAGE -- 2.11.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] Remove --no-gui option from difftool usage string 2017-02-04 2:56 [PATCH] Remove --no-gui option from difftool usage string Denton Liu @ 2017-02-04 5:58 ` Jacob Keller 2017-02-04 6:23 ` Denton Liu 2017-02-05 20:17 ` [BUG] was: " David Aguilar 1 sibling, 1 reply; 12+ messages in thread From: Jacob Keller @ 2017-02-04 5:58 UTC (permalink / raw) To: Denton Liu; +Cc: Git mailing list On Fri, Feb 3, 2017 at 6:56 PM, Denton Liu <liu.denton@gmail.com> wrote: > The --no-gui option not documented in the manpage, nor is it actually > used in the source code. This change removes it from the usage help > that's printed. > > Signed-off-by: Denton Liu <liu.denton@gmail.com> > --- > git-difftool.perl | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/git-difftool.perl b/git-difftool.perl > index a5790d03a..657d5622d 100755 > --- a/git-difftool.perl > +++ b/git-difftool.perl > @@ -29,8 +29,8 @@ sub usage > print << 'USAGE'; > usage: git difftool [-t|--tool=<tool>] [--tool-help] > [-x|--extcmd=<cmd>] > - [-g|--gui] [--no-gui] > - [--prompt] [-y|--no-prompt] > + [-g|--gui] [--prompt] > + [-y|--no-prompt] > [-d|--dir-diff] > ['git diff' options] > USAGE > -- > 2.11.0 > Aren't "--no-foo" options automatically created for booleans when using our option parsing code? Thanks, Jake ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Remove --no-gui option from difftool usage string 2017-02-04 5:58 ` Jacob Keller @ 2017-02-04 6:23 ` Denton Liu 2017-02-05 10:22 ` David Aguilar 0 siblings, 1 reply; 12+ messages in thread From: Denton Liu @ 2017-02-04 6:23 UTC (permalink / raw) To: Jacob Keller; +Cc: Git mailing list On Fri, Feb 03, 2017 at 09:58:09PM -0800, Jacob Keller wrote: > On Fri, Feb 3, 2017 at 6:56 PM, Denton Liu <liu.denton@gmail.com> wrote: > > The --no-gui option not documented in the manpage, nor is it actually > > used in the source code. This change removes it from the usage help > > that's printed. > > > > Signed-off-by: Denton Liu <liu.denton@gmail.com> > > --- > > git-difftool.perl | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/git-difftool.perl b/git-difftool.perl > > index a5790d03a..657d5622d 100755 > > --- a/git-difftool.perl > > +++ b/git-difftool.perl > > @@ -29,8 +29,8 @@ sub usage > > print << 'USAGE'; > > usage: git difftool [-t|--tool=<tool>] [--tool-help] > > [-x|--extcmd=<cmd>] > > - [-g|--gui] [--no-gui] > > - [--prompt] [-y|--no-prompt] > > + [-g|--gui] [--prompt] > > + [-y|--no-prompt] > > [-d|--dir-diff] > > ['git diff' options] > > USAGE > > -- > > 2.11.0 > > > > Aren't "--no-foo" options automatically created for booleans when > using our option parsing code? > > Thanks, > Jake Sorry, I guess I didn't notice that. Would it make sense to document it in the manpage then? -- Denton Liu ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Remove --no-gui option from difftool usage string 2017-02-04 6:23 ` Denton Liu @ 2017-02-05 10:22 ` David Aguilar 0 siblings, 0 replies; 12+ messages in thread From: David Aguilar @ 2017-02-05 10:22 UTC (permalink / raw) To: Denton Liu; +Cc: Jacob Keller, Git mailing list On Fri, Feb 03, 2017 at 10:23:51PM -0800, Denton Liu wrote: > On Fri, Feb 03, 2017 at 09:58:09PM -0800, Jacob Keller wrote: > > On Fri, Feb 3, 2017 at 6:56 PM, Denton Liu <liu.denton@gmail.com> wrote: > > > The --no-gui option not documented in the manpage, nor is it actually > > > used in the source code. This change removes it from the usage help > > > that's printed. > > > > > > Signed-off-by: Denton Liu <liu.denton@gmail.com> > > > --- > > > git-difftool.perl | 4 ++-- > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git a/git-difftool.perl b/git-difftool.perl > > > index a5790d03a..657d5622d 100755 > > > --- a/git-difftool.perl > > > +++ b/git-difftool.perl > > > @@ -29,8 +29,8 @@ sub usage > > > print << 'USAGE'; > > > usage: git difftool [-t|--tool=<tool>] [--tool-help] > > > [-x|--extcmd=<cmd>] > > > - [-g|--gui] [--no-gui] > > > - [--prompt] [-y|--no-prompt] > > > + [-g|--gui] [--prompt] > > > + [-y|--no-prompt] > > > [-d|--dir-diff] > > > ['git diff' options] > > > USAGE > > > -- > > > 2.11.0 > > > > > > > Aren't "--no-foo" options automatically created for booleans when > > using our option parsing code? > > > > Thanks, > > Jake > > Sorry, I guess I didn't notice that. Would it make sense to document it > in the manpage then? The general "--no-*" convention is mentioned in "git help cli", but the manpages and usage strings across all commands are inconsistent about mentioning the "--no-*" variants; some do, some don't. IMO it probably wouldn't hurt to document --no-gui in the manpage. cheers, -- David ^ permalink raw reply [flat|nested] 12+ messages in thread
* [BUG] was: Re: [PATCH] Remove --no-gui option from difftool usage string 2017-02-04 2:56 [PATCH] Remove --no-gui option from difftool usage string Denton Liu 2017-02-04 5:58 ` Jacob Keller @ 2017-02-05 20:17 ` David Aguilar 2017-02-05 21:23 ` [PATCH] difftool: fix bug when printing usage David Aguilar 1 sibling, 1 reply; 12+ messages in thread From: David Aguilar @ 2017-02-05 20:17 UTC (permalink / raw) To: Denton Liu, Johannes Schindelin; +Cc: git, Junio C Hamano On Fri, Feb 03, 2017 at 06:56:17PM -0800, Denton Liu wrote: > The --no-gui option not documented in the manpage, nor is it actually > used in the source code. This change removes it from the usage help > that's printed. > > Signed-off-by: Denton Liu <liu.denton@gmail.com> > --- > git-difftool.perl | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) [Dscho, I found a bug, see below] I forgot to mention that the scripted difftool has been rewritten in C and will be going away soon. builtin/difftool.c is already in "next". New patches against difftool should target the builtin. Regarding removing it from the usage string, IMO that can be considered a good change if the rationale were instead that we never expect users to ever type "--no-gui" in practice. Wasting the short usage string screen space with a useless to 99.99% users option is bad for usability. From that perspective we shouldn't mention it there, so reframing the commit message towards that argument would make for a better motivation. Removing the mention from the usage string and adding it to the manpage would be the a good change from that perspective as well. BTW, in "next", it seems that the builtin difftool crashes when doing "git difftool -h", so we should probably add a test for that too... From the tip of next: $ git difftool -h fatal: BUG: setup_git_env called without repository -- David ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH] difftool: fix bug when printing usage 2017-02-05 20:17 ` [BUG] was: " David Aguilar @ 2017-02-05 21:23 ` David Aguilar 2017-02-06 16:58 ` Johannes Schindelin 0 siblings, 1 reply; 12+ messages in thread From: David Aguilar @ 2017-02-05 21:23 UTC (permalink / raw) To: Junio C Hamano; +Cc: Git ML, Johannes Schindelin, Denton Liu "git difftool -h" reports an error: fatal: BUG: setup_git_env called without repository Defer repository setup so that the help option processing happens before the repository is initialized. Add tests to ensure that the basic usage works inside and outside of a repository. Signed-off-by: David Aguilar <davvid@gmail.com> --- This bug exists in both "master" and "next". This patch has been tested on both branches. builtin/difftool.c | 8 ++++---- t/t7800-difftool.sh | 13 +++++++++++++ 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/builtin/difftool.c b/builtin/difftool.c index b5e85ab079..d13350ce83 100644 --- a/builtin/difftool.c +++ b/builtin/difftool.c @@ -647,10 +647,6 @@ int cmd_difftool(int argc, const char **argv, const char *prefix) OPT_END() }; - /* NEEDSWORK: once we no longer spawn anything, remove this */ - setenv(GIT_DIR_ENVIRONMENT, absolute_path(get_git_dir()), 1); - setenv(GIT_WORK_TREE_ENVIRONMENT, absolute_path(get_git_work_tree()), 1); - git_config(difftool_config, NULL); symlinks = has_symlinks; @@ -661,6 +657,10 @@ int cmd_difftool(int argc, const char **argv, const char *prefix) if (tool_help) return print_tool_help(); + /* NEEDSWORK: once we no longer spawn anything, remove this */ + setenv(GIT_DIR_ENVIRONMENT, absolute_path(get_git_dir()), 1); + setenv(GIT_WORK_TREE_ENVIRONMENT, absolute_path(get_git_work_tree()), 1); + if (use_gui_tool && diff_gui_tool && *diff_gui_tool) setenv("GIT_DIFF_TOOL", diff_gui_tool, 1); else if (difftool_cmd) { diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh index aa0ef02597..21e2ac4ad6 100755 --- a/t/t7800-difftool.sh +++ b/t/t7800-difftool.sh @@ -23,6 +23,19 @@ prompt_given () test "$prompt" = "Launch 'test-tool' [Y/n]? branch" } +test_expect_success 'basic usage requires no repo' ' + lines=$(git difftool -h | grep ^usage: | wc -l) && + test "$lines" -eq 1 && + # create a ceiling directory to prevent Git from finding a repo + mkdir -p not/repo && + ceiling="$PWD/not" && + lines=$(cd not/repo && + GIT_CEILING_DIRECTORIES="$ceiling" git difftool -h | + grep ^usage: | wc -l) && + test "$lines" -eq 1 && + rmdir -p not/repo +' + # Create a file on master and change it on branch test_expect_success 'setup' ' echo master >file && -- 2.12.0.rc0.209.g3d1e53e462 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] difftool: fix bug when printing usage 2017-02-05 21:23 ` [PATCH] difftool: fix bug when printing usage David Aguilar @ 2017-02-06 16:58 ` Johannes Schindelin 2017-02-06 18:13 ` Junio C Hamano 0 siblings, 1 reply; 12+ messages in thread From: Johannes Schindelin @ 2017-02-06 16:58 UTC (permalink / raw) To: David Aguilar; +Cc: Junio C Hamano, Git ML, Denton Liu Hi David, On Sun, 5 Feb 2017, David Aguilar wrote: > "git difftool -h" reports an error: > > fatal: BUG: setup_git_env called without repository > > Defer repository setup so that the help option processing happens before > the repository is initialized. > > Add tests to ensure that the basic usage works inside and outside of a > repository. > > Signed-off-by: David Aguilar <davvid@gmail.com> > --- > This bug exists in both "master" and "next". > This patch has been tested on both branches. Thanks for noticing and for patching! > diff --git a/builtin/difftool.c b/builtin/difftool.c > index b5e85ab079..d13350ce83 100644 > --- a/builtin/difftool.c > +++ b/builtin/difftool.c > @@ -647,10 +647,6 @@ int cmd_difftool(int argc, const char **argv, const char *prefix) > OPT_END() > }; > > - /* NEEDSWORK: once we no longer spawn anything, remove this */ > - setenv(GIT_DIR_ENVIRONMENT, absolute_path(get_git_dir()), 1); > - setenv(GIT_WORK_TREE_ENVIRONMENT, absolute_path(get_git_work_tree()), 1); > - > git_config(difftool_config, NULL); > symlinks = has_symlinks; > > @@ -661,6 +657,10 @@ int cmd_difftool(int argc, const char **argv, const char *prefix) > if (tool_help) > return print_tool_help(); > > + /* NEEDSWORK: once we no longer spawn anything, remove this */ > + setenv(GIT_DIR_ENVIRONMENT, absolute_path(get_git_dir()), 1); > + setenv(GIT_WORK_TREE_ENVIRONMENT, absolute_path(get_git_work_tree()), 1); > + > if (use_gui_tool && diff_gui_tool && *diff_gui_tool) > setenv("GIT_DIFF_TOOL", diff_gui_tool, 1); > else if (difftool_cmd) { Aaargh. You know, when you are used to code review systems where you can easily increase the context of the diff hunks, static patches are... tedious to review. For the record: the context between those two hunks is: argc = parse_options(argc, argv, prefix, builtin_difftool_options, builtin_difftool_usage, PARSE_OPT_KEEP_UNKNOWN | PARSE_OPT_KEEP_DASHDASH); > diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh > index aa0ef02597..21e2ac4ad6 100755 > --- a/t/t7800-difftool.sh > +++ b/t/t7800-difftool.sh > @@ -23,6 +23,19 @@ prompt_given () > test "$prompt" = "Launch 'test-tool' [Y/n]? branch" > } > > +test_expect_success 'basic usage requires no repo' ' > + lines=$(git difftool -h | grep ^usage: | wc -l) && > + test "$lines" -eq 1 && It may be easier to debug future breakages if you wrote git difftool -h | grep ^usage: >output && test_line_count = 1 output && or even better (changing the semantics now): test_expect_code 129 git difftool -h >output && grep ^usage: output && > + # create a ceiling directory to prevent Git from finding a repo > + mkdir -p not/repo && > + ceiling="$PWD/not" && > + lines=$(cd not/repo && > + GIT_CEILING_DIRECTORIES="$ceiling" git difftool -h | > + grep ^usage: | wc -l) && > + test "$lines" -eq 1 && Likewise, this would become GIT_CEILING_DIRECTORIES="$PWD/not" \ test_expect_code 129 git -C not/repo difftool -h >output && grep ^usage: output > + rmdir -p not/repo I am not sure how portable `rmdir -p` is. Why not use test_when_finished rm -r not instead? But those are all really minor bike-sheddings, and I promised myself that I would avoid those (as I find them rather pointless) unless there is at least one real benefit. In this case, I think the result becomes more readable: -- snip -- diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh index 21e2ac4ad6d..7d7cb63d61e 100755 --- a/t/t7800-difftool.sh +++ b/t/t7800-difftool.sh @@ -24,16 +24,14 @@ prompt_given () } test_expect_success 'basic usage requires no repo' ' - lines=$(git difftool -h | grep ^usage: | wc -l) && - test "$lines" -eq 1 && - # create a ceiling directory to prevent Git from finding a repo - mkdir -p not/repo && - ceiling="$PWD/not" && - lines=$(cd not/repo && - GIT_CEILING_DIRECTORIES="$ceiling" git difftool -h | - grep ^usage: | wc -l) && - test "$lines" -eq 1 && - rmdir -p not/repo + test_expect_code 129 git difftool -h >output && + grep ^usage: output && + # create a ceiling directory to prevent Git from finding a repo + mkdir -p not/repo && + test_when_finished rm -r not && + GIT_CEILING_DIRECTORIES="$PWD/not" \ + test_expect_code 129 git -C not/repo difftool -h >output && + grep ^usage: output ' # Create a file on master and change it on branch -- snap -- More importantly: When I read $PWD all kinds of alarm bells go off in my head, as I immediately think of all the issues we have on Windows due to Git's regression test using POSIX paths all over the place. In this particular instance, it *does* work, magically, because the POSIX path $PWD is automatically converted (by the MSYS2 runtime) into a Windows path when git.exe is called. Read: I actually tested this, in line with my resolution to avoid to review merely patches (which would likely miss serious bugs) and instead try to do full code reviews, including testing. That means that I will review less, but better. Insofar as I am the author of the builtin difftool: Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de> Thanks, Dscho ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] difftool: fix bug when printing usage 2017-02-06 16:58 ` Johannes Schindelin @ 2017-02-06 18:13 ` Junio C Hamano 2017-02-07 11:21 ` Johannes Schindelin 0 siblings, 1 reply; 12+ messages in thread From: Junio C Hamano @ 2017-02-06 18:13 UTC (permalink / raw) To: Johannes Schindelin; +Cc: David Aguilar, Git ML, Denton Liu Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: >> +test_expect_success 'basic usage requires no repo' ' >> + lines=$(git difftool -h | grep ^usage: | wc -l) && >> + test "$lines" -eq 1 && > > It may be easier to debug future breakages if you wrote > > git difftool -h | grep ^usage: >output && > test_line_count = 1 output && > or even better (changing the semantics now): > > test_expect_code 129 git difftool -h >output && > grep ^usage: output && True. >> + # create a ceiling directory to prevent Git from finding a repo >> + mkdir -p not/repo && >> + ceiling="$PWD/not" && >> + lines=$(cd not/repo && >> + GIT_CEILING_DIRECTORIES="$ceiling" git difftool -h | >> + grep ^usage: | wc -l) && >> + test "$lines" -eq 1 && > > Likewise, this would become > > GIT_CEILING_DIRECTORIES="$PWD/not" \ > test_expect_code 129 git -C not/repo difftool -h >output && > grep ^usage: output I agree with the intent, but the execution here is "Not quite". test_expect_code being a shell function, it does not take the "one-shot environment assignment for this single invocation," like external commands do. > More importantly: When I read $PWD all kinds of alarm bells go off in my > head, as I immediately think of all the issues we have on Windows due to > Git's regression test using POSIX paths all over the place. And we appreciate that somebody who is more familiar with the issue is watching ;-). > Insofar as I am the author of the builtin difftool: > > Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de> OK, thanks. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] difftool: fix bug when printing usage 2017-02-06 18:13 ` Junio C Hamano @ 2017-02-07 11:21 ` Johannes Schindelin 2017-02-07 20:03 ` Junio C Hamano 0 siblings, 1 reply; 12+ messages in thread From: Johannes Schindelin @ 2017-02-07 11:21 UTC (permalink / raw) To: Junio C Hamano; +Cc: David Aguilar, Git ML, Denton Liu Hi Junio, On Mon, 6 Feb 2017, Junio C Hamano wrote: > Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: > > > [... and he quoted someone ...] > > > >> + # create a ceiling directory to prevent Git from finding a repo > >> + mkdir -p not/repo && > >> + ceiling="$PWD/not" && > >> + lines=$(cd not/repo && > >> + GIT_CEILING_DIRECTORIES="$ceiling" git difftool -h | > >> + grep ^usage: | wc -l) && > >> + test "$lines" -eq 1 && > > > > Likewise, this would become > > > > GIT_CEILING_DIRECTORIES="$PWD/not" \ > > test_expect_code 129 git -C not/repo difftool -h >output && > > grep ^usage: output > > I agree with the intent, but the execution here is "Not quite". > test_expect_code being a shell function, it does not take the > "one-shot environment assignment for this single invocation," like > external commands do. So now that we know what is wrong, can you please enlighten me about what is right? Thanks, Johannes ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] difftool: fix bug when printing usage 2017-02-07 11:21 ` Johannes Schindelin @ 2017-02-07 20:03 ` Junio C Hamano 2017-02-07 20:06 ` Junio C Hamano 0 siblings, 1 reply; 12+ messages in thread From: Junio C Hamano @ 2017-02-07 20:03 UTC (permalink / raw) To: Johannes Schindelin; +Cc: David Aguilar, Git ML, Denton Liu Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: >> > Likewise, this would become >> > >> > GIT_CEILING_DIRECTORIES="$PWD/not" \ >> > test_expect_code 129 git -C not/repo difftool -h >output && >> > grep ^usage: output >> >> I agree with the intent, but the execution here is "Not quite". >> test_expect_code being a shell function, it does not take the >> "one-shot environment assignment for this single invocation," like >> external commands do. > > So now that we know what is wrong, can you please enlighten me about what > is right? David's original is just fine, isn't it? ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] difftool: fix bug when printing usage 2017-02-07 20:03 ` Junio C Hamano @ 2017-02-07 20:06 ` Junio C Hamano 2017-02-08 15:06 ` Johannes Schindelin 0 siblings, 1 reply; 12+ messages in thread From: Junio C Hamano @ 2017-02-07 20:06 UTC (permalink / raw) To: Johannes Schindelin; +Cc: David Aguilar, Git ML, Denton Liu Junio C Hamano <gitster@pobox.com> writes: > Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: > >>> > Likewise, this would become >>> > >>> > GIT_CEILING_DIRECTORIES="$PWD/not" \ >>> > test_expect_code 129 git -C not/repo difftool -h >output && >>> > grep ^usage: output >>> >>> I agree with the intent, but the execution here is "Not quite". >>> test_expect_code being a shell function, it does not take the >>> "one-shot environment assignment for this single invocation," like >>> external commands do. >> >> So now that we know what is wrong, can you please enlighten me about what >> is right? > > David's original is just fine, isn't it? I've also seen people use "env VAR=VAL git command" as the command to be tested in t/ scripts. You can run that under test_expect_code, methinks. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] difftool: fix bug when printing usage 2017-02-07 20:06 ` Junio C Hamano @ 2017-02-08 15:06 ` Johannes Schindelin 0 siblings, 0 replies; 12+ messages in thread From: Johannes Schindelin @ 2017-02-08 15:06 UTC (permalink / raw) To: Junio C Hamano; +Cc: David Aguilar, Git ML, Denton Liu Hi Junio, On Tue, 7 Feb 2017, Junio C Hamano wrote: > Junio C Hamano <gitster@pobox.com> writes: > > > Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: > > > >>> > Likewise, this would become > >>> > > >>> > GIT_CEILING_DIRECTORIES="$PWD/not" \ > >>> > test_expect_code 129 git -C not/repo difftool -h >output && > >>> > grep ^usage: output > >>> > >>> I agree with the intent, but the execution here is "Not quite". > >>> test_expect_code being a shell function, it does not take the > >>> "one-shot environment assignment for this single invocation," like > >>> external commands do. > >> > >> So now that we know what is wrong, can you please enlighten me about > >> what is right? > > > > David's original is just fine, isn't it? > > I've also seen people use "env VAR=VAL git command" as the command to be > tested in t/ scripts. You can run that under test_expect_code, > methinks. That is exactly what David ended up sending out as follow-up patches. I did not mean to be critical, I just found it to be more helpful to accompany "that does not work" comments with "but this does" in the past. Ciao, Johannes ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2017-02-08 15:38 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-02-04 2:56 [PATCH] Remove --no-gui option from difftool usage string Denton Liu 2017-02-04 5:58 ` Jacob Keller 2017-02-04 6:23 ` Denton Liu 2017-02-05 10:22 ` David Aguilar 2017-02-05 20:17 ` [BUG] was: " David Aguilar 2017-02-05 21:23 ` [PATCH] difftool: fix bug when printing usage David Aguilar 2017-02-06 16:58 ` Johannes Schindelin 2017-02-06 18:13 ` Junio C Hamano 2017-02-07 11:21 ` Johannes Schindelin 2017-02-07 20:03 ` Junio C Hamano 2017-02-07 20:06 ` Junio C Hamano 2017-02-08 15:06 ` Johannes Schindelin
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 NNTP newsgroup(s).