Ramsay Jones [off-list ref] writes:
Signed-off-by: Ramsay Jones <redacted>
---
Hi Jens,
commit 61b6a633 ("commit -v: strip diffs and submodule shortlogs
from the commit message", 19-11-2013) in 'pu' fails the new test
it added to t7507.
I didn't spend too long looking at the problem, so take this patch
as nothing more than a quick suggestion for a possible solution! :-P
[The err file contained something like: "There was a problem with the
editor '"$FAKE_EDITOR"'"].
Having said that, this fixes it for me ...
Well spotted. test_must_fail being a shell function, not a command,
we shouldn't have used the "VAR=val cmd" one-shot environment
assignment for portability.
Even though this happens to be the last test in the script, using
test_set_editor to permanently affect the choice of editor for tests
that follow is not generally a good idea. It would be safer to do
this, I would have to say:
git commit -a -m "submodule commit"
) &&
(
GIT_EDITOR=cat &&
export GIT_EDITOR &&
test_must_fail git commit -a -v 2>err
) &&
test_i18ngrep "Aborting ..."
Thanks.
quoted hunk
ATB,
Ramsay Jones
t/t7507-commit-verbose.sh | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/t/t7507-commit-verbose.sh b/t/t7507-commit-verbose.sh
index 09c1150..49cfb3c 100755
--- a/t/t7507-commit-verbose.sh
+++ b/t/t7507-commit-verbose.sh
@@ -79,7 +79,8 @@ test_expect_success 'submodule log is stripped out too with -v' '
echo "more" >>file &&
git commit -a -m "submodule commit"
) &&
- GIT_EDITOR=cat test_must_fail git commit -a -v 2>err &&
+ test_set_editor cat &&
+ test_must_fail git commit -a -v 2>err &&
test_i18ngrep "Aborting commit due to empty commit message." err
'
On Wed, Nov 20, 2013 at 09:22:31AM -0800, Junio C Hamano wrote:
quoted
commit 61b6a633 ("commit -v: strip diffs and submodule shortlogs
from the commit message", 19-11-2013) in 'pu' fails the new test
it added to t7507.
I didn't spend too long looking at the problem, so take this patch
as nothing more than a quick suggestion for a possible solution! :-P
[The err file contained something like: "There was a problem with the
editor '"$FAKE_EDITOR"'"].
Having said that, this fixes it for me ...
Well spotted. test_must_fail being a shell function, not a command,
we shouldn't have used the "VAR=val cmd" one-shot environment
assignment for portability.
Yeah, I noticed that, too upon reading Ramsay's patch. But I thought the
usual symptom there was that the variable is not properly unset after
the function returns? In other words, it might affect later tests, but
the failure that Ramsay sees is in _this_ test, so it must be a separate
issue.
The test_set_editor helper does some magic to help with quoting, but
that should not be an issue in this case (since we are using "cat"). We
are using test_set_editor elsewhere in the script, which would have set
EDITOR previously. But I would think that GIT_EDITOR, which we are using
here, would supersede that. However, the error message he shows
indicates that git is using EDITOR (as FAKE_EDITOR is part of that quote
magic).
Am I misremembering the issues with one-shot variables and functions?
Puzzled...
-Peff
On 20/11/13 17:22, Junio C Hamano wrote:
Ramsay Jones [off-list ref] writes:
quoted
Signed-off-by: Ramsay Jones <redacted>
---
Hi Jens,
commit 61b6a633 ("commit -v: strip diffs and submodule shortlogs
from the commit message", 19-11-2013) in 'pu' fails the new test
it added to t7507.
I didn't spend too long looking at the problem, so take this patch
as nothing more than a quick suggestion for a possible solution! :-P
[The err file contained something like: "There was a problem with the
editor '"$FAKE_EDITOR"'"].
Having said that, this fixes it for me ...
Well spotted. test_must_fail being a shell function, not a command,
we shouldn't have used the "VAR=val cmd" one-shot environment
assignment for portability.
Even though this happens to be the last test in the script, using
test_set_editor to permanently affect the choice of editor for tests
that follow is not generally a good idea. It would be safer to do
this, I would have to say:
git commit -a -m "submodule commit"
) &&
(
GIT_EDITOR=cat &&
export GIT_EDITOR &&
test_must_fail git commit -a -v 2>err
) &&
test_i18ngrep "Aborting ..."
Yes, this works great (and I very nearly wrote exactly this, but since
the test was using test_set_editor anyway ...) :-D
Thanks.
ATB,
Ramsay Jones
Am 20.11.2013 20:35, schrieb Ramsay Jones:
On 20/11/13 17:22, Junio C Hamano wrote:
quoted
Ramsay Jones [off-list ref] writes:
quoted
Signed-off-by: Ramsay Jones <redacted>
---
Hi Jens,
commit 61b6a633 ("commit -v: strip diffs and submodule shortlogs
from the commit message", 19-11-2013) in 'pu' fails the new test
it added to t7507.
I didn't spend too long looking at the problem, so take this patch
as nothing more than a quick suggestion for a possible solution! :-P
[The err file contained something like: "There was a problem with the
editor '"$FAKE_EDITOR"'"].
Having said that, this fixes it for me ...
Well spotted. test_must_fail being a shell function, not a command,
we shouldn't have used the "VAR=val cmd" one-shot environment
assignment for portability.
Even though this happens to be the last test in the script, using
test_set_editor to permanently affect the choice of editor for tests
that follow is not generally a good idea. It would be safer to do
this, I would have to say:
git commit -a -m "submodule commit"
) &&
(
GIT_EDITOR=cat &&
export GIT_EDITOR &&
test_must_fail git commit -a -v 2>err
) &&
test_i18ngrep "Aborting ..."
Yes, this works great (and I very nearly wrote exactly this, but since
the test was using test_set_editor anyway ...) :-D
Thanks all, will use that in the next iteration.