{"thread":{"id":"34742","subject":"[PATCH 0/3] t3404 incremental improvements","startedAt":"2013-08-21T19:12:55Z","lastAt":"2013-08-26T05:29:40Z","messageCount":12,"participants":["Eric Sunshine","Junio C Hamano","Jonathan Nieder"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"225629","messageId":"1377112378-45511-1-git-send-email-sunshine@sunshineco.com","threadId":"34742","inReplyTo":null,"subject":"[PATCH 0/3] t3404 incremental improvements","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2013-08-21T19:12:55Z","receivedAt":"2013-08-21T19:12:55Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"This set of patches was meant to be a re-roll of [1] addressing Junio's\ncomments, however [1] graduated to 'next' before I found time to work on\nit further, so these are instead incremental patches atop 'next'.\n\npatch 1: address Junio's comment [2]\n\npatch 2: address Junio's comment [3] with a sledge-hammer approach;\nwhether to accept this patch is a judgment call\n\npatch 3: trivial cleanup which should make the test easier to comprehend\nfor future readers\n\n[1]: http://thread.gmane.org/gmane.comp.version-control.git/232146\n[2]: http://thread.gmane.org/gmane.comp.version-control.git/232146/focus=232159\n[3]: http://thread.gmane.org/gmane.comp.version-control.git/232146/focus=232158\n\nEric Sunshine (3):\n  t3404: preserve test_tick state across short SHA-1 collision test\n  t3404: make tests more self-contained\n  t3404: simplify short SHA-1 collision test\n\n t/t3404-rebase-interactive.sh | 77 ++++++++++++++++++++++++++++++++++++++++---\n 1 file changed, 72 insertions(+), 5 deletions(-)\n\n-- \n1.8.4.rc4.499.gfb33910\n"},{"id":"225632","messageId":"1377112378-45511-2-git-send-email-sunshine@sunshineco.com","threadId":"34742","inReplyTo":"1377112378-45511-1-git-send-email-sunshine@sunshineco.com","subject":"[PATCH 1/3] t3404: preserve test_tick state across short SHA-1 collision test","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2013-08-21T19:12:56Z","receivedAt":"2013-08-21T19:12:56Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"The short SHA-1 collision test requires carefully crafted commits in\norder to ensure a collision at rebase time. This involves managing state\nwhich impacts the resulting SHA-1, including commit time. To accomplish\nthis, test_tick is set to a known state for the short SHA-1 collision\ntest.\n\nUnfortunately, doing so throws away the existing state of test_tick,\nwhich may be problematic for subsequently added tests. Fix this by\npreserving the existing state of test_tick across the short SHA-1\ncollision test.\n\nSigned-off-by: Eric Sunshine <sunshine@sunshineco.com>\n---\n t/t3404-rebase-interactive.sh | 6 ++++++\n 1 file changed, 6 insertions(+)\n\ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex b65b774..6be97ba 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -994,17 +994,23 @@ test_expect_success 'short SHA-1 setup' '\n \ttest_when_finished \"git checkout master\" &&\n \tgit checkout --orphan collide &&\n \tgit rm -rf . &&\n+\t(\n \tunset test_tick &&\n \ttest_commit collide1 collide &&\n \ttest_commit --notick collide2 collide &&\n \ttest_commit --notick \"collide3 115158b5\" collide collide3 collide3\n+\t)\n '\n \n test_expect_success 'short SHA-1 collide' '\n \ttest_when_finished \"reset_rebase && git checkout master\" &&\n \tgit checkout collide &&\n+\t(\n+\tunset test_tick &&\n+\ttest_tick &&\n \tFAKE_COMMIT_MESSAGE=\"collide2 815200e\" \\\n \tFAKE_LINES=\"reword 1 2\" git rebase -i HEAD~2\n+\t)\n '\n \n test_done\n-- \n1.8.4.rc4.499.gfb33910\n"},{"id":"225630","messageId":"1377112378-45511-3-git-send-email-sunshine@sunshineco.com","threadId":"34742","inReplyTo":"1377112378-45511-1-git-send-email-sunshine@sunshineco.com","subject":"[PATCH 2/3] t3404: make tests more self-contained","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2013-08-21T19:12:57Z","receivedAt":"2013-08-21T19:12:57Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"As its very first action, t3404 installs (via set_fake_editor) a\nspecialized $EDITOR which simplifies automated 'rebase -i' testing. Many\ntests rely upon this setting, thus tests which need a different editor\nmust take extra care upon completion to restore $EDITOR in order to\navoid breaking following tests. This places extra burden upon such tests\nand requires that they undesirably have extra knowledge about\nsurrounding tests. Ease this burden by having each test install the\n$EDITOR it requires, rather than relying upon a global setting.\n\nSigned-off-by: Eric Sunshine <sunshine@sunshineco.com>\n---\n t/t3404-rebase-interactive.sh | 67 +++++++++++++++++++++++++++++++++++++++++--\n 1 file changed, 64 insertions(+), 3 deletions(-)\n\ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex 6be97ba..7d15c7a 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -29,8 +29,6 @@ Initial setup:\n \n . \"$TEST_DIRECTORY\"/lib-rebase.sh\n \n-set_fake_editor\n-\n # WARNING: Modifications to the initial repository can change the SHA ID used\n # in the expect2 file for the 'stop on conflicting pick' test.\n \n@@ -72,6 +70,7 @@ export SHELL\n test_expect_success 'rebase -i with the exec command' '\n \tgit checkout master &&\n \t(\n+\tset_fake_editor &&\n \tFAKE_LINES=\"1 exec_>touch-one\n \t\t2 exec_>touch-two exec_false exec_>touch-three\n \t\t3 4 exec_>\\\"touch-file__name_with_spaces\\\";_>touch-after-semicolon 5\" &&\n@@ -93,6 +92,7 @@ test_expect_success 'rebase -i with the exec command' '\n test_expect_success 'rebase -i with the exec command runs from tree root' '\n \tgit checkout master &&\n \tmkdir subdir && (cd subdir &&\n+\tset_fake_editor &&\n \tFAKE_LINES=\"1 exec_>touch-subdir\" \\\n \t\tgit rebase -i HEAD^\n \t) &&\n@@ -103,6 +103,7 @@ test_expect_success 'rebase -i with the exec command runs from tree root' '\n test_expect_success 'rebase -i with the exec command checks tree cleanness' '\n \tgit checkout master &&\n \t(\n+\tset_fake_editor &&\n \tFAKE_LINES=\"exec_echo_foo_>file1 1\" &&\n \texport FAKE_LINES &&\n \ttest_must_fail git rebase -i HEAD^\n@@ -116,6 +117,7 @@ test_expect_success 'rebase -i with exec of inexistent command' '\n \tgit checkout master &&\n \ttest_when_finished \"git rebase --abort\" &&\n \t(\n+\tset_fake_editor &&\n \tFAKE_LINES=\"exec_this-command-does-not-exist 1\" &&\n \texport FAKE_LINES &&\n \ttest_must_fail git rebase -i HEAD^ >actual 2>&1\n@@ -125,6 +127,7 @@ test_expect_success 'rebase -i with exec of inexistent command' '\n \n test_expect_success 'no changes are a nop' '\n \tgit checkout branch2 &&\n+\tset_fake_editor &&\n \tgit rebase -i F &&\n \ttest \"$(git symbolic-ref -q HEAD)\" = \"refs/heads/branch2\" &&\n \ttest $(git rev-parse I) = $(git rev-parse HEAD)\n@@ -134,6 +137,7 @@ test_expect_success 'test the [branch] option' '\n \tgit checkout -b dead-end &&\n \tgit rm file6 &&\n \tgit commit -m \"stop here\" &&\n+\tset_fake_editor &&\n \tgit rebase -i F branch2 &&\n \ttest \"$(git symbolic-ref -q HEAD)\" = \"refs/heads/branch2\" &&\n \ttest $(git rev-parse I) = $(git rev-parse branch2) &&\n@@ -142,6 +146,7 @@ test_expect_success 'test the [branch] option' '\n \n test_expect_success 'test --onto <branch>' '\n \tgit checkout -b test-onto branch2 &&\n+\tset_fake_editor &&\n \tgit rebase -i --onto branch1 F &&\n \ttest \"$(git symbolic-ref -q HEAD)\" = \"refs/heads/test-onto\" &&\n \ttest $(git rev-parse HEAD^) = $(git rev-parse branch1) &&\n@@ -151,6 +156,7 @@ test_expect_success 'test --onto <branch>' '\n test_expect_success 'rebase on top of a non-conflicting commit' '\n \tgit checkout branch1 &&\n \tgit tag original-branch1 &&\n+\tset_fake_editor &&\n \tgit rebase -i branch2 &&\n \ttest file6 = $(git diff --name-only original-branch1) &&\n \ttest \"$(git symbolic-ref -q HEAD)\" = \"refs/heads/branch1\" &&\n@@ -163,6 +169,7 @@ test_expect_success 'reflog for the branch shows state before rebase' '\n '\n \n test_expect_success 'exchange two commits' '\n+\tset_fake_editor &&\n \tFAKE_LINES=\"2 1\" git rebase -i HEAD~2 &&\n \ttest H = $(git cat-file commit HEAD^ | sed -ne \\$p) &&\n \ttest G = $(git cat-file commit HEAD | sed -ne \\$p)\n@@ -188,6 +195,7 @@ EOF\n \n test_expect_success 'stop on conflicting pick' '\n \tgit tag new-branch1 &&\n+\tset_fake_editor &&\n \ttest_must_fail git rebase -i master &&\n \ttest \"$(git rev-parse HEAD~3)\" = \"$(git rev-parse master)\" &&\n \ttest_cmp expect .git/rebase-merge/patch &&\n@@ -208,6 +216,7 @@ test_expect_success 'abort' '\n test_expect_success 'abort with error when new base cannot be checked out' '\n \tgit rm --cached file1 &&\n \tgit commit -m \"remove file in base\" &&\n+\tset_fake_editor &&\n \ttest_must_fail git rebase -i master > output 2>&1 &&\n \tgrep \"The following untracked working tree files would be overwritten by checkout:\" \\\n \t\toutput &&\n@@ -222,6 +231,7 @@ test_expect_success 'retain authorship' '\n \ttest_tick &&\n \tGIT_AUTHOR_NAME=\"Twerp Snog\" git commit -m \"different author\" &&\n \tgit tag twerp &&\n+\tset_fake_editor &&\n \tgit rebase -i --onto master HEAD^ &&\n \tgit show HEAD | grep \"^Author: Twerp Snog\"\n '\n@@ -232,6 +242,7 @@ test_expect_success 'squash' '\n \ttest_tick &&\n \tGIT_AUTHOR_NAME=\"Nitfol\" git commit -m \"nitfol\" file7 &&\n \techo \"******************************\" &&\n+\tset_fake_editor &&\n \tFAKE_LINES=\"1 squash 2\" EXPECT_HEADER_COUNT=2 \\\n \t\tgit rebase -i --onto master HEAD~2 &&\n \ttest B = $(cat file7) &&\n@@ -244,6 +255,7 @@ test_expect_success 'retain authorship when squashing' '\n \n test_expect_success '-p handles \"no changes\" gracefully' '\n \tHEAD=$(git rev-parse HEAD) &&\n+\tset_fake_editor &&\n \tgit rebase -i -p HEAD^ &&\n \tgit update-index --refresh &&\n \tgit diff-files --quiet &&\n@@ -253,6 +265,7 @@ test_expect_success '-p handles \"no changes\" gracefully' '\n \n test_expect_failure 'exchange two commits with -p' '\n \tgit checkout H &&\n+\tset_fake_editor &&\n \tFAKE_LINES=\"2 1\" git rebase -i -p HEAD~2 &&\n \ttest H = $(git cat-file commit HEAD^ | sed -ne \\$p) &&\n \ttest G = $(git cat-file commit HEAD | sed -ne \\$p)\n@@ -287,6 +300,7 @@ test_expect_success 'preserve merges with -p' '\n \tgit commit -m M file1 &&\n \tgit checkout -b to-be-rebased &&\n \ttest_tick &&\n+\tset_fake_editor &&\n \tgit rebase -i -p --onto branch1 master &&\n \tgit update-index --refresh &&\n \tgit diff-files --quiet &&\n@@ -301,6 +315,7 @@ test_expect_success 'preserve merges with -p' '\n '\n \n test_expect_success 'edit ancestor with -p' '\n+\tset_fake_editor &&\n \tFAKE_LINES=\"1 2 edit 3 4\" git rebase -i -p HEAD~3 &&\n \techo 2 > unrelated-file &&\n \ttest_tick &&\n@@ -314,6 +329,7 @@ test_expect_success 'edit ancestor with -p' '\n \n test_expect_success '--continue tries to commit' '\n \ttest_tick &&\n+\tset_fake_editor &&\n \ttest_must_fail git rebase -i --onto new-branch1 HEAD^ &&\n \techo resolved > file1 &&\n \tgit add file1 &&\n@@ -325,6 +341,7 @@ test_expect_success '--continue tries to commit' '\n test_expect_success 'verbose flag is heeded, even after --continue' '\n \tgit reset --hard master@{1} &&\n \ttest_tick &&\n+\tset_fake_editor &&\n \ttest_must_fail git rebase -v -i --onto new-branch1 HEAD^ &&\n \techo resolved > file1 &&\n \tgit add file1 &&\n@@ -334,6 +351,7 @@ test_expect_success 'verbose flag is heeded, even after --continue' '\n \n test_expect_success 'multi-squash only fires up editor once' '\n \tbase=$(git rev-parse HEAD~4) &&\n+\tset_fake_editor &&\n \tFAKE_COMMIT_AMEND=\"ONCE\" FAKE_LINES=\"1 squash 2 squash 3 squash 4\" \\\n \t\tEXPECT_HEADER_COUNT=4 \\\n \t\tgit rebase -i $base &&\n@@ -344,6 +362,7 @@ test_expect_success 'multi-squash only fires up editor once' '\n test_expect_success 'multi-fixup does not fire up editor' '\n \tgit checkout -b multi-fixup E &&\n \tbase=$(git rev-parse HEAD~4) &&\n+\tset_fake_editor &&\n \tFAKE_COMMIT_AMEND=\"NEVER\" FAKE_LINES=\"1 fixup 2 fixup 3 fixup 4\" \\\n \t\tgit rebase -i $base &&\n \ttest $base = $(git rev-parse HEAD^) &&\n@@ -355,6 +374,7 @@ test_expect_success 'multi-fixup does not fire up editor' '\n test_expect_success 'commit message used after conflict' '\n \tgit checkout -b conflict-fixup conflict-branch &&\n \tbase=$(git rev-parse HEAD~4) &&\n+\tset_fake_editor &&\n \t(\n \t\tFAKE_LINES=\"1 fixup 3 fixup 4\" &&\n \t\texport FAKE_LINES &&\n@@ -373,6 +393,7 @@ test_expect_success 'commit message used after conflict' '\n test_expect_success 'commit message retained after conflict' '\n \tgit checkout -b conflict-squash conflict-branch &&\n \tbase=$(git rev-parse HEAD~4) &&\n+\tset_fake_editor &&\n \t(\n \t\tFAKE_LINES=\"1 fixup 3 squash 4\" &&\n \t\texport FAKE_LINES &&\n@@ -399,6 +420,7 @@ EOF\n test_expect_success 'squash and fixup generate correct log messages' '\n \tgit checkout -b squash-fixup E &&\n \tbase=$(git rev-parse HEAD~4) &&\n+\tset_fake_editor &&\n \tFAKE_COMMIT_AMEND=\"ONCE\" FAKE_LINES=\"1 fixup 2 squash 3 fixup 4\" \\\n \t\tEXPECT_HEADER_COUNT=4 \\\n \t\tgit rebase -i $base &&\n@@ -411,6 +433,7 @@ test_expect_success 'squash and fixup generate correct log messages' '\n test_expect_success 'squash ignores comments' '\n \tgit checkout -b skip-comments E &&\n \tbase=$(git rev-parse HEAD~4) &&\n+\tset_fake_editor &&\n \tFAKE_COMMIT_AMEND=\"ONCE\" FAKE_LINES=\"# 1 # squash 2 # squash 3 # squash 4 #\" \\\n \t\tEXPECT_HEADER_COUNT=4 \\\n \t\tgit rebase -i $base &&\n@@ -423,6 +446,7 @@ test_expect_success 'squash ignores comments' '\n test_expect_success 'squash ignores blank lines' '\n \tgit checkout -b skip-blank-lines E &&\n \tbase=$(git rev-parse HEAD~4) &&\n+\tset_fake_editor &&\n \tFAKE_COMMIT_AMEND=\"ONCE\" FAKE_LINES=\"> 1 > squash 2 > squash 3 > squash 4 >\" \\\n \t\tEXPECT_HEADER_COUNT=4 \\\n \t\tgit rebase -i $base &&\n@@ -435,6 +459,7 @@ test_expect_success 'squash ignores blank lines' '\n test_expect_success 'squash works as expected' '\n \tgit checkout -b squash-works no-conflict-branch &&\n \tone=$(git rev-parse HEAD~3) &&\n+\tset_fake_editor &&\n \tFAKE_LINES=\"1 squash 3 2\" EXPECT_HEADER_COUNT=2 \\\n \t\tgit rebase -i HEAD~3 &&\n \ttest $one = $(git rev-parse HEAD~2)\n@@ -443,6 +468,7 @@ test_expect_success 'squash works as expected' '\n test_expect_success 'interrupted squash works as expected' '\n \tgit checkout -b interrupted-squash conflict-branch &&\n \tone=$(git rev-parse HEAD~3) &&\n+\tset_fake_editor &&\n \t(\n \t\tFAKE_LINES=\"1 squash 3 2\" &&\n \t\texport FAKE_LINES &&\n@@ -460,6 +486,7 @@ test_expect_success 'interrupted squash works as expected' '\n test_expect_success 'interrupted squash works as expected (case 2)' '\n \tgit checkout -b interrupted-squash2 conflict-branch &&\n \tone=$(git rev-parse HEAD~3) &&\n+\tset_fake_editor &&\n \t(\n \t\tFAKE_LINES=\"3 squash 1 2\" &&\n \t\texport FAKE_LINES &&\n@@ -484,6 +511,7 @@ test_expect_success '--continue tries to commit, even for \"edit\"' '\n \tgit commit -m \"unrelated change\" &&\n \tparent=$(git rev-parse HEAD^) &&\n \ttest_tick &&\n+\tset_fake_editor &&\n \tFAKE_LINES=\"edit 1\" git rebase -i HEAD^ &&\n \techo edited > file7 &&\n \tgit add file7 &&\n@@ -496,6 +524,7 @@ test_expect_success '--continue tries to commit, even for \"edit\"' '\n test_expect_success 'aborted --continue does not squash commits after \"edit\"' '\n \told=$(git rev-parse HEAD) &&\n \ttest_tick &&\n+\tset_fake_editor &&\n \tFAKE_LINES=\"edit 1\" git rebase -i HEAD^ &&\n \techo \"edited again\" > file7 &&\n \tgit add file7 &&\n@@ -510,6 +539,7 @@ test_expect_success 'aborted --continue does not squash commits after \"edit\"' '\n \n test_expect_success 'auto-amend only edited commits after \"edit\"' '\n \ttest_tick &&\n+\tset_fake_editor &&\n \tFAKE_LINES=\"edit 1\" git rebase -i HEAD^ &&\n \techo \"edited again\" > file7 &&\n \tgit add file7 &&\n@@ -528,6 +558,7 @@ test_expect_success 'auto-amend only edited commits after \"edit\"' '\n test_expect_success 'clean error after failed \"exec\"' '\n \ttest_tick &&\n \ttest_when_finished \"git rebase --abort || :\" &&\n+\tset_fake_editor &&\n \t(\n \t\tFAKE_LINES=\"1 exec_false\" &&\n \t\texport FAKE_LINES &&\n@@ -543,6 +574,7 @@ test_expect_success 'rebase a detached HEAD' '\n \tgrandparent=$(git rev-parse HEAD~2) &&\n \tgit checkout $(git rev-parse HEAD) &&\n \ttest_tick &&\n+\tset_fake_editor &&\n \tFAKE_LINES=\"2 1\" git rebase -i HEAD~2 &&\n \ttest $grandparent = $(git rev-parse HEAD~2)\n '\n@@ -559,6 +591,7 @@ test_expect_success 'rebase a commit violating pre-commit' '\n \ttest_must_fail git commit -m doesnt-verify file1 &&\n \tgit commit -m doesnt-verify --no-verify file1 &&\n \ttest_tick &&\n+\tset_fake_editor &&\n \tFAKE_LINES=2 git rebase -i HEAD~2\n \n '\n@@ -580,6 +613,7 @@ test_expect_success 'rebase with a file named HEAD in worktree' '\n \t\tgit commit -m \"Add body\"\n \t) &&\n \n+\tset_fake_editor &&\n \tFAKE_LINES=\"1 squash 2\" git rebase -i to-be-rebased &&\n \ttest \"$(git show -s --pretty=format:%an)\" = \"Squashed Away\"\n \n@@ -591,6 +625,7 @@ test_expect_success 'do \"noop\" when there is nothing to cherry-pick' '\n \tGIT_EDITOR=: git commit --amend \\\n \t\t--author=\"Somebody else <somebody@else.com>\" &&\n \ttest $(git rev-parse branch3) != $(git rev-parse branch4) &&\n+\tset_fake_editor &&\n \tgit rebase -i branch3 &&\n \ttest $(git rev-parse branch3) = $(git rev-parse branch4)\n \n@@ -615,10 +650,12 @@ test_expect_success 'submodule rebase setup' '\n \t\tgit commit -a -m \"submodule second\"\n \t) &&\n \ttest_tick &&\n+\tset_fake_editor &&\n \tgit commit -a -m \"Three changes submodule\"\n '\n \n test_expect_success 'submodule rebase -i' '\n+\tset_fake_editor &&\n \tFAKE_LINES=\"1 squash 2 3\" git rebase -i A\n '\n \n@@ -636,6 +673,7 @@ test_expect_success 'submodule conflict setup' '\n '\n \n test_expect_success 'rebase -i continue with only submodule staged' '\n+\tset_fake_editor &&\n \ttest_must_fail git rebase -i submodule-base &&\n \tgit add sub &&\n \tgit rebase --continue &&\n@@ -645,6 +683,7 @@ test_expect_success 'rebase -i continue with only submodule staged' '\n test_expect_success 'rebase -i continue with unstaged submodule' '\n \tgit checkout submodule-topic &&\n \tgit reset --hard &&\n+\tset_fake_editor &&\n \ttest_must_fail git rebase -i submodule-base &&\n \tgit reset &&\n \tgit rebase --continue &&\n@@ -657,6 +696,7 @@ test_expect_success 'avoid unnecessary reset' '\n \ttest-chmtime =123456789 file3 &&\n \tgit update-index --refresh &&\n \tHEAD=$(git rev-parse HEAD) &&\n+\tset_fake_editor &&\n \tgit rebase -i HEAD~4 &&\n \ttest $HEAD = $(git rev-parse HEAD) &&\n \tMTIME=$(test-chmtime -v +0 file3 | sed 's/[^0-9].*$//') &&\n@@ -665,6 +705,7 @@ test_expect_success 'avoid unnecessary reset' '\n \n test_expect_success 'reword' '\n \tgit checkout -b reword-branch master &&\n+\tset_fake_editor &&\n \tFAKE_LINES=\"1 2 3 reword 4\" FAKE_COMMIT_MESSAGE=\"E changed\" git rebase -i A &&\n \tgit show HEAD | grep \"E changed\" &&\n \ttest $(git rev-parse master) != $(git rev-parse HEAD) &&\n@@ -684,6 +725,7 @@ test_expect_success 'rebase -i can copy notes' '\n \ttest_commit n2 &&\n \ttest_commit n3 &&\n \tgit notes add -m\"a note\" n3 &&\n+\tset_fake_editor &&\n \tgit rebase -i --onto n1 n2 &&\n \ttest \"a note\" = \"$(git notes show HEAD)\"\n '\n@@ -697,6 +739,7 @@ EOF\n test_expect_success 'rebase -i can copy notes over a fixup' '\n \tgit reset --hard n3 &&\n \tgit notes add -m\"an earlier note\" n2 &&\n+\tset_fake_editor &&\n \tGIT_NOTES_REWRITE_MODE=concatenate FAKE_LINES=\"1 fixup 2\" git rebase -i n1 &&\n \tgit notes show > output &&\n \ttest_cmp expect output\n@@ -706,6 +749,7 @@ test_expect_success 'rebase while detaching HEAD' '\n \tgit symbolic-ref HEAD &&\n \tgrandparent=$(git rev-parse HEAD~2) &&\n \ttest_tick &&\n+\tset_fake_editor &&\n \tFAKE_LINES=\"2 1\" git rebase -i HEAD~2 HEAD^0 &&\n \ttest $grandparent = $(git rev-parse HEAD~2) &&\n \ttest_must_fail git symbolic-ref HEAD\n@@ -715,6 +759,7 @@ test_tick # Ensure that the rebased commits get a different timestamp.\n test_expect_success 'always cherry-pick with --no-ff' '\n \tgit checkout no-ff-branch &&\n \tgit tag original-no-ff-branch &&\n+\tset_fake_editor &&\n \tgit rebase -i --no-ff A &&\n \ttouch empty &&\n \tfor p in 0 1 2\n@@ -747,6 +792,7 @@ test_expect_success 'set up commits with funny messages' '\n test_expect_success 'rebase-i history with funny messages' '\n \tgit rev-list A..funny >expect &&\n \ttest_tick &&\n+\tset_fake_editor &&\n \tFAKE_LINES=\"1 2 3 4\" git rebase -i A &&\n \tgit rev-list A.. >actual &&\n \ttest_cmp expect actual\n@@ -763,6 +809,7 @@ test_expect_success 'prepare for rebase -i --exec' '\n \n \n test_expect_success 'running \"git rebase -i --exec git show HEAD\"' '\n+\tset_fake_editor &&\n \tgit rebase -i --exec \"git show HEAD\" HEAD~2 >actual &&\n \t(\n \t\tFAKE_LINES=\"1 exec_git_show_HEAD 2 exec_git_show_HEAD\" &&\n@@ -776,6 +823,7 @@ test_expect_success 'running \"git rebase -i --exec git show HEAD\"' '\n \n test_expect_success 'running \"git rebase --exec git show HEAD -i\"' '\n \tgit reset --hard execute &&\n+\tset_fake_editor &&\n \tgit rebase --exec \"git show HEAD\" -i HEAD~2 >actual &&\n \t(\n \t\tFAKE_LINES=\"1 exec_git_show_HEAD 2 exec_git_show_HEAD\" &&\n@@ -789,6 +837,7 @@ test_expect_success 'running \"git rebase --exec git show HEAD -i\"' '\n \n test_expect_success 'running \"git rebase -ix git show HEAD\"' '\n \tgit reset --hard execute &&\n+\tset_fake_editor &&\n \tgit rebase -ix \"git show HEAD\" HEAD~2 >actual &&\n \t(\n \t\tFAKE_LINES=\"1 exec_git_show_HEAD 2 exec_git_show_HEAD\" &&\n@@ -802,6 +851,7 @@ test_expect_success 'running \"git rebase -ix git show HEAD\"' '\n \n test_expect_success 'rebase -ix with several <CMD>' '\n \tgit reset --hard execute &&\n+\tset_fake_editor &&\n \tgit rebase -ix \"git show HEAD; pwd\" HEAD~2 >actual &&\n \t(\n \t\tFAKE_LINES=\"1 exec_git_show_HEAD;_pwd 2 exec_git_show_HEAD;_pwd\" &&\n@@ -815,6 +865,7 @@ test_expect_success 'rebase -ix with several <CMD>' '\n \n test_expect_success 'rebase -ix with several instances of --exec' '\n \tgit reset --hard execute &&\n+\tset_fake_editor &&\n \tgit rebase -i --exec \"git show HEAD\" --exec \"pwd\" HEAD~2 >actual &&\n \t(\n \t\tFAKE_LINES=\"1 exec_git_show_HEAD exec_pwd 2\n@@ -836,6 +887,7 @@ test_expect_success 'rebase -ix with --autosquash' '\n \techo bis >bis.txt &&\n \tgit add bis.txt &&\n \tgit commit -m \"fixup! two_exec\" &&\n+\tset_fake_editor &&\n \t(\n \t\tgit checkout -b autosquash_actual &&\n \t\tgit rebase -i --exec \"git show HEAD\" --autosquash HEAD~4 >actual\n@@ -854,6 +906,7 @@ test_expect_success 'rebase -ix with --autosquash' '\n \n test_expect_success 'rebase --exec without -i shows error message' '\n \tgit reset --hard execute &&\n+\tset_fake_editor &&\n \ttest_must_fail git rebase --exec \"git show HEAD\" HEAD~2 2>actual &&\n \techo \"The --exec option must be used with the --interactive option\" >expected &&\n \ttest_i18ncmp expected actual\n@@ -862,6 +915,7 @@ test_expect_success 'rebase --exec without -i shows error message' '\n \n test_expect_success 'rebase -i --exec without <CMD>' '\n \tgit reset --hard execute &&\n+\tset_fake_editor &&\n \ttest_must_fail git rebase -i --exec 2>tmp &&\n \tsed -e \"1d\" tmp >actual &&\n \ttest_must_fail git rebase -h >expected &&\n@@ -871,6 +925,7 @@ test_expect_success 'rebase -i --exec without <CMD>' '\n \n test_expect_success 'rebase -i --root re-order and drop commits' '\n \tgit checkout E &&\n+\tset_fake_editor &&\n \tFAKE_LINES=\"3 1 2 5\" git rebase -i --root &&\n \ttest E = $(git cat-file commit HEAD | sed -ne \\$p) &&\n \ttest B = $(git cat-file commit HEAD^ | sed -ne \\$p) &&\n@@ -884,6 +939,7 @@ test_expect_success 'rebase -i --root retain root commit author and message' '\n \techo B >file7 &&\n \tgit add file7 &&\n \tGIT_AUTHOR_NAME=\"Twerp Snog\" git commit -m \"different author\" &&\n+\tset_fake_editor &&\n \tFAKE_LINES=\"2\" git rebase -i --root &&\n \tgit cat-file commit HEAD | grep -q \"^author Twerp Snog\" &&\n \tgit cat-file commit HEAD | grep -q \"^different author$\"\n@@ -892,6 +948,7 @@ test_expect_success 'rebase -i --root retain root commit author and message' '\n test_expect_success 'rebase -i --root temporary sentinel commit' '\n \tgit checkout B &&\n \t(\n+\t\tset_fake_editor &&\n \t\tFAKE_LINES=\"2\" &&\n \t\texport FAKE_LINES &&\n \t\ttest_must_fail git rebase -i --root\n@@ -902,6 +959,7 @@ test_expect_success 'rebase -i --root temporary sentinel commit' '\n \n test_expect_success 'rebase -i --root fixup root commit' '\n \tgit checkout B &&\n+\tset_fake_editor &&\n \tFAKE_LINES=\"1 fixup 2\" git rebase -i --root &&\n \ttest A = $(git cat-file commit HEAD | sed -ne \\$p) &&\n \ttest B = $(git show HEAD:file1) &&\n@@ -911,6 +969,7 @@ test_expect_success 'rebase -i --root fixup root commit' '\n test_expect_success 'rebase --edit-todo does not works on non-interactive rebase' '\n \tgit reset --hard &&\n \tgit checkout conflict-branch &&\n+\tset_fake_editor &&\n \ttest_must_fail git rebase --onto HEAD~2 HEAD~ &&\n \ttest_must_fail git rebase --edit-todo &&\n \tgit rebase --abort\n@@ -919,6 +978,7 @@ test_expect_success 'rebase --edit-todo does not works on non-interactive rebase\n test_expect_success 'rebase --edit-todo can be used to modify todo' '\n \tgit reset --hard &&\n \tgit checkout no-conflict-branch^0 &&\n+\tset_fake_editor &&\n \tFAKE_LINES=\"edit 1 2 3\" git rebase -i HEAD~3 &&\n \tFAKE_LINES=\"2 1\" git rebase --edit-todo &&\n \tgit rebase --continue\n@@ -929,6 +989,7 @@ test_expect_success 'rebase --edit-todo can be used to modify todo' '\n test_expect_success 'rebase -i produces readable reflog' '\n \tgit reset --hard &&\n \tgit branch -f branch-reflog-test H &&\n+\tset_fake_editor &&\n \tgit rebase -i --onto I F branch-reflog-test &&\n \tcat >expect <<-\\EOF &&\n \trebase -i (start): checkout I\n@@ -949,7 +1010,6 @@ test_expect_success 'rebase -i respects core.commentchar' '\n \tsed -e \"2,\\$s/^/\\\\\\\\/\" \"$1\" >\"$1.tmp\" &&\n \tmv \"$1.tmp\" \"$1\"\n \tEOF\n-\ttest_when_finished \"set_fake_editor\" &&\n \ttest_set_editor \"$(pwd)/remove-all-but-first.sh\" &&\n \tgit rebase -i B &&\n \ttest B = $(git cat-file commit HEAD^ | sed -ne \\$p)\n@@ -1008,6 +1068,7 @@ test_expect_success 'short SHA-1 collide' '\n \t(\n \tunset test_tick &&\n \ttest_tick &&\n+\tset_fake_editor &&\n \tFAKE_COMMIT_MESSAGE=\"collide2 815200e\" \\\n \tFAKE_LINES=\"reword 1 2\" git rebase -i HEAD~2\n \t)\n-- \n1.8.4.rc4.499.gfb33910\n"},{"id":"225631","messageId":"1377112378-45511-4-git-send-email-sunshine@sunshineco.com","threadId":"34742","inReplyTo":"1377112378-45511-1-git-send-email-sunshine@sunshineco.com","subject":"[PATCH 3/3] t3404: simplify short SHA-1 collision test","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2013-08-21T19:12:58Z","receivedAt":"2013-08-21T19:12:58Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"The short SHA-1 collision test employs two \"magic\" values in commit\nmessages to trigger an \"ambiguous SHA-1 error\" during 'rebase -i' -- one\nfor 'collide3' during setup, and one for 'collide2' at rebase time --\neven though the collision can just as easily be triggered by a single\nmagic value at rebase time only. The setup-time 'collide3' magic value\nserves no purpose [1], but can easily mislead readers into thinking that\nit is somehow significant even though it is not. Drop this unneeded bit\nof magic.\n\nAs a side-effect, this also simplifies the setup-time \"test_commit\ncollide3\" invocation, making it easier to read.\n\n[1]: The magic value on 'collide3' gave it a short SHA-1 prefix of\n\"badbeef\" which could be spotted easily in verbose output during\ndevelopment of the test, but is otherwise not meaningful or helpful.\n\nSigned-off-by: Eric Sunshine <sunshine@sunshineco.com>\n---\n t/t3404-rebase-interactive.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex 7d15c7a..50e22b1 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -1058,7 +1058,7 @@ test_expect_success 'short SHA-1 setup' '\n \tunset test_tick &&\n \ttest_commit collide1 collide &&\n \ttest_commit --notick collide2 collide &&\n-\ttest_commit --notick \"collide3 115158b5\" collide collide3 collide3\n+\ttest_commit --notick collide3 collide\n \t)\n '\n \n@@ -1069,7 +1069,7 @@ test_expect_success 'short SHA-1 collide' '\n \tunset test_tick &&\n \ttest_tick &&\n \tset_fake_editor &&\n-\tFAKE_COMMIT_MESSAGE=\"collide2 815200e\" \\\n+\tFAKE_COMMIT_MESSAGE=\"collide2 ac4f2ee\" \\\n \tFAKE_LINES=\"reword 1 2\" git rebase -i HEAD~2\n \t)\n '\n-- \n1.8.4.rc4.499.gfb33910\n"},{"id":"225661","messageId":"xmqqk3jesk2a.fsf@gitster.dls.corp.google.com","threadId":"34742","inReplyTo":"1377112378-45511-1-git-send-email-sunshine@sunshineco.com","subject":"Re: [PATCH 0/3] t3404 incremental improvements","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-08-21T23:25:33Z","receivedAt":"2013-08-21T23:25:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine <sunshine@sunshineco.com> writes:\n\n> This set of patches was meant to be a re-roll of [1] addressing Junio's\n> comments, however [1] graduated to 'next' before I found time to work on\n> it further, so these are instead incremental patches atop 'next'.\n\nJust FYI, 'next' will be rewound once the upcoming release is done,\nso we have a chance to rewind and squash.\n"},{"id":"225662","messageId":"xmqqfvu2sjya.fsf@gitster.dls.corp.google.com","threadId":"34742","inReplyTo":"1377112378-45511-3-git-send-email-sunshine@sunshineco.com","subject":"Re: [PATCH 2/3] t3404: make tests more self-contained","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-08-21T23:27:57Z","receivedAt":"2013-08-21T23:27:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine <sunshine@sunshineco.com> writes:\n\n> As its very first action, t3404 installs (via set_fake_editor) a\n> specialized $EDITOR which simplifies automated 'rebase -i' testing. Many\n> tests rely upon this setting, thus tests which need a different editor\n> must take extra care upon completion to restore $EDITOR in order to\n> avoid breaking following tests. This places extra burden upon such tests\n> and requires that they undesirably have extra knowledge about\n> surrounding tests. Ease this burden by having each test install the\n> $EDITOR it requires, rather than relying upon a global setting.\n>\n> Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>\n> ---\n\nMakes sense.  Thanks.\n"},{"id":"225681","messageId":"CAPig+cRM_j7KOKG+YS1aTqz7suDY5LC7kh5-oP5H-GAenUqq-w@mail.gmail.com","threadId":"34742","inReplyTo":"xmqqk3jesk2a.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 0/3] t3404 incremental improvements","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2013-08-22T18:17:03Z","receivedAt":"2013-08-22T18:17:03Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, Aug 21, 2013 at 7:25 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Eric Sunshine <sunshine@sunshineco.com> writes:\n>\n>> This set of patches was meant to be a re-roll of [1] addressing Junio's\n>> comments, however [1] graduated to 'next' before I found time to work on\n>> it further, so these are instead incremental patches atop 'next'.\n>\n> Just FYI, 'next' will be rewound once the upcoming release is done,\n> so we have a chance to rewind and squash.\n\nHow would we go about this? Is there something I can do to streamline\nthe squashing?\n\nUnfortunately, the various fix-up patches do not have a one-to-one\ncorrespondence to the original three patches in 'next'.\n"},{"id":"225682","messageId":"xmqqtxihr1kv.fsf@gitster.dls.corp.google.com","threadId":"34742","inReplyTo":"CAPig+cRM_j7KOKG+YS1aTqz7suDY5LC7kh5-oP5H-GAenUqq-w@mail.gmail.com","subject":"Re: [PATCH 0/3] t3404 incremental improvements","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-08-22T19:02:24Z","receivedAt":"2013-08-22T19:02:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine <sunshine@sunshineco.com> writes:\n\n> On Wed, Aug 21, 2013 at 7:25 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>> Eric Sunshine <sunshine@sunshineco.com> writes:\n>>\n>>> This set of patches was meant to be a re-roll of [1] addressing Junio's\n>>> comments, however [1] graduated to 'next' before I found time to work on\n>>> it further, so these are instead incremental patches atop 'next'.\n>>\n>> Just FYI, 'next' will be rewound once the upcoming release is done,\n>> so we have a chance to rewind and squash.\n>\n> How would we go about this? Is there something I can do to streamline\n> the squashing?\n>\n> Unfortunately, the various fix-up patches do not have a one-to-one\n> correspondence to the original three patches in 'next'.\n\nThe most stream-lined way would be to send a replacement series\nearly next week, by which time hopefully the 1.8.4 final is out; as\nlong as the end-results of applying the series are the same, we know\nthat the new code we will be using is the same as the code already\nin 'next' that people have been testing.\n\nThat way, there is no risk of me screwing up while trying to wiggle\nthe existing patches and ending up with a split that do not match a\nlogical progression of the series you would expect to see.\n"},{"id":"225833","messageId":"20130825055550.GN2882@elie.Belkin","threadId":"34742","inReplyTo":"1377112378-45511-2-git-send-email-sunshine@sunshineco.com","subject":"Re: [PATCH 1/3] t3404: preserve test_tick state across short SHA-1 collision test","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-08-25T05:55:50Z","receivedAt":"2013-08-25T05:55:50Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nEric Sunshine wrote:\n\n> The short SHA-1 collision test requires carefully crafted commits in\n> order to ensure a collision at rebase time.\n\nYeah, this breaks the usual rule that tests should be independent\nof hashing function.  But it's the best we can do, I think.\n\n[...]\n> --- a/t/t3404-rebase-interactive.sh\n> +++ b/t/t3404-rebase-interactive.sh\n> @@ -994,17 +994,23 @@ test_expect_success 'short SHA-1 setup' '\n>  \ttest_when_finished \"git checkout master\" &&\n>  \tgit checkout --orphan collide &&\n>  \tgit rm -rf . &&\n> +\t(\n>  \tunset test_tick &&\n>  \ttest_commit collide1 collide &&\n>  \ttest_commit --notick collide2 collide &&\n>  \ttest_commit --notick \"collide3 115158b5\" collide collide3 collide3\n> +\t)\n\nWould be clearer if the code in a subshell were indented:\n\n\t(\n\t\tunset test_tick &&\n\t\ttest_commit ...\n\t)\n\n[...]\n>  test_expect_success 'short SHA-1 collide' '\n>  \ttest_when_finished \"reset_rebase && git checkout master\" &&\n>  \tgit checkout collide &&\n> +\t(\n> +\tunset test_tick &&\n> +\ttest_tick &&\n>  \tFAKE_COMMIT_MESSAGE=\"collide2 815200e\" \\\n>  \tFAKE_LINES=\"reword 1 2\" git rebase -i HEAD~2\n> +\t)\n\nLikewise.\n\nHope that helps,\nJonathan\n"},{"id":"225842","messageId":"CAPig+cSns+_mOMRVRv2Kqb3vHNQGoE4NGZ3ELu7y3pQ57VqPXw@mail.gmail.com","threadId":"34742","inReplyTo":"20130825055550.GN2882@elie.Belkin","subject":"Re: [PATCH 1/3] t3404: preserve test_tick state across short SHA-1 collision test","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2013-08-25T07:53:56Z","receivedAt":"2013-08-25T07:53:56Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Aug 25, 2013 at 1:55 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:\n> Hi,\n>\n> Eric Sunshine wrote:\n>\n>> The short SHA-1 collision test requires carefully crafted commits in\n>> order to ensure a collision at rebase time.\n>\n> Yeah, this breaks the usual rule that tests should be independent\n> of hashing function.  But it's the best we can do, I think.\n>\n> [...]\n>> --- a/t/t3404-rebase-interactive.sh\n>> +++ b/t/t3404-rebase-interactive.sh\n>> @@ -994,17 +994,23 @@ test_expect_success 'short SHA-1 setup' '\n>>       test_when_finished \"git checkout master\" &&\n>>       git checkout --orphan collide &&\n>>       git rm -rf . &&\n>> +     (\n>>       unset test_tick &&\n>>       test_commit collide1 collide &&\n>>       test_commit --notick collide2 collide &&\n>>       test_commit --notick \"collide3 115158b5\" collide collide3 collide3\n>> +     )\n>\n> Would be clearer if the code in a subshell were indented:\n>\n>         (\n>                 unset test_tick &&\n>                 test_commit ...\n>         )\n\nI considered it, but decided against it for a couple reasons:\n\n* In this script, there already is a mix between the two styles:\nindented vs. unindented.\n\n* In this particular patch, the test_commit line creating commit3\nwrapped beyond 80 columns when indented.\n\nIn v2 of the series (for which you also made the same observation),\nthe collide3 test_commit line is shorter, so I could have indented,\nhowever, I left it alone since nobody complained about it (and because\nthere already is the mix of styles). Should this be worth a re-roll?\n\n> [...]\n>>  test_expect_success 'short SHA-1 collide' '\n>>       test_when_finished \"reset_rebase && git checkout master\" &&\n>>       git checkout collide &&\n>> +     (\n>> +     unset test_tick &&\n>> +     test_tick &&\n>>       FAKE_COMMIT_MESSAGE=\"collide2 815200e\" \\\n>>       FAKE_LINES=\"reword 1 2\" git rebase -i HEAD~2\n>> +     )\n>\n> Likewise.\n>\n> Hope that helps,\n> Jonathan\n"},{"id":"225847","messageId":"20130825085318.GV2882@elie.Belkin","threadId":"34742","inReplyTo":"CAPig+cSns+_mOMRVRv2Kqb3vHNQGoE4NGZ3ELu7y3pQ57VqPXw@mail.gmail.com","subject":"Re: [PATCH 1/3] t3404: preserve test_tick state across short SHA-1 collision test","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-08-25T08:53:18Z","receivedAt":"2013-08-25T08:53:18Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Eric Sunshine wrote:\n> On Sun, Aug 25, 2013 at 1:55 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:\n\n>> Would be clearer if the code in a subshell were indented:\n>>\n>>         (\n>>                 unset test_tick &&\n>>                 test_commit ...\n>>         )\n>\n> I considered it, but decided against it for a couple reasons:\n>\n> * In this script, there already is a mix between the two styles:\n> indented vs. unindented.\n>\n> * In this particular patch, the test_commit line creating commit3\n> wrapped beyond 80 columns when indented.\n\nI'm just one person, but I find the indented form way more readable.\nLong lines or lines with \\ continuation are not a big deal.\n\n[...]\n>                                      Should this be worth a re-roll?\n\nSince the file already has a mixture of styles, if there's no other\nreason to reroll, I'd suggest leaving it alone.\n\nThe next time it bugs me or someone else, that person can write a\npatch that cleans up the whole file on top. :)\n\nThanks,\nJonathan\n"},{"id":"225879","messageId":"xmqqwqn9j9yz.fsf@gitster.dls.corp.google.com","threadId":"34742","inReplyTo":"20130825085318.GV2882@elie.Belkin","subject":"Re: [PATCH 1/3] t3404: preserve test_tick state across short SHA-1 collision test","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-08-26T05:29:40Z","receivedAt":"2013-08-26T05:29:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder <jrnieder@gmail.com> writes:\n\n> Eric Sunshine wrote:\n>> On Sun, Aug 25, 2013 at 1:55 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:\n>\n>>> Would be clearer if the code in a subshell were indented:\n>>>\n>>>         (\n>>>                 unset test_tick &&\n>>>                 test_commit ...\n>>>         )\n>>\n>> I considered it, but decided against it for a couple reasons:\n>>\n>> * In this script, there already is a mix between the two styles:\n>> indented vs. unindented.\n>>\n>> * In this particular patch, the test_commit line creating commit3\n>> wrapped beyond 80 columns when indented.\n>\n> I'm just one person, but I find the indented form way more readable.\n> Long lines or lines with \\ continuation are not a big deal.\n>\n> [...]\n>>                                      Should this be worth a re-roll?\n>\n> Since the file already has a mixture of styles, if there's no other\n> reason to reroll, I'd suggest leaving it alone.\n>\n> The next time it bugs me or someone else, that person can write a\n> patch that cleans up the whole file on top. :)\n\nYeah, I think we saw \"style modernization\" patches to some test\nscripts during the last cycle. This one and the rest may want to go\nthrough a similar modernization, but it can be done after a real\nchange like the proposed one matures.\n\nThanks.\n"}]}