Re: [PATCH 3/7] test patch hunk editing with commit -p -m
Benoit Pierre benoit.pie...@gmail.com writes: Add (failing) tests: with commit changing the environment to let hooks know that no editor will be used (by setting GIT_EDITOR to :), the edit hunk functionality does not work (no editor is launched and the whole hunk is committed). Signed-off-by: Benoit Pierre benoit.pie...@gmail.com --- t/t7513-commit-patch.sh | 32 1 file changed, 32 insertions(+) create mode 100755 t/t7513-commit-patch.sh diff --git a/t/t7513-commit-patch.sh b/t/t7513-commit-patch.sh Again, as I said, I'll rename this to t7514-commit.patch.sh while I queue this. Thanks. new file mode 100755 index 000..9311b0c --- /dev/null +++ b/t/t7513-commit-patch.sh @@ -0,0 +1,32 @@ +#!/bin/sh + +test_description='hunk edit with commit -p -m' +. ./test-lib.sh + +if ! test_have_prereq PERL +then + skip_all=skipping '$test_description' tests, perl not available + test_done +fi + +test_expect_success 'setup (initial)' ' + echo line1 file + git add file + git commit -m commit1 +' + +test_expect_failure 'edit hunk commit -p -m message' ' + test_when_finished rm -f editor_was_started + echo more file + echo e | env GIT_EDITOR=touch editor_was_started git commit -p -m commit2 file + test -r editor_was_started +' + +test_expect_failure 'edit hunk commit --dry-run -p -m message' ' + test_when_finished rm -f editor_was_started + echo more file + echo e | env GIT_EDITOR=touch editor_was_started git commit -p -m commit3 file + test -r editor_was_started +' + +test_done -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/7] test patch hunk editing with commit -p -m
Benoit Pierre benoit.pie...@gmail.com writes: +test_expect_failure 'edit hunk commit -p -m message' ' + test_when_finished rm -f editor_was_started Not just when finished, run rm -f here to make sure that the file does not exist. Later other people may add new tests before this test piece and affect the state of your throw-away working tree, and you would want to protect yourself from their changes. + echo more file + echo e | env GIT_EDITOR=touch editor_was_started git commit -p -m commit2 file Avoid touch unless you are interested in the timestamp to be updated. Use : editor_was_started or something like that when you are only interested in it was not there, now it is. The same comment applies to the next one. Thanks. + test -r editor_was_started +' + +test_expect_failure 'edit hunk commit --dry-run -p -m message' ' + test_when_finished rm -f editor_was_started + echo more file + echo e | env GIT_EDITOR=touch editor_was_started git commit -p -m commit3 file + test -r editor_was_started +' + +test_done -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/7] test patch hunk editing with commit -p -m
On Mon, Mar 17, 2014 at 7:49 PM, Junio C Hamano gits...@pobox.com wrote: Benoit Pierre benoit.pie...@gmail.com writes: Add (failing) tests: with commit changing the environment to let hooks know that no editor will be used (by setting GIT_EDITOR to :), the edit hunk functionality does not work (no editor is launched and the whole hunk is committed). Signed-off-by: Benoit Pierre benoit.pie...@gmail.com --- t/t7513-commit-patch.sh | 32 1 file changed, 32 insertions(+) create mode 100755 t/t7513-commit-patch.sh diff --git a/t/t7513-commit-patch.sh b/t/t7513-commit-patch.sh Again, as I said, I'll rename this to t7514-commit.patch.sh while I queue this. I assumed the 14 was a typo, will rename, but to t7514-commit-patch.sh right? (with 2 '-'). -- A: Because it destroys the flow of conversation. Q: Why is top posting dumb? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/7] test patch hunk editing with commit -p -m
On Mon, Mar 17, 2014 at 3:46 PM, Benoit Pierre benoit.pie...@gmail.com wrote: On Mon, Mar 17, 2014 at 7:49 PM, Junio C Hamano gits...@pobox.com wrote: Benoit Pierre benoit.pie...@gmail.com writes: Add (failing) tests: with commit changing the environment to let hooks know that no editor will be used (by setting GIT_EDITOR to :), the edit hunk functionality does not work (no editor is launched and the whole hunk is committed). Signed-off-by: Benoit Pierre benoit.pie...@gmail.com --- t/t7513-commit-patch.sh | 32 1 file changed, 32 insertions(+) create mode 100755 t/t7513-commit-patch.sh diff --git a/t/t7513-commit-patch.sh b/t/t7513-commit-patch.sh Again, as I said, I'll rename this to t7514-commit.patch.sh while I queue this. I assumed the 14 was a typo, will rename, but to t7514-commit-patch.sh right? (with 2 '-'). Yes, two '-'. In the 'pu' branch, there is a new t7513-interpret-trailers.sh. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/7] test patch hunk editing with commit -p -m
On Mon, Mar 17, 2014 at 7:53 PM, Junio C Hamano gits...@pobox.com wrote: Benoit Pierre benoit.pie...@gmail.com writes: +test_expect_failure 'edit hunk commit -p -m message' ' + test_when_finished rm -f editor_was_started Not just when finished, run rm -f here to make sure that the file does not exist. Later other people may add new tests before this test piece and affect the state of your throw-away working tree, and you would want to protect yourself from their changes. + echo more file + echo e | env GIT_EDITOR=touch editor_was_started git commit -p -m commit2 file Avoid touch unless you are interested in the timestamp to be updated. Use : editor_was_started or something like that when you are only interested in it was not there, now it is. The same comment applies to the next one. Isn't the point of using when finished to have each test leave the tree clean after execution, to avoid bleeding onto the next test(s)? Instead of having each test clean after the previous test(s). It seems to me using both is kind of redundant, no? -- A: Because it destroys the flow of conversation. Q: Why is top posting dumb? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/7] test patch hunk editing with commit -p -m
On Mon, Mar 17, 2014 at 8:51 PM, Eric Sunshine sunsh...@sunshineco.com wrote: On Mon, Mar 17, 2014 at 3:46 PM, Benoit Pierre benoit.pie...@gmail.com wrote: On Mon, Mar 17, 2014 at 7:49 PM, Junio C Hamano gits...@pobox.com wrote: Benoit Pierre benoit.pie...@gmail.com writes: [...] diff --git a/t/t7513-commit-patch.sh b/t/t7513-commit-patch.sh Again, as I said, I'll rename this to t7514-commit.patch.sh while I queue this. I assumed the 14 was a typo, will rename, but to t7514-commit-patch.sh right? (with 2 '-'). Yes, two '-'. In the 'pu' branch, there is a new t7513-interpret-trailers.sh. OK. -- A: Because it destroys the flow of conversation. Q: Why is top posting dumb? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/7] test patch hunk editing with commit -p -m
Benoit Pierre benoit.pie...@gmail.com writes: On Mon, Mar 17, 2014 at 7:49 PM, Junio C Hamano gits...@pobox.com wrote: Benoit Pierre benoit.pie...@gmail.com writes: Add (failing) tests: with commit changing the environment to let hooks know that no editor will be used (by setting GIT_EDITOR to :), the edit hunk functionality does not work (no editor is launched and the whole hunk is committed). Signed-off-by: Benoit Pierre benoit.pie...@gmail.com --- t/t7513-commit-patch.sh | 32 1 file changed, 32 insertions(+) create mode 100755 t/t7513-commit-patch.sh diff --git a/t/t7513-commit-patch.sh b/t/t7513-commit-patch.sh Again, as I said, I'll rename this to t7514-commit.patch.sh while I queue this. I assumed the 14 was a typo, will rename, but to t7514-commit-patch.sh right? (with 2 '-'). Thanks, yes. That is how I queued the previous one on 'pu', I think. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/7] test patch hunk editing with commit -p -m
Benoit Pierre benoit.pie...@gmail.com writes: Isn't the point of using when finished to have each test leave the tree clean after execution, to avoid bleeding onto the next test(s)? But you cannot anticipate what other people will do in the future before you have a chance to run this piece. Your using when-finished is courtesy for others. Your explicitly making sure what you do not want to be stray behind is protecting yourself from others. They serve different purposes, and you need both. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/7] test patch hunk editing with commit -p -m
On 2014-03-11 22.03, Junio C Hamano wrote: Benoit Pierre benoit.pie...@gmail.com writes: Add (failing) test: with commit changing the environment to let hooks now that no editor will be used (by setting GIT_EDITOR to :), the edit hunk functionality does not work (no editor is launched and the whole hunk is committed). Signed-off-by: Benoit Pierre benoit.pie...@gmail.com --- t/t7513-commit_-p_-m_hunk_edit.sh | 34 ++ 1 file changed, 34 insertions(+) create mode 100755 t/t7513-commit_-p_-m_hunk_edit.sh diff --git a/t/t7513-commit_-p_-m_hunk_edit.sh b/t/t7513-commit_-p_-m_hunk_edit.sh I'll move this to t/t7514-commit-patch.sh for now while queuing. This line is problematic: echo e | env GIT_EDITOR=sed s/+line3\$/+line2/ -i git commit -p -m commit2 f (sed -i is not portable: http://pubs.opengroup.org/onlinepubs/007908799/xcu/sed.html) The whole test hangs in a forever loop loop under MacOS: debug=t verbose=t ./t7514-commit-patch.sh Stage this hunk [y,n,q,a,d,/,e,?]? @@ -1 +1,2 @@ line1 +line3 I think perl can be used instead of sed (but I haven't found the exact syntax yet) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/7] test patch hunk editing with commit -p -m
On Sat, Mar 15, 2014 at 1:28 PM, Torsten Bögershausen tbo...@web.de wrote: On 2014-03-11 22.03, Junio C Hamano wrote: Benoit Pierre benoit.pie...@gmail.com writes: Add (failing) test: with commit changing the environment to let hooks now that no editor will be used (by setting GIT_EDITOR to :), the edit hunk functionality does not work (no editor is launched and the whole hunk is committed). Signed-off-by: Benoit Pierre benoit.pie...@gmail.com --- t/t7513-commit_-p_-m_hunk_edit.sh | 34 ++ 1 file changed, 34 insertions(+) create mode 100755 t/t7513-commit_-p_-m_hunk_edit.sh diff --git a/t/t7513-commit_-p_-m_hunk_edit.sh b/t/t7513-commit_-p_-m_hunk_edit.sh I'll move this to t/t7514-commit-patch.sh for now while queuing. This line is problematic: echo e | env GIT_EDITOR=sed s/+line3\$/+line2/ -i git commit -p -m commit2 f (sed -i is not portable: http://pubs.opengroup.org/onlinepubs/007908799/xcu/sed.html) The whole test hangs in a forever loop loop under MacOS: debug=t verbose=t ./t7514-commit-patch.sh Stage this hunk [y,n,q,a,d,/,e,?]? @@ -1 +1,2 @@ line1 +line3 I think perl can be used instead of sed (but I haven't found the exact syntax yet) Or maybe change the test to just 'touch' a temporary file or change its content like Jun Hao did with for its version of the tests: https://github.com/bloomberg/git/compare/commit-patch-allow-hunk-editing Should I make a third version? I'll simplify and move the tests to t/t7514-commit-patch.sh and add a test for the '--dry-run' case. And also: - 'now' = 'know' in one of the commit message - sign off the last patch (which I forgot to do) - fix the indentation in one of the patch -- A: Because it destroys the flow of conversation. Q: Why is top posting dumb? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/7] test patch hunk editing with commit -p -m
On 2014-03-15 17.11, Benoit Pierre wrote: On Sat, Mar 15, 2014 at 1:28 PM, Torsten Bögershausen tbo...@web.de wrote: On 2014-03-11 22.03, Junio C Hamano wrote: Benoit Pierre benoit.pie...@gmail.com writes: Add (failing) test: with commit changing the environment to let hooks now that no editor will be used (by setting GIT_EDITOR to :), the edit hunk functionality does not work (no editor is launched and the whole hunk is committed). Signed-off-by: Benoit Pierre benoit.pie...@gmail.com --- t/t7513-commit_-p_-m_hunk_edit.sh | 34 ++ 1 file changed, 34 insertions(+) create mode 100755 t/t7513-commit_-p_-m_hunk_edit.sh diff --git a/t/t7513-commit_-p_-m_hunk_edit.sh b/t/t7513-commit_-p_-m_hunk_edit.sh I'll move this to t/t7514-commit-patch.sh for now while queuing. This line is problematic: echo e | env GIT_EDITOR=sed s/+line3\$/+line2/ -i git commit -p -m commit2 f (sed -i is not portable: http://pubs.opengroup.org/onlinepubs/007908799/xcu/sed.html) The whole test hangs in a forever loop loop under MacOS: debug=t verbose=t ./t7514-commit-patch.sh Stage this hunk [y,n,q,a,d,/,e,?]? @@ -1 +1,2 @@ line1 +line3 I think perl can be used instead of sed (but I haven't found the exact syntax yet) Or maybe change the test to just 'touch' a temporary file or change its content like Jun Hao did with for its version of the tests: https://github.com/bloomberg/git/compare/commit-patch-allow-hunk-editing Should I make a third version? I'll simplify and move the tests to t/t7514-commit-patch.sh and add a test for the '--dry-run' case. And also: - 'now' = 'know' in one of the commit message - sign off the last patch (which I forgot to do) - fix the indentation in one of the patch The following works for me, (diff against pu) (and if you want to send a 3rd version, please do so :-) diff --git a/t/t7514-commit-patch.sh b/t/t7514-commit-patch.sh index 1f05d32..da92669 100755 --- a/t/t7514-commit-patch.sh +++ b/t/t7514-commit-patch.sh @@ -25,10 +25,20 @@ test_expect_success 'setup (initial)' ' EOF ' +write_script .git/FAKE_EDITOR \EOF + sed -e s/+line3\$/+line2/ $1 tmp + mv -f tmp $1 + exit 0 +EOF + test_expect_success 'edit hunk commit -p -m message' ' - echo e | env GIT_EDITOR=sed s/+line3\$/+line2/ -i git commit -p -m commit2 file - git diff HEAD^ HEAD actual - test_cmp expect actual + ( + GIT_EDITOR=\$(pwd)/.git/FAKE_EDITOR\ + export GIT_EDITOR + echo e | git commit -p -m commit2 file + git diff HEAD^ HEAD actual + test_cmp expect actual + ) ' test_done -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/7] test patch hunk editing with commit -p -m
Eric Sunshine sunsh...@sunshineco.com writes: On Mon, Mar 10, 2014 at 2:49 PM, Benoit Pierre benoit.pie...@gmail.com wrote: Add (failing) test: with commit changing the environment to let hooks now that no editor will be used (by setting GIT_EDITOR to :), the edit hunk functionality does not work (no editor is launched and the whole hunk is committed). Signed-off-by: Benoit Pierre benoit.pie...@gmail.com --- t/t7513-commit_-p_-m_hunk_edit.sh | 34 ++ 1 file changed, 34 insertions(+) create mode 100755 t/t7513-commit_-p_-m_hunk_edit.sh Is it possible to give this file a name less unusual and more consistent with other test scripts? Perhaps choose a more generic name which may allow other similar tests to be added to the file in the future (if needed)? Surely. There are reset-patch and checkout-patch tests, and if we were to add something like this, I'd imagine commit-patch would be a logical name for the new test. diff --git a/t/t7513-commit_-p_-m_hunk_edit.sh b/t/t7513-commit_-p_-m_hunk_edit.sh new file mode 100755 index 000..994939a --- /dev/null +++ b/t/t7513-commit_-p_-m_hunk_edit.sh @@ -0,0 +1,34 @@ +#!/bin/sh + +test_description='hunk edit with commit -p -m' +. ./test-lib.sh + +if ! test_have_prereq PERL +then + skip_all=skipping '$test_description' tests, perl not available + test_done +fi + +test_expect_success 'setup (initial)' ' + echo line1 file + git add file + git commit -m commit1 + echo line3 file + cat expect -\EOF + diff --git a/file b/file + index a29bdeb..c0d0fb4 100644 + --- a/file + +++ b/file + @@ -1 +1,2 @@ +line1 + +line2 + EOF In the previous review, the suggestion was that creation of 'expect' should be moved to the test below where it is actually used rather than into the 'setup' phase above. +' + +test_expect_failure 'edit hunk commit -p -m message' ' + echo e | env GIT_EDITOR=sed s/+line3\$/+line2/ -i git commit -p -m commit2 file + git diff HEAD^ HEAD actual + test_cmp expect actual +' + +test_done -- 1.9.0 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/7] test patch hunk editing with commit -p -m
Benoit Pierre benoit.pie...@gmail.com writes: Add (failing) test: with commit changing the environment to let hooks now that no editor will be used (by setting GIT_EDITOR to :), the edit hunk functionality does not work (no editor is launched and the whole hunk is committed). Signed-off-by: Benoit Pierre benoit.pie...@gmail.com --- t/t7513-commit_-p_-m_hunk_edit.sh | 34 ++ 1 file changed, 34 insertions(+) create mode 100755 t/t7513-commit_-p_-m_hunk_edit.sh diff --git a/t/t7513-commit_-p_-m_hunk_edit.sh b/t/t7513-commit_-p_-m_hunk_edit.sh I'll move this to t/t7514-commit-patch.sh for now while queuing. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/7] test patch hunk editing with commit -p -m
On Mon, Mar 10, 2014 at 2:49 PM, Benoit Pierre benoit.pie...@gmail.com wrote: Add (failing) test: with commit changing the environment to let hooks now that no editor will be used (by setting GIT_EDITOR to :), the edit hunk functionality does not work (no editor is launched and the whole hunk is committed). Signed-off-by: Benoit Pierre benoit.pie...@gmail.com --- t/t7513-commit_-p_-m_hunk_edit.sh | 34 ++ 1 file changed, 34 insertions(+) create mode 100755 t/t7513-commit_-p_-m_hunk_edit.sh Is it possible to give this file a name less unusual and more consistent with other test scripts? Perhaps choose a more generic name which may allow other similar tests to be added to the file in the future (if needed)? diff --git a/t/t7513-commit_-p_-m_hunk_edit.sh b/t/t7513-commit_-p_-m_hunk_edit.sh new file mode 100755 index 000..994939a --- /dev/null +++ b/t/t7513-commit_-p_-m_hunk_edit.sh @@ -0,0 +1,34 @@ +#!/bin/sh + +test_description='hunk edit with commit -p -m' +. ./test-lib.sh + +if ! test_have_prereq PERL +then + skip_all=skipping '$test_description' tests, perl not available + test_done +fi + +test_expect_success 'setup (initial)' ' + echo line1 file + git add file + git commit -m commit1 + echo line3 file + cat expect -\EOF + diff --git a/file b/file + index a29bdeb..c0d0fb4 100644 + --- a/file + +++ b/file + @@ -1 +1,2 @@ +line1 + +line2 + EOF In the previous review, the suggestion was that creation of 'expect' should be moved to the test below where it is actually used rather than into the 'setup' phase above. +' + +test_expect_failure 'edit hunk commit -p -m message' ' + echo e | env GIT_EDITOR=sed s/+line3\$/+line2/ -i git commit -p -m commit2 file + git diff HEAD^ HEAD actual + test_cmp expect actual +' + +test_done -- 1.9.0 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/7] test patch hunk editing with commit -p -m
mincro nit. From: Benoit Pierre benoit.pie...@gmail.com Add (failing) test: with commit changing the environment to let hooks now that no editor will be used (by setting GIT_EDITOR to :), the s/now/know/ edit hunk functionality does not work (no editor is launched and the whole hunk is committed). Signed-off-by: Benoit Pierre benoit.pie...@gmail.com --- t/t7513-commit_-p_-m_hunk_edit.sh | 34 ++ 1 file changed, 34 insertions(+) create mode 100755 t/t7513-commit_-p_-m_hunk_edit.sh diff --git a/t/t7513-commit_-p_-m_hunk_edit.sh b/t/t7513-commit_-p_-m_hunk_edit.sh new file mode 100755 index 000..994939a --- /dev/null +++ b/t/t7513-commit_-p_-m_hunk_edit.sh @@ -0,0 +1,34 @@ +#!/bin/sh + +test_description='hunk edit with commit -p -m' +. ./test-lib.sh + +if ! test_have_prereq PERL +then + skip_all=skipping '$test_description' tests, perl not available + test_done +fi + +test_expect_success 'setup (initial)' ' + echo line1 file + git add file + git commit -m commit1 + echo line3 file + cat expect -\EOF + diff --git a/file b/file + index a29bdeb..c0d0fb4 100644 + --- a/file + +++ b/file + @@ -1 +1,2 @@ + line1 + +line2 + EOF +' + +test_expect_failure 'edit hunk commit -p -m message' ' + echo e | env GIT_EDITOR=sed s/+line3\$/+line2/ -i git commit -p -m commit2 file + git diff HEAD^ HEAD actual + test_cmp expect actual +' + +test_done -- 1.9.0 -- -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html