{"thread":{"id":"59169","subject":"[GSoC][PATCH] t/t4113-apply-ending.sh: Modernize a test script","startedAt":"2023-01-31T22:50:00Z","lastAt":"2023-02-15T02:40:57Z","messageCount":49,"participants":["Shuqi Liang","Andrei Rybak","cheska fran","Junio C Hamano","Eric Sunshine","Ævar Arnfjörð Bjarmason"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"471209","messageId":"20230131224929.2018546-1-cheskaqiqi@gmail.com","threadId":"59169","inReplyTo":null,"subject":"[GSoC][PATCH] t/t4113-apply-ending.sh: Modernize a test script","fromName":"Shuqi Liang","fromEmail":"cheskaqiqi@gmail.com","sentAt":"2023-01-31T22:49:29Z","receivedAt":"2023-01-31T22:50:00Z","isPatch":true,"sender":{"key":"cheskaqiqi@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109261504?v=4"},"body":"\nI cleaned up some old style in test script.\n\nfor example :\n\n* old style:\n\n test_expect_success \\\n 'title' \\\n 'body line 1 &&\n body line 2'\n\n should become:\n\n test_expect_success 'title' '\n body line 1 &&\n body line 2\n '\n\n\n\n\nSigned-off-by: Shuqi Liang \n---\nHi,I'm Shuqi Liang.a junior student majors in Computer Science at University of Western Ontario. \n\nThis patch is the microproject I try to getting involved with the Git project.\n\nI have read 'MyFirstContribution.txt', 'Hacking Git' and the book 《pro git》 ,and I know more about objects, references, packfile format, etc.\nOver the coming period, I will delve into the source code and gain a deeper understanding and try to contribute more meaningful patch to the community.\n\n t/t4113-apply-ending.sh | 16 +++++++++-------\n 1 file changed, 9 insertions(+), 7 deletions(-)\n\ndiff --git a/t/t4113-apply-ending.sh b/t/t4113-apply-ending.sh\nindex 66fa51591e..aa57895b22 100755\n--- a/t/t4113-apply-ending.sh\n+++ b/t/t4113-apply-ending.sh\n@@ -24,13 +24,14 @@ echo 'a' >file\n echo 'b' >>file\n echo 'c' >>file\n \n-test_expect_success setup \\\n- 'git update-index --add file'\n-\n+test_expect_success setup '\n+ git update-index --add file\n+'\n # test\n \n-test_expect_success 'apply at the end' \\\n- 'test_must_fail git apply --index test-patch'\n+test_expect_success 'apply at the end' '\n+ test_must_fail git apply --index test-patch\n+'\n \n cat >test-patch <<\\EOF\n diff a/file b/file\n@@ -47,7 +48,8 @@ b\n c'\n git update-index file\n \n-test_expect_success 'apply at the beginning' \\\n-\t'test_must_fail git apply --index test-patch'\n+test_expect_success 'apply at the beginning' '\n+ test_must_fail git apply --index test-patch\n+'\n \n test_done\n-- \n2.39.0\n\n"},{"id":"471213","messageId":"b9a38b83-cdfe-0389-3097-c20a699f183c@gmail.com","threadId":"59169","inReplyTo":"20230131224929.2018546-1-cheskaqiqi@gmail.com","subject":"Re: [GSoC][PATCH] t/t4113-apply-ending.sh: Modernize a test script","fromName":"Andrei Rybak","fromEmail":"rybak.a.v@gmail.com","sentAt":"2023-02-01T02:21:03Z","receivedAt":"2023-02-01T02:21:10Z","isPatch":true,"sender":{"key":"rybak.a.v@gmail.com","avatar":"https://avatars.githubusercontent.com/u/624072?v=4"},"body":"Hi Shuqi Liang,\n\n> Subject: [GSoC][PATCH] t/t4113-apply-ending.sh: Modernize a test script\n\nFor patches that change a single test, the subject line can include just\nthe \"t\" and the number. The part after the colon should start with a\nlowercase letter. Something like\n\n t4113: modernize test style\n\nOn 31/01/2023 23:49, Shuqi Liang wrote:\n> \n> I cleaned up some old style in test script.\n\nCommit message should start with description of the existing problem\nin present tense, something like:\n\n Test scripts in file t4113-apply-ending.sh are written in old style,\n where the test_expect_success command and test title are written on\n separate lines ...\n\nThen changes should be described using imperative mood, as if you are\ngiving commands to the codebase. See section \"[[describe-changes]]\"\nin \"Documentation/SubmittingPatches\" for details.\n\nYou can also find examples of existing commit messages for similar\nchanges:\n\n $ git log --no-merges --grep='modernize' -- t\n\n> \n> for example :\n> \n> * old style:\n> \n> test_expect_success \\\n> 'title' \\\n> 'body line 1 &&\n> body line 2'\n> \n> should become:\n> \n> test_expect_success 'title' '\n> body line 1 &&\n> body line 2\n> '\n> \n> \n> \n> \n> Signed-off-by: Shuqi Liang \n> ---\n > Hi,I'm Shuqi Liang.a junior student majors in Computer Science at\n > University of Western Ontario.\n\nWelcome!\n\n> t/t4113-apply-ending.sh | 16 +++++++++-------\n> 1 file changed, 9 insertions(+), 7 deletions(-)\n> \n> diff --git a/t/t4113-apply-ending.sh b/t/t4113-apply-ending.sh\n> index 66fa51591e..aa57895b22 100755\n> --- a/t/t4113-apply-ending.sh\n> +++ b/t/t4113-apply-ending.sh\n> @@ -24,13 +24,14 @@ echo 'a' >file\n> echo 'b' >>file\n> echo 'c' >>file\n\nA \"modern\" test could also do such preparation for test files as\npart of its \"setup\" step. This could its own patch in the same\nseries, separate from style changes.\n\nIn case of t4113, files \"test-patch\" and \"file\" are created twice.\nThe second creation of the files could be either its own step\n'setup for apply at the beginning', or incorporated into the step\n'apply at the beginning'.\n\nSection \"Recommended style\" in t/README also has some notes about\nhow heredocs should be indented.\n\n> \n> -test_expect_success setup \\\n> - 'git update-index --add file'\n> -\n> +test_expect_success setup '\n> + git update-index --add file\n> +'\n\nWhile changing the quoting around test tiles and commands, the\nindentation with spaces could also be changed to TABs.\n\n> # test\n\nIf the setup code on top level of the file is moved into test\nsteps, this comment and the \"# setup\" comment at line 11 will\nbecome unnecessary.\n\n> \n> -test_expect_success 'apply at the end' \\\n> - 'test_must_fail git apply --index test-patch'\n> +test_expect_success 'apply at the end' '\n> + test_must_fail git apply --index test-patch\n> +'\n> \n> cat >test-patch <<\\EOF\n> diff a/file b/file\n> @@ -47,7 +48,8 @@ b\n> c'\n> git update-index file\n> \n> -test_expect_success 'apply at the beginning' \\\n> -\t'test_must_fail git apply --index test-patch'\n> +test_expect_success 'apply at the beginning' '\n> + test_must_fail git apply --index test-patch\n> +'\n> \n> test_done\n\nThanks.\n\n"},{"id":"471375","messageId":"20230202171821.10508-1-cheskaqiqi@gmail.com","threadId":"59169","inReplyTo":"20230131224929.2018546-1-cheskaqiqi@gmail.com","subject":"[PATCH v2 0/4] t4113: modernize test style","fromName":"Shuqi Liang","fromEmail":"cheskaqiqi@gmail.com","sentAt":"2023-02-02T17:18:17Z","receivedAt":"2023-02-02T17:19:53Z","isPatch":true,"sender":{"key":"cheskaqiqi@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109261504?v=4"},"body":"Modernize the test script t4113.\n\nComparing to v1:\n\n1.Test scripts in file in this script are written in old style,\n where the test_expect_success command and test title are written on\n separate lines.Change the old style '\\' to new style \"'\".\nfor example :\n-test_expect_success setup \\\n- 'git update-index --add file'\n-\n+test_expect_success setup '\n+ git update-index --add file\n+' \n\n2.Files \"test-patch\" and \"file\" are created twice.\nput the second creation of the files to its own step\n'setup for apply at the beginning'\n\n3.This script still use the old style \"<<\".\nChange \"<<-\" instead of \"<<\"\nfor exmaple:\n-\tcat >test-patch <<\\EOF\n+\tcat >test-patch <<-\\EOF\n\n4.The test bodies in this script are written in old style .which indented with space, but not TAB.replace indentation spaces with tabs.\nfor example :\n\n test_expect_success setup '\n- git update-index --add file\n+\tgit update-index --add file\n '\n\nShuqi Liang (4):\n t/t4113-apply-ending.sh: Modernize a test script\n Test scripts in file t4113-apply-ending.sh, files \"test-patch\" and\n \"file\" are created twice.\n use \"<<-\" instead of \"<<\"\n t4113-apply-ending.sh used 4-column indent with space,fix it in use\n tabs for indentation.\n\n t/t4113-apply-ending.sh | 51 +++++++++++++++++++++--------------------\n 1 file changed, 26 insertions(+), 25 deletions(-)\n\nRange-diff against v1:\n1: d7d52f1f79 = 1: d7d52f1f79 t/t4113-apply-ending.sh: Modernize a test script\n-: ---------- > 2: d9e5a54e32 Test scripts in file t4113-apply-ending.sh, files \"test-patch\" and \"file\" are created twice.\n-: ---------- > 3: 01a5c3285e use \"<<-\" instead of \"<<\"\n-: ---------- > 4: cf2b2ca5a0 t4113-apply-ending.sh used 4-column indent with space,fix it in use tabs for indentation.\n-- \n2.39.0\n\n"},{"id":"471376","messageId":"20230202171821.10508-2-cheskaqiqi@gmail.com","threadId":"59169","inReplyTo":"20230202171821.10508-1-cheskaqiqi@gmail.com","subject":"[PATCH v2 1/4]t4113: replace backslash with single quote","fromName":"Shuqi Liang","fromEmail":"cheskaqiqi@gmail.com","sentAt":"2023-02-02T17:18:18Z","receivedAt":"2023-02-02T17:19:59Z","isPatch":true,"sender":{"key":"cheskaqiqi@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109261504?v=4"},"body":"Change the old style '\\' to new style \"'\"\n\nSigned-off-by: Shuqi Liang \n---\n t/t4113-apply-ending.sh | 16 +++++++++-------\n 1 file changed, 9 insertions(+), 7 deletions(-)\n\ndiff --git a/t/t4113-apply-ending.sh b/t/t4113-apply-ending.sh\nindex 66fa51591e..aa57895b22 100755\n--- a/t/t4113-apply-ending.sh\n+++ b/t/t4113-apply-ending.sh\n@@ -24,13 +24,14 @@ echo 'a' >file\n echo 'b' >>file\n echo 'c' >>file\n \n-test_expect_success setup \\\n- 'git update-index --add file'\n-\n+test_expect_success setup '\n+ git update-index --add file\n+'\n # test\n \n-test_expect_success 'apply at the end' \\\n- 'test_must_fail git apply --index test-patch'\n+test_expect_success 'apply at the end' '\n+ test_must_fail git apply --index test-patch\n+'\n \n cat >test-patch <<\\EOF\n diff a/file b/file\n@@ -47,7 +48,8 @@ b\n c'\n git update-index file\n \n-test_expect_success 'apply at the beginning' \\\n-\t'test_must_fail git apply --index test-patch'\n+test_expect_success 'apply at the beginning' '\n+ test_must_fail git apply --index test-patch\n+'\n \n test_done\n-- \n2.39.0\n\n"},{"id":"471377","messageId":"20230202171821.10508-3-cheskaqiqi@gmail.com","threadId":"59169","inReplyTo":"20230202171821.10508-1-cheskaqiqi@gmail.com","subject":"[PATCH v2 2/4] t4113:put second creation in own step","fromName":"Shuqi Liang","fromEmail":"cheskaqiqi@gmail.com","sentAt":"2023-02-02T17:18:19Z","receivedAt":"2023-02-02T17:20:04Z","isPatch":true,"sender":{"key":"cheskaqiqi@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109261504?v=4"},"body":"files \"test-patch\" and \"file\" are created twice.put the second creation \nof the files to its own step'setup for apply at the beginning'.\n\nMake the second creation of the files its own step\n'setup for apply at the beginning'.\n\nSigned-off-by: Shuqi Liang \n---\n t/t4113-apply-ending.sh | 33 ++++++++++++++++-----------------\n 1 file changed, 16 insertions(+), 17 deletions(-)\n\ndiff --git a/t/t4113-apply-ending.sh b/t/t4113-apply-ending.sh\nindex aa57895b22..d84f632bc3 100755\n--- a/t/t4113-apply-ending.sh\n+++ b/t/t4113-apply-ending.sh\n@@ -8,8 +8,6 @@ test_description='git apply trying to add an ending line.\n '\n . ./test-lib.sh\n \n-# setup\n-\n cat >test-patch <<\\EOF\n diff --git a/file b/file\n --- a/file\n@@ -27,26 +25,27 @@ echo 'c' >>file\n test_expect_success setup '\n git update-index --add file\n '\n-# test\n \n test_expect_success 'apply at the end' '\n test_must_fail git apply --index test-patch\n '\n \n-cat >test-patch <<\\EOF\n-diff a/file b/file\n---- a/file\n-+++ b/file\n-@@ -1,2 +1,3 @@\n-+a\n- b\n- c\n-EOF\n-\n-echo >file 'a\n-b\n-c'\n-git update-index file\n+test_expect_success 'setup for apply at the beginning' '\n+\tcat >test-patch <<\\EOF\n+\tdiff a/file b/file\n+\t--- a/file\n+\t+++ b/file\n+\t@@ -1,2 +1,3 @@\n+\t+a\n+\tb\n+\tc\n+\tEOF\n+\n+\techo >file 'a\n+\tb\n+\tc'\n+\tgit update-index file\n+'\n \n test_expect_success 'apply at the beginning' '\n test_must_fail git apply --index test-patch\n-- \n2.39.0\n\n"},{"id":"471378","messageId":"20230202171821.10508-4-cheskaqiqi@gmail.com","threadId":"59169","inReplyTo":"20230202171821.10508-1-cheskaqiqi@gmail.com","subject":"[PATCH v2 3/4] t4113: use \"<<-\" instead of \"<<\"","fromName":"Shuqi Liang","fromEmail":"cheskaqiqi@gmail.com","sentAt":"2023-02-02T17:18:20Z","receivedAt":"2023-02-02T17:20:15Z","isPatch":true,"sender":{"key":"cheskaqiqi@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109261504?v=4"},"body":"use \"<<-\" instead of \"<<\"\n\nSigned-off-by: Shuqi Liang \n---\n t/t4113-apply-ending.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t4113-apply-ending.sh b/t/t4113-apply-ending.sh\nindex d84f632bc3..d5b15e97d3 100755\n--- a/t/t4113-apply-ending.sh\n+++ b/t/t4113-apply-ending.sh\n@@ -8,7 +8,7 @@ test_description='git apply trying to add an ending line.\n '\n . ./test-lib.sh\n \n-cat >test-patch <<\\EOF\n+cat >test-patch <<-\\EOF\n diff --git a/file b/file\n --- a/file\n +++ b/file\n@@ -31,7 +31,7 @@ test_expect_success 'apply at the end' '\n '\n \n test_expect_success 'setup for apply at the beginning' '\n-\tcat >test-patch <<\\EOF\n+\tcat >test-patch <<-\\EOF\n \tdiff a/file b/file\n \t--- a/file\n \t+++ b/file\n-- \n2.39.0\n\n"},{"id":"471379","messageId":"20230202171821.10508-5-cheskaqiqi@gmail.com","threadId":"59169","inReplyTo":"20230202171821.10508-1-cheskaqiqi@gmail.com","subject":"[PATCH v2 4/4] t4113: indent with tabs","fromName":"Shuqi Liang","fromEmail":"cheskaqiqi@gmail.com","sentAt":"2023-02-02T17:18:21Z","receivedAt":"2023-02-02T17:20:16Z","isPatch":true,"sender":{"key":"cheskaqiqi@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109261504?v=4"},"body":"t4113-apply-ending.sh used 4-column indent with\nspace,fix it in use tabs for indentation.\n\nSigned-off-by: Shuqi Liang \n---\n t/t4113-apply-ending.sh | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/t/t4113-apply-ending.sh b/t/t4113-apply-ending.sh\nindex d5b15e97d3..9e28c72355 100755\n--- a/t/t4113-apply-ending.sh\n+++ b/t/t4113-apply-ending.sh\n@@ -23,11 +23,11 @@ echo 'b' >>file\n echo 'c' >>file\n \n test_expect_success setup '\n- git update-index --add file\n+\tgit update-index --add file\n '\n \n test_expect_success 'apply at the end' '\n- test_must_fail git apply --index test-patch\n+\ttest_must_fail git apply --index test-patch\n '\n \n test_expect_success 'setup for apply at the beginning' '\n@@ -48,7 +48,7 @@ test_expect_success 'setup for apply at the beginning' '\n '\n \n test_expect_success 'apply at the beginning' '\n- test_must_fail git apply --index test-patch\n+\ttest_must_fail git apply --index test-patch\n '\n \n test_done\n-- \n2.39.0\n\n"},{"id":"471380","messageId":"CAMO4yUHJCWGNV7esYqC7T4HB=pv3=Z4vTfaBFzh9hAsftfnN3w@mail.gmail.com","threadId":"59169","inReplyTo":"b9a38b83-cdfe-0389-3097-c20a699f183c@gmail.com","subject":"Re: [GSoC][PATCH] t/t4113-apply-ending.sh: Modernize a test script","fromName":"cheska fran","fromEmail":"cheskaqiqi@gmail.com","sentAt":"2023-02-02T17:20:30Z","receivedAt":"2023-02-02T17:20:49Z","isPatch":true,"sender":{"key":"cheskaqiqi@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109261504?v=4"},"body":"Hi Andrei,\n\n\nAndrei Rybak On 31/01/2023 21:21：\n> For patches that change a single test, the subject line can include just\n> the \"t\" and the number. The part after the colon should start with a\n> lowercase letter. Something like\n>\n> t4113: modernize test style\n>\n\nThanks, this tip is really helpful. I will change it.\n\n\n> Commit message should start with description of the existing problem\n> in present tense, something like:\n>\n> Test scripts in file t4113-apply-ending.sh are written in old style,\n> where the test_expect_success command and test title are written on\n> separate lines ...\n>\n> Then changes should be described using imperative mood, as if you are\n> giving commands to the codebase. See section \"[[describe-changes]]\"\n> in \"Documentation/SubmittingPatches\" for details.\n\n> You can also find examples of existing commit messages for similar\n> changes:\n>\n> $ git log --no-merges --grep='modernize' -- t\n>\nThanks,that is cool! I tried it and I saw a lot of examples and their\ndescriptions were very clear and I learned a lot\n\n\n> In case of t4113, files \"test-patch\" and \"file\" are created twice.\n> The second creation of the files could be either its own step\n> 'setup for apply at the beginning', or incorporated into the step\n> 'apply at the beginning'.\nyeah,once before the first instance of test-patch and then again\nbefore the second instance of test-patch.\nI will move the second creation of the files to its own step in setup\nfor apply at the beginning.'\n\n> Section \"Recommended style\" in t/README also has some notes about\n> how heredocs should be indented.\n\nSure, I did not realize this.I will use \"<<-\" instead of \"<<\".\n\n> While changing the quoting around test tiles and commands, the\n> indentation with spaces could also be changed to TABs.\nwill do.\n\n> If the setup code on top level of the file is moved into test\n> steps, this comment and the \"# setup\" comment at line 11 will\n> become unnecessary.\nThanks. It's easy to miss.The purpose of these tags is to distinguish\nthe setup and test parts of the script,\nbut if the file creation has been moved to a separate step, then these\ntags are no longer needed.\n\n\n\nThanks for the reply and it is really helpful!\n\n--\nThanks,\nShuqi\n"},{"id":"471394","messageId":"xmqq1qn7u6if.fsf@gitster.g","threadId":"59169","inReplyTo":"20230202171821.10508-2-cheskaqiqi@gmail.com","subject":"Re: [PATCH v2 1/4]t4113: replace backslash with single quote","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-02-02T21:00:56Z","receivedAt":"2023-02-02T21:01:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Shuqi Liang writes:\n\nThe example Andrei gave you, i.e.\n\n Test scripts in file t4113-apply-ending.sh are written in old style,\n where the test_expect_success command and test title are written on\n separate lines ...\n\nwas quite readable, but this\n\n> Change the old style '\\' to new style \"'\"\n\nis almost impossible to understand without knowing that this wanted\nto say what Andrei gave in a different way. The title is worse.\nIt's not replacing a backslash with a single quote, which would\nresult in\n\n -test_expect_success setup \\\n +test_expect_success setup '\n 'git update-index --add file'\n\nand obviously that is not what you did (or wanted to do).\n\n> Signed-off-by: Shuqi Liang \n> ---\n> t/t4113-apply-ending.sh | 16 +++++++++-------\n> 1 file changed, 9 insertions(+), 7 deletions(-)\n\nThe patch text looks OK.\n"},{"id":"471395","messageId":"xmqqwn4zsrr2.fsf@gitster.g","threadId":"59169","inReplyTo":"20230202171821.10508-4-cheskaqiqi@gmail.com","subject":"Re: [PATCH v2 3/4] t4113: use \"<<-\" instead of \"<<\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-02-02T21:05:05Z","receivedAt":"2023-02-02T21:05:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Shuqi Liang writes:\n\n> use \"<<-\" instead of \"<<\"\n\nYou forgot to explain \"Why?\". What you did, anybody can see in the\npatch text. Why you did so is what you are expected to explain in\nyour proposed log message.\n\n> -cat >test-patch <<\\EOF\n> +cat >test-patch <<-\\EOF\n> diff --git a/file b/file\n> --- a/file\n> +++ b/file\n\nThere is no need to do this, as the body of the here-doc is not\nindented/prefixed with HT at all. \n\n> @@ -31,7 +31,7 @@ test_expect_success 'apply at the end' '\n> '\n> \n> test_expect_success 'setup for apply at the beginning' '\n> -\tcat >test-patch <<\\EOF\n> +\tcat >test-patch <<-\\EOF\n> \tdiff a/file b/file\n> \t--- a/file\n> \t+++ b/file\n\nThis is necessary but that is only because [PATCH v2 2/4] broke it.\nIn general, we frown upon a series where a bug is introduced in an\nearlier step, with another patch fixing that bug. \n\nLet's not introduce such a bug that we need to fix later from the\nbeginning instead.\n\nThanks.\n\n"},{"id":"471396","messageId":"xmqqsffnsrik.fsf@gitster.g","threadId":"59169","inReplyTo":"20230202171821.10508-5-cheskaqiqi@gmail.com","subject":"Re: [PATCH v2 4/4] t4113: indent with tabs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-02-02T21:10:11Z","receivedAt":"2023-02-02T21:10:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Shuqi Liang writes:\n\n> t4113-apply-ending.sh used 4-column indent with\n> space,fix it in use tabs for indentation.\n\nGood, but end the sentence with a full-top with a space after it,\nand start the next sentence with a capital letter.\n\n\n> Signed-off-by: Shuqi Liang \n> ---\n> t/t4113-apply-ending.sh | 6 +++---\n> 1 file changed, 3 insertions(+), 3 deletions(-)\n>\n> diff --git a/t/t4113-apply-ending.sh b/t/t4113-apply-ending.sh\n> index d5b15e97d3..9e28c72355 100755\n> --- a/t/t4113-apply-ending.sh\n> +++ b/t/t4113-apply-ending.sh\n> @@ -23,11 +23,11 @@ echo 'b' >>file\n> echo 'c' >>file\n> \n> test_expect_success setup '\n> - git update-index --add file\n> +\tgit update-index --add file\n> '\n\nThis is not wrong per se, but the modern style is to avoid having\nany executable lines outside test_expect_foo. I'd expect that the\nresulting script begins more like the attached. [PATCH 4/4] stops\nthe conversion in the middle, which leaves funny taste in our mouth.\n\nThanks.\n\ndiff --git i/t/t4113-apply-ending.sh w/t/t4113-apply-ending.sh\nindex 66fa51591e..9746f45898 100755\n--- i/t/t4113-apply-ending.sh\n+++ w/t/t4113-apply-ending.sh\n@@ -8,24 +8,20 @@ test_description='git apply trying to add an ending line.\n '\n . ./test-lib.sh\n \n-# setup\n-\n-cat >test-patch <<\\EOF\n-diff --git a/file b/file\n---- a/file\n-+++ b/file\n-@@ -1,2 +1,3 @@\n- a\n- b\n-+c\n-EOF\n-\n-echo 'a' >file\n-echo 'b' >>file\n-echo 'c' >>file\n-\n-test_expect_success setup \\\n- 'git update-index --add file'\n+test_expect_success setup '\n+\tcat >test-patch <<-\\EOF\n+\tdiff --git a/file b/file\n+\t--- a/file\n+\t+++ b/file\n+\t@@ -1,2 +1,3 @@\n+\t a\n+\t b\n+\t+c\n+\tEOF\n+\n+\ttest_write_lines a b c >file\n+\tgit update-index --add file\n+'\n \n # test\n \n\n\n"},{"id":"471520","messageId":"CAMO4yUEyYNHfvB7Tp1_=EKh7jqESvHKJ3YL5EuYEj1qQ23swfQ@mail.gmail.com","threadId":"59169","inReplyTo":"xmqq1qn7u6if.fsf@gitster.g","subject":"Re: [PATCH v2 1/4]t4113: replace backslash with single quote","fromName":"Shuqi Liang","fromEmail":"cheskaqiqi@gmail.com","sentAt":"2023-02-05T14:28:05Z","receivedAt":"2023-02-05T14:28:22Z","isPatch":true,"sender":{"key":"cheskaqiqi@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109261504?v=4"},"body":"Hi Junio,\n\nOn Thu, Feb 2, 2023 at 4:00 PM Junio C Hamano wrote:\n\n> is almost impossible to understand without knowing that this wanted\n> to say what Andrei gave in a different way. The title is worse.\n> It's not replacing a backslash with a single quote, which would\n> result in\n>\n> -test_expect_success setup \\\n> +test_expect_success setup '\n> 'git update-index --add file'\n>\n> and obviously that is not what you did (or wanted to do).\n\nThanks, I will modify it to make it clear about my motivation and the\nreal changes to my patch.\n"},{"id":"471521","messageId":"CAMO4yUEMHFCkcNa_oaEQLUCTTy+iwrahyWzyX=HTE0USv2WgPg@mail.gmail.com","threadId":"59169","inReplyTo":"xmqqwn4zsrr2.fsf@gitster.g","subject":"Re: [PATCH v2 3/4] t4113: use \"<<-\" instead of \"<<\"","fromName":"Shuqi Liang","fromEmail":"cheskaqiqi@gmail.com","sentAt":"2023-02-05T14:38:31Z","receivedAt":"2023-02-05T14:38:48Z","isPatch":true,"sender":{"key":"cheskaqiqi@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109261504?v=4"},"body":"On Thu, Feb 2, 2023 at 4:05 PM Junio C Hamano wrote:\n\n> You forgot to explain \"Why?\". What you did, anybody can see in the\n> patch text. Why you did so is what you are expected to explain in\n> your proposed log message.\nThanks. I did make a lot of mistakes in writing a good commit message.\nI will modify it.\n\n> > -cat >test-patch <<\\EOF\n> > +cat >test-patch <<-\\EOF\n> > diff --git a/file b/file\n> > --- a/file\n> > +++ b/file\n>\n> There is no need to do this, as the body of the here-doc is not\n> indented/prefixed with HT at all.\n\nyeah, I did not notice that , According to t/README says, Indent the\nbody of here-document, and use \"<<-\" instead of \"<<\"\nto strip leading TABs used for indentation. But here do not have the\nleading TABS in front of it.\n\n> > @@ -31,7 +31,7 @@ test_expect_success 'apply at the end' '\n> > '\n> >\n> > test_expect_success 'setup for apply at the beginning' '\n> > - cat >test-patch <<\\EOF\n> > + cat >test-patch <<-\\EOF\n> > diff a/file b/file\n> > --- a/file\n> > +++ b/file\n>\n> This is necessary but that is only because [PATCH v2 2/4] broke it.\n> In general, we frown upon a series where a bug is introduced in an\n> earlier step, with another patch fixing that bug.\n>\n> Let's not introduce such a bug that we need to fix later from the\n> beginning instead.\n\nThanks, I will introduce the new bug caused by the current patch in\nthe beginning.\n\n--------\nThanks,\nshuqi\n"},{"id":"471522","messageId":"CAMO4yUEfcnmfqBEDBxuLfAqwsxOTAOdmOf4L8VDvNq3-6qaFrQ@mail.gmail.com","threadId":"59169","inReplyTo":"xmqqsffnsrik.fsf@gitster.g","subject":"Re: [PATCH v2 4/4] t4113: indent with tabs","fromName":"Shuqi Liang","fromEmail":"cheskaqiqi@gmail.com","sentAt":"2023-02-05T14:51:52Z","receivedAt":"2023-02-05T14:52:08Z","isPatch":true,"sender":{"key":"cheskaqiqi@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109261504?v=4"},"body":"On Thu, Feb 2, 2023 at 4:10 PM Junio C Hamano wrote:\n\n> Good, but end the sentence with a full-top with a space after it,\n> and start the next sentence with a capital letter.\n\nSure, think I need to change it to \" ...space. Fix..\". I will pay more\nattention to my English written style.\n\n\n> This is not wrong per se, but the modern style is to avoid having\n> any executable lines outside test_expect_foo. I'd expect that the\n> resulting script begins more like the attached. [PATCH 4/4] stops\n> the conversion in the middle, which leaves funny taste in our mouth.\n\nThanks, Will avoid having any executable lines outside test_expect_foo.\n\n\nOverall, thanks for the reply and it is really helpful!\n--------------------------------\nShuqi\n"},{"id":"471523","messageId":"20230205145245.11078-1-cheskaqiqi@gmail.com","threadId":"59169","inReplyTo":"20230202171821.10508-1-cheskaqiqi@gmail.com","subject":"[PATCH v3 0/3] modernize style","fromName":"Shuqi Liang","fromEmail":"cheskaqiqi@gmail.com","sentAt":"2023-02-05T14:52:42Z","receivedAt":"2023-02-05T14:53:27Z","isPatch":true,"sender":{"key":"cheskaqiqi@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109261504?v=4"},"body":"different between V2:\n\n1.change the commit massage in t4113: Modernize test script and t4113: indent with space.\n\n2. Put the executable lines inside the test_expect_success.Mention the new style problem cause \nby this change,which is change the \"<<\" to \"<<-\" to strip leading TABs used for indentation.\n\nShuqi Liang (3):\n t/t4113-apply-ending.sh: Modernize test script\n t4113: put executable lines to test_expect_success\n t4113: indent with space\n\n t/t4113-apply-ending.sh | 79 ++++++++++++++++++++---------------------\n 1 file changed, 38 insertions(+), 41 deletions(-)\n\nRange-diff against v2:\n1: d7d52f1f79 ! 1: 3d40bcce13 t/t4113-apply-ending.sh: Modernize a test script\n @@ Metadata\n Author: Shuqi Liang \n \n ## Commit message ##\n - t/t4113-apply-ending.sh: Modernize a test script\n + t/t4113-apply-ending.sh: Modernize test script\n \n + Test scripts in file in this script are written in old style,\n + where the test_expect_success command and test title are written on\n + separate lines. Change the old style to modern style.\n +\n + for example :\n + -test_expect_success setup \\\n + - 'git update-index --add file'\n + -\n + +test_expect_success setup '\n + + git update-index --add file\n + +'\n Signed-off-by: Shuqi Liang \n \n ## t/t4113-apply-ending.sh ##\n2: d9e5a54e32 ! 2: 5c55b208a8 Test scripts in file t4113-apply-ending.sh, files \"test-patch\" and \"file\" are created twice.\n @@ Metadata\n Author: Shuqi Liang \n \n ## Commit message ##\n - Test scripts in file t4113-apply-ending.sh, files\n - \"test-patch\" and \"file\" are created twice.\n + t4113: put executable lines to test_expect_success\n \n - Make the second creation of the files its own step\n - 'setup for apply at the beginning'.\n + This script is written in old style,where there are\n + some executable lines outside test_expect_success. Put the executable\n + lines inside the test_expect_success.\n +\n + As t/README says,use \"<<-\" instead of \"<<\"\n + to strip leading TABs used for indentation. change the \"<<\" to \"<<-\"\n +\n + for example:\n + -cat >test-patch <<\\EOF\n + -diff a/file b/file\n +\n + test_expect_success 'apply at the beginning' '\n + + cat >test-patch <<-\\EOF\n + + diff a/file b/file\n + + --- a/file\n \n Signed-off-by: Shuqi Liang \n \n @@ t/t4113-apply-ending.sh: test_description='git apply trying to add an ending lin\n \n -# setup\n -\n - cat >test-patch <<\\EOF\n - diff --git a/file b/file\n - --- a/file\n -@@ t/t4113-apply-ending.sh: echo 'c' >>file\n +-cat >test-patch <<\\EOF\n +-diff --git a/file b/file\n +---- a/file\n +-+++ b/file\n +-@@ -1,2 +1,3 @@\n +- a\n +- b\n +-+c\n +-EOF\n +-\n +-echo 'a' >file\n +-echo 'b' >>file\n +-echo 'c' >>file\n +-\n test_expect_success setup '\n ++\tcat >test-patch <<-\\EOF\n ++\tdiff --git a/file b/file\n ++\t--- a/file\n ++\t+++ b/file\n ++\t@@ -1,2 +1,3 @@\n ++\ta\n ++\tb\n ++\t+c\n ++\tEOF\n ++\n ++\techo 'a' >file\n ++\techo 'b' >>file\n ++\techo 'c' >>file\n git update-index --add file\n '\n -# test\n @@ -1,2 +1,3 @@\n -b\n -c'\n -git update-index file\n -+test_expect_success 'setup for apply at the beginning' '\n -+\tcat >test-patch <<\\EOF\n +-\n + test_expect_success 'apply at the beginning' '\n ++\tcat >test-patch <<-\\EOF\n +\tdiff a/file b/file\n +\t--- a/file\n +\t+++ b/file\n @@ -1,2 +1,3 @@\n +\tb\n +\tc'\n +\tgit update-index file\n -+'\n - \n - test_expect_success 'apply at the beginning' '\n test_must_fail git apply --index test-patch\n + '\n + \n3: 01a5c3285e < -: ---------- use \"<<-\" instead of \"<<\"\n4: cf2b2ca5a0 < -: ---------- t4113-apply-ending.sh used 4-column indent with space,fix it in use tabs for indentation.\n-: ---------- > 3: 02b661279f t4113: indent with space\n-- \n2.39.0\n\n"},{"id":"471524","messageId":"20230205145245.11078-2-cheskaqiqi@gmail.com","threadId":"59169","inReplyTo":"20230205145245.11078-1-cheskaqiqi@gmail.com","subject":"[PATCH v3 1/3]t4113: modernize a test script","fromName":"Shuqi Liang","fromEmail":"cheskaqiqi@gmail.com","sentAt":"2023-02-05T14:52:43Z","receivedAt":"2023-02-05T14:53:30Z","isPatch":true,"sender":{"key":"cheskaqiqi@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109261504?v=4"},"body":"Test scripts in file in this script are written in old style,\nwhere the test_expect_success command and test title are written on\nseparate lines. Change the old style to modern style.\n\nfor example :\n-test_expect_success setup \\\n- 'git update-index --add file'\n-\n+test_expect_success setup '\n+ git update-index --add file\n+'\nSigned-off-by: Shuqi Liang \n---\n t/t4113-apply-ending.sh | 16 +++++++++-------\n 1 file changed, 9 insertions(+), 7 deletions(-)\n\ndiff --git a/t/t4113-apply-ending.sh b/t/t4113-apply-ending.sh\nindex 66fa51591e..aa57895b22 100755\n--- a/t/t4113-apply-ending.sh\n+++ b/t/t4113-apply-ending.sh\n@@ -24,13 +24,14 @@ echo 'a' >file\n echo 'b' >>file\n echo 'c' >>file\n \n-test_expect_success setup \\\n- 'git update-index --add file'\n-\n+test_expect_success setup '\n+ git update-index --add file\n+'\n # test\n \n-test_expect_success 'apply at the end' \\\n- 'test_must_fail git apply --index test-patch'\n+test_expect_success 'apply at the end' '\n+ test_must_fail git apply --index test-patch\n+'\n \n cat >test-patch <<\\EOF\n diff a/file b/file\n@@ -47,7 +48,8 @@ b\n c'\n git update-index file\n \n-test_expect_success 'apply at the beginning' \\\n-\t'test_must_fail git apply --index test-patch'\n+test_expect_success 'apply at the beginning' '\n+ test_must_fail git apply --index test-patch\n+'\n \n test_done\n-- \n2.39.0\n\n"},{"id":"471525","messageId":"20230205145245.11078-3-cheskaqiqi@gmail.com","threadId":"59169","inReplyTo":"20230205145245.11078-1-cheskaqiqi@gmail.com","subject":"[PATCH v3 2/3] t4113: put executable lines to test_expect_success","fromName":"Shuqi Liang","fromEmail":"cheskaqiqi@gmail.com","sentAt":"2023-02-05T14:52:44Z","receivedAt":"2023-02-05T14:53:37Z","isPatch":true,"sender":{"key":"cheskaqiqi@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109261504?v=4"},"body":"This script is written in old style,where there are\nsome executable lines outside test_expect_success. Put the executable\nlines inside the test_expect_success.\n\nAs t/README says,use \"<<-\" instead of \"<<\"\nto strip leading TABs used for indentation. Change the \"<<\" to \"<<-\"\n\nfor example:\n-cat >test-patch <<\\EOF\n-diff a/file b/file\n\n test_expect_success 'apply at the beginning' '\n+\tcat >test-patch <<-\\EOF\n+\tdiff a/file b/file\n+\t--- a/file\n\nSigned-off-by: Shuqi Liang \n---\n t/t4113-apply-ending.sh | 59 +++++++++++++++++++----------------------\n 1 file changed, 27 insertions(+), 32 deletions(-)\n\ndiff --git a/t/t4113-apply-ending.sh b/t/t4113-apply-ending.sh\nindex aa57895b22..e0a52a12c4 100755\n--- a/t/t4113-apply-ending.sh\n+++ b/t/t4113-apply-ending.sh\n@@ -8,47 +8,42 @@ test_description='git apply trying to add an ending line.\n '\n . ./test-lib.sh\n \n-# setup\n-\n-cat >test-patch <<\\EOF\n-diff --git a/file b/file\n---- a/file\n-+++ b/file\n-@@ -1,2 +1,3 @@\n- a\n- b\n-+c\n-EOF\n-\n-echo 'a' >file\n-echo 'b' >>file\n-echo 'c' >>file\n-\n test_expect_success setup '\n+\tcat >test-patch <<-\\EOF\n+\tdiff --git a/file b/file\n+\t--- a/file\n+\t+++ b/file\n+\t@@ -1,2 +1,3 @@\n+\ta\n+\tb\n+\t+c\n+\tEOF\n+\n+\techo 'a' >file\n+\techo 'b' >>file\n+\techo 'c' >>file\n git update-index --add file\n '\n-# test\n \n test_expect_success 'apply at the end' '\n test_must_fail git apply --index test-patch\n '\n \n-cat >test-patch <<\\EOF\n-diff a/file b/file\n---- a/file\n-+++ b/file\n-@@ -1,2 +1,3 @@\n-+a\n- b\n- c\n-EOF\n-\n-echo >file 'a\n-b\n-c'\n-git update-index file\n-\n test_expect_success 'apply at the beginning' '\n+\tcat >test-patch <<-\\EOF\n+\tdiff a/file b/file\n+\t--- a/file\n+\t+++ b/file\n+\t@@ -1,2 +1,3 @@\n+\t+a\n+\tb\n+\tc\n+\tEOF\n+\n+\techo >file 'a\n+\tb\n+\tc'\n+\tgit update-index file\n test_must_fail git apply --index test-patch\n '\n \n-- \n2.39.0\n\n"},{"id":"471526","messageId":"20230205145245.11078-4-cheskaqiqi@gmail.com","threadId":"59169","inReplyTo":"20230205145245.11078-1-cheskaqiqi@gmail.com","subject":"[PATCH v3 3/3] t4113: indent with space","fromName":"Shuqi Liang","fromEmail":"cheskaqiqi@gmail.com","sentAt":"2023-02-05T14:52:45Z","receivedAt":"2023-02-05T14:53:38Z","isPatch":true,"sender":{"key":"cheskaqiqi@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109261504?v=4"},"body":"As Documentation/CodingGuidelines says, the shell scripts\nare to use tabs for indentation, but this script\nuses 4-column indent with space. Fix it in use tabs for indentation.\n\nSigned-off-by: Shuqi Liang \n---\n t/t4113-apply-ending.sh | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/t/t4113-apply-ending.sh b/t/t4113-apply-ending.sh\nindex e0a52a12c4..ab5ecaab7f 100755\n--- a/t/t4113-apply-ending.sh\n+++ b/t/t4113-apply-ending.sh\n@@ -22,11 +22,11 @@ test_expect_success setup '\n \techo 'a' >file\n \techo 'b' >>file\n \techo 'c' >>file\n- git update-index --add file\n+\tgit update-index --add file\n '\n \n test_expect_success 'apply at the end' '\n- test_must_fail git apply --index test-patch\n+\ttest_must_fail git apply --index test-patch\n '\n \n test_expect_success 'apply at the beginning' '\n@@ -44,7 +44,7 @@ test_expect_success 'apply at the beginning' '\n \tb\n \tc'\n \tgit update-index file\n- test_must_fail git apply --index test-patch\n+\ttest_must_fail git apply --index test-patch\n '\n \n test_done\n-- \n2.39.0\n\n"},{"id":"471540","messageId":"CAPig+cTH_5Bu48fO1Eub25CLZmRCmz4EjYz9O++H12HGTVXU0w@mail.gmail.com","threadId":"59169","inReplyTo":"20230205145245.11078-4-cheskaqiqi@gmail.com","subject":"Re: [PATCH v3 3/3] t4113: indent with space","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2023-02-05T20:29:58Z","receivedAt":"2023-02-05T20:30:16Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Feb 5, 2023 at 9:56 AM Shuqi Liang wrote:\n> t4113: indent with space\n\nThis probably ought to say \"indent with tab\" since that's what this\npatch is doing.\n\n> As Documentation/CodingGuidelines says, the shell scripts\n> are to use tabs for indentation, but this script\n> uses 4-column indent with space. Fix it in use tabs for indentation.\n\ns/in use/to use/\n\n> Signed-off-by: Shuqi Liang \n> ---\n> diff --git a/t/t4113-apply-ending.sh b/t/t4113-apply-ending.sh\n> index e0a52a12c4..ab5ecaab7f 100755\n> --- a/t/t4113-apply-ending.sh\n> +++ b/t/t4113-apply-ending.sh\n> @@ -22,11 +22,11 @@ test_expect_success setup '\n> echo 'a' >file\n> echo 'b' >>file\n> echo 'c' >>file\n> - git update-index --add file\n> + git update-index --add file\n> '\n\nAs a GSoC microproject, v3 is probably \"good enough\", so there may not\nbe a compelling reason to re-roll.\n\nIf you do find a reason to re-roll, though, I might suggest swapping\npatches 2 and 3 since the current organization leaves a mix of tab and\nspace indentation in the tests, which makes reviewers do extra work\nsince they have to look ahead in the patch series to see if you fix\nthe inconsistent indentation in a later patch.\n"},{"id":"471602","messageId":"CAMO4yUFnSQzf9w=gKU6JhaRYbSwhCi_QK1pAQbOuaTa8uEiEog@mail.gmail.com","threadId":"59169","inReplyTo":"CAPig+cTH_5Bu48fO1Eub25CLZmRCmz4EjYz9O++H12HGTVXU0w@mail.gmail.com","subject":"Re: [PATCH v3 3/3] t4113: indent with space","fromName":"Shuqi Liang","fromEmail":"cheskaqiqi@gmail.com","sentAt":"2023-02-06T21:17:15Z","receivedAt":"2023-02-06T21:17:33Z","isPatch":true,"sender":{"key":"cheskaqiqi@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109261504?v=4"},"body":"Hi, Eric\n\nOn Sun, Feb 5, 2023 at 3:30 PM Eric Sunshine wrote:\n\n> This probably ought to say \"indent with tab\" since that's what this\n> patch is doing.\n\nThanks ,I will fix it .\n\n> If you do find a reason to re-roll, though, I might suggest swapping\n> patches 2 and 3 since the current organization leaves a mix of tab and\n> space indentation in the tests, which makes reviewers do extra work\n> since they have to look ahead in the patch series to see if you fix\n> the inconsistent indentation in a later patch.\n\nYeah ,I didn‘t realize that .Thanks for reply! I will send the V4 soon.\n----------\nThanks,\nShuqi\n"},{"id":"471603","messageId":"20230206211823.8651-1-cheskaqiqi@gmail.com","threadId":"59169","inReplyTo":"20230205145245.11078-1-cheskaqiqi@gmail.com","subject":"[PATCH v4 0/3] t4113: modernize style","fromName":"Shuqi Liang","fromEmail":"cheskaqiqi@gmail.com","sentAt":"2023-02-06T21:18:20Z","receivedAt":"2023-02-06T21:18:51Z","isPatch":true,"sender":{"key":"cheskaqiqi@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109261504?v=4"},"body":"Slightly different from V3:\n\n1.Swap patches 2 and 3 because in patch [v3 2/3] leaves a mix of tab and\nspace indentation in the tests\n\n2.Change the commit message in patch [v3 3/3] to indent with tab.\n\nShuqi Liang (3):\n t4113: modernize test script\n t4113: indent with tab\n t4113: put executable lines to test_expect_success\n\n t/t4113-apply-ending.sh | 79 ++++++++++++++++++++---------------------\n 1 file changed, 38 insertions(+), 41 deletions(-)\n\n\nbase-commit: c48035d29b4e524aed3a32f0403676f0d9128863\n-- \n2.39.0\n\n"},{"id":"471604","messageId":"20230206211823.8651-2-cheskaqiqi@gmail.com","threadId":"59169","inReplyTo":"20230206211823.8651-1-cheskaqiqi@gmail.com","subject":"[PATCH v4 1/3] t4113: modernize test script","fromName":"Shuqi Liang","fromEmail":"cheskaqiqi@gmail.com","sentAt":"2023-02-06T21:18:21Z","receivedAt":"2023-02-06T21:18:59Z","isPatch":true,"sender":{"key":"cheskaqiqi@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109261504?v=4"},"body":"Test scripts in file in this script are written in old style,\nwhere the test_expect_success command and test title are written on\nseparate lines. Change the old style to modern style.\n\nfor example :\n-test_expect_success setup \\\n- 'git update-index --add file'\n-\n+test_expect_success setup '\n+ git update-index --add file\n+'\nSigned-off-by: Shuqi Liang \n---\n t/t4113-apply-ending.sh | 16 +++++++++-------\n 1 file changed, 9 insertions(+), 7 deletions(-)\n\ndiff --git a/t/t4113-apply-ending.sh b/t/t4113-apply-ending.sh\nindex 66fa51591e..aa57895b22 100755\n--- a/t/t4113-apply-ending.sh\n+++ b/t/t4113-apply-ending.sh\n@@ -24,13 +24,14 @@ echo 'a' >file\n echo 'b' >>file\n echo 'c' >>file\n \n-test_expect_success setup \\\n- 'git update-index --add file'\n-\n+test_expect_success setup '\n+ git update-index --add file\n+'\n # test\n \n-test_expect_success 'apply at the end' \\\n- 'test_must_fail git apply --index test-patch'\n+test_expect_success 'apply at the end' '\n+ test_must_fail git apply --index test-patch\n+'\n \n cat >test-patch <<\\EOF\n diff a/file b/file\n@@ -47,7 +48,8 @@ b\n c'\n git update-index file\n \n-test_expect_success 'apply at the beginning' \\\n-\t'test_must_fail git apply --index test-patch'\n+test_expect_success 'apply at the beginning' '\n+ test_must_fail git apply --index test-patch\n+'\n \n test_done\n-- \n2.39.0\n\n"},{"id":"471605","messageId":"20230206211823.8651-3-cheskaqiqi@gmail.com","threadId":"59169","inReplyTo":"20230206211823.8651-1-cheskaqiqi@gmail.com","subject":"[PATCH v4 2/3] t4113: indent with tab","fromName":"Shuqi Liang","fromEmail":"cheskaqiqi@gmail.com","sentAt":"2023-02-06T21:18:22Z","receivedAt":"2023-02-06T21:19:02Z","isPatch":true,"sender":{"key":"cheskaqiqi@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109261504?v=4"},"body":"As Documentation/CodingGuidelines says, the shell scripts\nare to use tabs for indentation, but this script\nuses 4-column indent with space. Fix it in use tabs for indentation.\n\nSigned-off-by: Shuqi Liang \n---\n t/t4113-apply-ending.sh | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/t/t4113-apply-ending.sh b/t/t4113-apply-ending.sh\nindex aa57895b22..5ee177e8eb 100755\n--- a/t/t4113-apply-ending.sh\n+++ b/t/t4113-apply-ending.sh\n@@ -25,12 +25,12 @@ echo 'b' >>file\n echo 'c' >>file\n \n test_expect_success setup '\n- git update-index --add file\n+\tgit update-index --add file\n '\n # test\n \n test_expect_success 'apply at the end' '\n- test_must_fail git apply --index test-patch\n+\ttest_must_fail git apply --index test-patch\n '\n \n cat >test-patch <<\\EOF\n@@ -49,7 +49,7 @@ c'\n git update-index file\n \n test_expect_success 'apply at the beginning' '\n- test_must_fail git apply --index test-patch\n+\ttest_must_fail git apply --index test-patch\n '\n \n test_done\n-- \n2.39.0\n\n"},{"id":"471606","messageId":"20230206211823.8651-4-cheskaqiqi@gmail.com","threadId":"59169","inReplyTo":"20230206211823.8651-1-cheskaqiqi@gmail.com","subject":"[PATCH v4 3/3] t4113: put executable lines to test_expect_success","fromName":"Shuqi Liang","fromEmail":"cheskaqiqi@gmail.com","sentAt":"2023-02-06T21:18:23Z","receivedAt":"2023-02-06T21:19:06Z","isPatch":true,"sender":{"key":"cheskaqiqi@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109261504?v=4"},"body":"This script is written in old style,where there are\nsome executable lines outside test_expect_success. Put the executable\nlines inside the test_expect_success.\n\nAs t/README says,use \"<<-\" instead of \"<<\"\nto strip leading TABs used for indentation. Change the \"<<\" to \"<<-\"\n\nfor example:\n-cat >test-patch <<\\EOF\n-diff a/file b/file\n\n test_expect_success 'apply at the beginning' '\n+ cat >test-patch <<-\\EOF\n+ diff a/file b/file\n+ --- a/file\n\nSigned-off-by: Shuqi Liang \n---\n t/t4113-apply-ending.sh | 59 +++++++++++++++++++----------------------\n 1 file changed, 27 insertions(+), 32 deletions(-)\n\ndiff --git a/t/t4113-apply-ending.sh b/t/t4113-apply-ending.sh\nindex 5ee177e8eb..ab5ecaab7f 100755\n--- a/t/t4113-apply-ending.sh\n+++ b/t/t4113-apply-ending.sh\n@@ -8,47 +8,42 @@ test_description='git apply trying to add an ending line.\n '\n . ./test-lib.sh\n \n-# setup\n-\n-cat >test-patch <<\\EOF\n-diff --git a/file b/file\n---- a/file\n-+++ b/file\n-@@ -1,2 +1,3 @@\n- a\n- b\n-+c\n-EOF\n-\n-echo 'a' >file\n-echo 'b' >>file\n-echo 'c' >>file\n-\n test_expect_success setup '\n+\tcat >test-patch <<-\\EOF\n+\tdiff --git a/file b/file\n+\t--- a/file\n+\t+++ b/file\n+\t@@ -1,2 +1,3 @@\n+\ta\n+\tb\n+\t+c\n+\tEOF\n+\n+\techo 'a' >file\n+\techo 'b' >>file\n+\techo 'c' >>file\n \tgit update-index --add file\n '\n-# test\n \n test_expect_success 'apply at the end' '\n \ttest_must_fail git apply --index test-patch\n '\n \n-cat >test-patch <<\\EOF\n-diff a/file b/file\n---- a/file\n-+++ b/file\n-@@ -1,2 +1,3 @@\n-+a\n- b\n- c\n-EOF\n-\n-echo >file 'a\n-b\n-c'\n-git update-index file\n-\n test_expect_success 'apply at the beginning' '\n+\tcat >test-patch <<-\\EOF\n+\tdiff a/file b/file\n+\t--- a/file\n+\t+++ b/file\n+\t@@ -1,2 +1,3 @@\n+\t+a\n+\tb\n+\tc\n+\tEOF\n+\n+\techo >file 'a\n+\tb\n+\tc'\n+\tgit update-index file\n \ttest_must_fail git apply --index test-patch\n '\n \n-- \n2.39.0\n\n"},{"id":"471612","messageId":"230206.86ttzy4g3c.gmgdl@evledraar.gmail.com","threadId":"59169","inReplyTo":"20230206211823.8651-4-cheskaqiqi@gmail.com","subject":"Re: [PATCH v4 3/3] t4113: put executable lines to test_expect_success","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2023-02-06T21:50:05Z","receivedAt":"2023-02-06T21:52:13Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Mon, Feb 06 2023, Shuqi Liang wrote:\n\n. ./test-lib.sh\n> \n> -# setup\n> -\n> -cat >test-patch <<\\EOF\n> -diff --git a/file b/file\n> ---- a/file\n> -+++ b/file\n> -@@ -1,2 +1,3 @@\n> - a\n> - b\n> -+c\n> -EOF\n> -\n> -echo 'a' >file\n> -echo 'b' >>file\n> -echo 'c' >>file\n> -\n> test_expect_success setup '\n> +\tcat >test-patch <<-\\EOF\n> +\tdiff --git a/file b/file\n> +\t--- a/file\n> +\t+++ b/file\n> +\t@@ -1,2 +1,3 @@\n> +\ta\n> +\tb\n> +\t+c\n> +\tEOF\n> +\n> +\techo 'a' >file\n> +\techo 'b' >>file\n> +\techo 'c' >>file\n\nI have not read the rest here, but this immediately fails with a very\nlarge error from chain-lint by default, and even if you manually disable\nit (which I assume you're doing, or just not testing these at all before\nsubmission), you'll get:\n\t\n\t$ ./t4113-apply-ending.sh --no-chain-lint\n\tok 1 - setup\n\tok 2 - apply at the end\n\tok 3 - apply at the beginning\n\t./t4113-apply-ending.sh: 44: b: not found\n\t./t4113-apply-ending.sh: 48: c\n\t git update-index file\n\t test_must_fail git apply --index test-patch\n\t: not found\n\t# passed all 3 test(s)\n\t1..3\n\nWhich shows that even with the &&-chaining fixed you have quoting issues\nhere, you're trying to execute 'b' etc.\n\nI didn't read the rest of this topic, but please test with chain-lint,\nsee if there's any unexpected new output from the tests etc. before a v5\nre-roll.\n"},{"id":"471622","messageId":"xmqqlela2z3p.fsf@gitster.g","threadId":"59169","inReplyTo":"20230206211823.8651-4-cheskaqiqi@gmail.com","subject":"Re: [PATCH v4 3/3] t4113: put executable lines to test_expect_success","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-02-06T22:44:26Z","receivedAt":"2023-02-06T22:44:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Shuqi Liang writes:\n\n> -cat >test-patch <<\\EOF\n> -diff --git a/file b/file\n> ---- a/file\n> -+++ b/file\n> -@@ -1,2 +1,3 @@\n> - a\n> - b\n> -+c\n> -EOF\n> -\n> -echo 'a' >file\n> -echo 'b' >>file\n> -echo 'c' >>file\n\nHave you run the resulting test?\n\n> test_expect_success setup '\n> +\tcat >test-patch <<-\\EOF\n> +\tdiff --git a/file b/file\n> +\t--- a/file\n> +\t+++ b/file\n> +\t@@ -1,2 +1,3 @@\n> +\ta\n> +\tb\n> +\t+c\n> +\tEOF\n\nThis creates a \"test-patch\" file with lines 'a' and 'b' that are\ncommon context lines without any whitespace before them, no? The\noriginal left the necessary single space in front of them (see the\nline removed above).\n"},{"id":"471664","messageId":"CAMO4yUGmQ371hLCSTODQct+CzY2mqywfLzZO6fsgqN2=1cWGrw@mail.gmail.com","threadId":"59169","inReplyTo":"xmqqlela2z3p.fsf@gitster.g","subject":"Re: [PATCH v4 3/3] t4113: put executable lines to test_expect_success","fromName":"Shuqi Liang","fromEmail":"cheskaqiqi@gmail.com","sentAt":"2023-02-06T23:42:33Z","receivedAt":"2023-02-06T23:42:57Z","isPatch":true,"sender":{"key":"cheskaqiqi@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109261504?v=4"},"body":"On Mon, Feb 6, 2023 at 5:44 PM Junio C Hamano wrote:\n\n> Have you run the resulting test?\n\nMy apologies for not testing after V1. That was a major oversight on\nmy part. I'll make sure to thoroughly test before each submission to\navoid any issues with the code in the future.\n\n\n> This creates a \"test-patch\" file with lines 'a' and 'b' that are\n> common context lines without any whitespace before them, no? The\n> original left the necessary single space in front of them (see the\n> line removed above).\n\nI try to change the code to(left the necessary single space in front\nof 'a' and 'b':\n\ndiff --git a/t/t4113-apply-ending.sh b/t/t4113-apply-ending.sh\nindex ab5ecaab7f..ef61a3187c 100755\n--- a/t/t4113-apply-ending.sh\n+++ b/t/t4113-apply-ending.sh\n@@ -14,8 +14,8 @@ test_expect_success setup '\n--- a/file\n+++ b/file\n@@ -1,2 +1,3 @@\n- a\n- b\n+ a\n+ b\n+c\nEOF\n\nHere I only show one part ,but I fix two same issue in the V4 patch\nand it still can not pass the test .\nIt say :\n\nTest Summary Report\n\n-------------------\n\nt4113-apply-ending.sh (Wstat: 256 Tests: 0 Failed: 0)\n\n Non-zero exit status: 1\n\n Parse errors: No plan found in TAP output\n\nFiles=1, Tests=0, 0 wallclock secs ( 0.01 usr 0.01 sys + 0.05 cusr\n0.02 csys = 0.09 CPU)\n\nResult: FAIL.\n\nI'm stumped as to why it's still failing. I've tried searching for\nanswers on StackOverflow, but I still can't figure it out.\n----------------\nThanks,\nShuqi\n"},{"id":"471670","messageId":"230207.86h6vx51x3.gmgdl@evledraar.gmail.com","threadId":"59169","inReplyTo":"CAMO4yUGmQ371hLCSTODQct+CzY2mqywfLzZO6fsgqN2=1cWGrw@mail.gmail.com","subject":"Re: [PATCH v4 3/3] t4113: put executable lines to test_expect_success","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2023-02-07T08:05:34Z","receivedAt":"2023-02-07T08:13:02Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Mon, Feb 06 2023, Shuqi Liang wrote:\n\n> On Mon, Feb 6, 2023 at 5:44 PM Junio C Hamano wrote:\n>\n>> Have you run the resulting test?\n>\n> My apologies for not testing after V1. That was a major oversight on\n> my part. I'll make sure to thoroughly test before each submission to\n> avoid any issues with the code in the future.\n>\n>\n>> This creates a \"test-patch\" file with lines 'a' and 'b' that are\n>> common context lines without any whitespace before them, no? The\n>> original left the necessary single space in front of them (see the\n>> line removed above).\n>\n> I try to change the code to(left the necessary single space in front\n> of 'a' and 'b':\n>\n> diff --git a/t/t4113-apply-ending.sh b/t/t4113-apply-ending.sh\n> index ab5ecaab7f..ef61a3187c 100755\n> --- a/t/t4113-apply-ending.sh\n> +++ b/t/t4113-apply-ending.sh\n> @@ -14,8 +14,8 @@ test_expect_success setup '\n> --- a/file\n> +++ b/file\n> @@ -1,2 +1,3 @@\n> - a\n> - b\n> + a\n> + b\n> +c\n> EOF\n>\n> Here I only show one part ,but I fix two same issue in the V4 patch\n> and it still can not pass the test .\n> It say :\n>\n> Test Summary Report\n>\n> -------------------\n>\n> t4113-apply-ending.sh (Wstat: 256 Tests: 0 Failed: 0)\n>\n> Non-zero exit status: 1\n>\n> Parse errors: No plan found in TAP output\n>\n> Files=1, Tests=0, 0 wallclock secs ( 0.01 usr 0.01 sys + 0.05 cusr\n> 0.02 csys = 0.09 CPU)\n>\n> Result: FAIL.\n>\n> I'm stumped as to why it's still failing. I've tried searching for\n> answers on StackOverflow, but I still can't figure it out.\n> ----------------\n> Thanks,\n> Shuqi\n\nThe error doesn't tell us much, instead of \"make prove\" or \"prove\n\" running e.g.:\n\n\t./t4113-apply-ending.sh -vixd\n\nGives you better output.\n\nBut this is almost certainly that you're trying to insert leading\nwhitespace into a line that's in a <<-EOF here-doc, the \"-\" part of that\nmeans that your leading whitespace is being stripped.\n\nA typical idiom for that is have a marker for the start of line, and\nstrip the whitespace with \"sed\". See this for existing examples:\n\n\tgit grep 'sed.*\\^.*>.*EOF'\n"},{"id":"471718","messageId":"CAMO4yUFMTSMuyDjiM_Ssa7X95Cm2x_uUR73Uf1UJoaPxqvoELg@mail.gmail.com","threadId":"59169","inReplyTo":"230207.86h6vx51x3.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH v4 3/3] t4113: put executable lines to test_expect_success","fromName":"Shuqi Liang","fromEmail":"cheskaqiqi@gmail.com","sentAt":"2023-02-07T19:55:05Z","receivedAt":"2023-02-07T19:55:29Z","isPatch":true,"sender":{"key":"cheskaqiqi@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109261504?v=4"},"body":"Hi Ævar,\n\nOn Tue, Feb 7, 2023 at 3:12 AM Ævar Arnfjörð Bjarmason wrote:\n\n\n> The error doesn't tell us much, instead of \"make prove\" or \"prove\n> \" running e.g.:\n>\n> ./t4113-apply-ending.sh -vixd\n>\n> Gives you better output.\n\nThanks, this is really helpful.\n\n> But this is almost certainly that you're trying to insert leading\n> whitespace into a line that's in a <<-EOF here-doc, the \"-\" part of that\n> means that your leading whitespace is being stripped.\n>\n> A typical idiom for that is have a marker for the start of line, and\n> strip the whitespace with \"sed\". See this for existing examples:\n>\n> git grep 'sed.*\\^.*>.*EOF'\n\nThank you for the tip! I'll try to fix the problem as soon as possible.\n\n---------\nThanks,\nShuqi\n"},{"id":"471753","messageId":"CAMO4yUEcweeGZDCBm-y51TQ0fgze_geKr95RXBpEEpH4AyYJuQ@mail.gmail.com","threadId":"59169","inReplyTo":"230207.86h6vx51x3.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH v4 3/3] t4113: put executable lines to test_expect_success","fromName":"Shuqi Liang","fromEmail":"cheskaqiqi@gmail.com","sentAt":"2023-02-08T05:44:08Z","receivedAt":"2023-02-08T05:44:26Z","isPatch":true,"sender":{"key":"cheskaqiqi@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109261504?v=4"},"body":"Hi Ævar,\n\nOn Tue, Feb 7, 2023 at 3:12 AM Ævar Arnfjörð Bjarmason wrote:\n\n> But this is almost certainly that you're trying to insert leading\n> whitespace into a line that's in a <<-EOF here-doc, the \"-\" part of that\n> means that your leading whitespace is being stripped.\n>\n> A typical idiom for that is have a marker for the start of line, and\n> strip the whitespace with \"sed\". See this for existing examples:\n>\n> git grep 'sed.*\\^.*>.*EOF'\n\n\nI try to use Z as the marker in front of 'a' and 'b' and use sed -e\n\"s/Z/ /g\" in order to replace Z with white space but it still can not\npass the test.\n\nThen I realize even if I don't add tab in front of the line but with\nspace in front of 'a' and 'b' like the original test script. It still\nsays it can't read \"b\" and \"c” ：\n\ntest_expect_success 'apply at the beginning' '\ncat >test-patch<<\\EOF &&\ndiff a/file b/file\n--- a/file\n+++ b/file\n@@ -1,2 +1,3 @@\n+a\n b\n c\nEOF\n\necho >file 'a\nb\nc'&&\ngit update-index file&&\ntest_must_fail git apply --index test-patch\n'\nMaybe the error is not caused by whitespace?\n\nThen I try to change:\n\necho >file 'a\nb\nc'\n\nTo:\necho >file \"a\nb\nc\"\n\nThen everything passes the test. I think double quotes allow for\nvariable substitution and command substitution, while single quotes\npreserve the literal value of all characters within the quotes. In\nthis case, the string contains no variables or commands, so either\ntype of quote would work. Is there something wrong with my idea? Is it\ngood to modify code like that?\n\nLooking forward to your reply!\n\n------\nShuqi\n"},{"id":"471755","messageId":"230208.865ycc1tqc.gmgdl@evledraar.gmail.com","threadId":"59169","inReplyTo":"CAMO4yUEcweeGZDCBm-y51TQ0fgze_geKr95RXBpEEpH4AyYJuQ@mail.gmail.com","subject":"Re: [PATCH v4 3/3] t4113: put executable lines to test_expect_success","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2023-02-08T07:44:50Z","receivedAt":"2023-02-08T07:50:26Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Feb 08 2023, Shuqi Liang wrote:\n\n> On Tue, Feb 7, 2023 at 3:12 AM Ævar Arnfjörð Bjarmason wrote:\n>\n>> But this is almost certainly that you're trying to insert leading\n>> whitespace into a line that's in a <<-EOF here-doc, the \"-\" part of that\n>> means that your leading whitespace is being stripped.\n>>\n>> A typical idiom for that is have a marker for the start of line, and\n>> strip the whitespace with \"sed\". See this for existing examples:\n>>\n>> git grep 'sed.*\\^.*>.*EOF'\n>\n>\n> I try to use Z as the marker in front of 'a' and 'b' and use sed -e\n> \"s/Z/ /g\" in order to replace Z with white space but it still can not\n> pass the test.\n>\n> Then I realize even if I don't add tab in front of the line but with\n> space in front of 'a' and 'b' like the original test script. It still\n> says it can't read \"b\" and \"c” ：\n>\n> test_expect_success 'apply at the beginning' '\n> cat >test-patch<<\\EOF &&\n> diff a/file b/file\n> --- a/file\n> +++ b/file\n> @@ -1,2 +1,3 @@\n> +a\n> b\n> c\n> EOF\n>\n> echo >file 'a\n> b\n> c'&&\n> git update-index file&&\n> test_must_fail git apply --index test-patch\n> '\n> Maybe the error is not caused by whitespace?\n>\n> Then I try to change:\n>\n> echo >file 'a\n> b\n> c'\n>\n> To:\n> echo >file \"a\n> b\n> c\"\n>\n> Then everything passes the test. I think double quotes allow for\n> variable substitution and command substitution, while single quotes\n> preserve the literal value of all characters within the quotes. In\n> this case, the string contains no variables or commands, so either\n> type of quote would work. Is there something wrong with my idea? Is it\n> good to modify code like that?\n>\n> Looking forward to your reply!\n\nI'm not sure because you're just posting snippets, if you have problems\nin the future it would be best to post the full diff to \"master\" that\nyou're having issues with, e.g. an RFC per Documentation/SubmittingPatches.\n\nBut I think this is because the test itself is using '-quotes, so you\nneed to use '\\'' if you want to single quote, and \" for double quotes,\nand \\\" if the test were in double quotes.\n\nBut the issues you're having here aren't with Git, but the very basics\nof POSIX shell syntax.\n\nI think it would be good for you to read some basic documentation on\nPOSIX shells, their syntax, common POSIX commands etc. Your local \"man\nsh\" is probably a good place to start, but there's also books, online\ntutorials etc.\n\nIn this case the syntax you're trying to get working is something we\nusually try to avoid in either case, i.e. even if it involves an\nexternal process we usually do:\n\n\tcat >out <<-\\EOF\n\ta\n b\n\tc\n\tEOF\n\nRather than:\n\n\techo \"a\n b\n\tc\" >out\n\nIf you are using \"echo\" I saw another change of yours had e.g.:\n\n\techo x >f &&\n\techo y >>f &&\n\techo z >>f\n\nIt's better to e.g. (assuming use of \"echo\", or other built-ins or\ncommands):\n\n\t{\n\t\techo x &&\n\t\techo y &&\n\t\techo z\n\t} >f\n"},{"id":"471861","messageId":"20230209154417.10763-1-cheskaqiqi@gmail.com","threadId":"59169","inReplyTo":"20230206211823.8651-1-cheskaqiqi@gmail.com","subject":"[PATCH v5 0/3] t4113: modernize style","fromName":"Shuqi Liang","fromEmail":"cheskaqiqi@gmail.com","sentAt":"2023-02-09T15:44:14Z","receivedAt":"2023-02-09T15:45:28Z","isPatch":true,"sender":{"key":"cheskaqiqi@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109261504?v=4"},"body":"different with V4: \n1.chain test assertions\n\nFor example:\n\n-cat >test-patch <<\\EOF\n-diff --git a/file b/file\n\n+ cat >test-patch <<-\\EOF &&\n+ diff --git a/file b/file\n\n\n2.change the modern style with echo \n echo x >file &&\n echo y >>file &&\n echo z >>file\n\n Change it to this stlye :\n {\n echo x &&\n echo y &&\n echo z\n } >file\n\n3.In order to escape for executable lines inside the test_expect_success.\nChange ' in executable lines to '\\'' in order to escape.\n\n\n\n(V4 didn't test before sumbit so I show the difference between orgin t4113 )\n\nShuqi Liang (3):\n t4113: modernize test script\n t4113: indent with tab\n t4113: put executable lines to test_expect_success\n\n t/t4113-apply-ending.sh | 81 ++++++++++++++++++++---------------------\n 1 file changed, 40 insertions(+), 41 deletions(-)\n\n\nbase-commit: c48035d29b4e524aed3a32f0403676f0d9128863\n-- \n2.39.0\n\n"},{"id":"471862","messageId":"20230209154417.10763-2-cheskaqiqi@gmail.com","threadId":"59169","inReplyTo":"20230209154417.10763-1-cheskaqiqi@gmail.com","subject":"[PATCH v5 1/3] t4113: modernize test script","fromName":"Shuqi Liang","fromEmail":"cheskaqiqi@gmail.com","sentAt":"2023-02-09T15:44:15Z","receivedAt":"2023-02-09T15:45:35Z","isPatch":true,"sender":{"key":"cheskaqiqi@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109261504?v=4"},"body":"Test scripts in file in this script are written in old style,\nwhere the test_expect_success command and test title are written on\nseparate lines. Change the old style to modern style.\n\nfor example :\n-test_expect_success setup \\\n- 'git update-index --add file'\n-\n+test_expect_success setup '\n+ git update-index --add file\n+'\n\nSigned-off-by: Shuqi Liang \n---\n t/t4113-apply-ending.sh | 18 +++++++++---------\n 1 file changed, 9 insertions(+), 9 deletions(-)\n\ndiff --git a/t/t4113-apply-ending.sh b/t/t4113-apply-ending.sh\nindex 66fa51591e..41526ca805 100755\n--- a/t/t4113-apply-ending.sh\n+++ b/t/t4113-apply-ending.sh\n@@ -24,14 +24,14 @@ echo 'a' >file\n echo 'b' >>file\n echo 'c' >>file\n \n-test_expect_success setup \\\n- 'git update-index --add file'\n-\n+test_expect_success setup '\n+ git update-index --add file\n+'\n # test\n \n-test_expect_success 'apply at the end' \\\n- 'test_must_fail git apply --index test-patch'\n-\n+test_expect_success 'apply at the end' '\n+ test_must_fail git apply --index test-patch\n+'\n cat >test-patch <<\\EOF\n diff a/file b/file\n --- a/file\n@@ -47,7 +47,7 @@ b\n c'\n git update-index file\n \n-test_expect_success 'apply at the beginning' \\\n-\t'test_must_fail git apply --index test-patch'\n-\n+test_expect_success 'apply at the beginning' '\n+\ttest_must_fail git apply --index test-patch\n+'\n test_done\n-- \n2.39.0\n\n"},{"id":"471863","messageId":"20230209154417.10763-3-cheskaqiqi@gmail.com","threadId":"59169","inReplyTo":"20230209154417.10763-1-cheskaqiqi@gmail.com","subject":"[PATCH v5 2/3] t4113: indent with tab","fromName":"Shuqi Liang","fromEmail":"cheskaqiqi@gmail.com","sentAt":"2023-02-09T15:44:16Z","receivedAt":"2023-02-09T15:45:37Z","isPatch":true,"sender":{"key":"cheskaqiqi@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109261504?v=4"},"body":"As Documentation/CodingGuidelines says, the shell scripts\nare to use tabs for indentation, but this script\nuses 4-column indent with space. Fix it in use tabs for indentation.\n\nSigned-off-by: Shuqi Liang \n---\n t/t4113-apply-ending.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t4113-apply-ending.sh b/t/t4113-apply-ending.sh\nindex 41526ca805..a470c9ce7b 100755\n--- a/t/t4113-apply-ending.sh\n+++ b/t/t4113-apply-ending.sh\n@@ -25,12 +25,12 @@ echo 'b' >>file\n echo 'c' >>file\n \n test_expect_success setup '\n- git update-index --add file\n+\tgit update-index --add file\n '\n # test\n \n test_expect_success 'apply at the end' '\n- test_must_fail git apply --index test-patch\n+\ttest_must_fail git apply --index test-patch\n '\n cat >test-patch <<\\EOF\n diff a/file b/file\n-- \n2.39.0\n\n"},{"id":"471864","messageId":"20230209154417.10763-4-cheskaqiqi@gmail.com","threadId":"59169","inReplyTo":"20230209154417.10763-1-cheskaqiqi@gmail.com","subject":"[PATCH v5 3/3] t4113: put executable lines to test_expect_success","fromName":"Shuqi Liang","fromEmail":"cheskaqiqi@gmail.com","sentAt":"2023-02-09T15:44:17Z","receivedAt":"2023-02-09T15:45:39Z","isPatch":true,"sender":{"key":"cheskaqiqi@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109261504?v=4"},"body":"As t/README says, put all code inside test_expect_success and\nother assertions. This script is written in old style,where there are\nsome executable lines outside test_expect_success. Put the executable\nlines inside the test_expect_success.\n\nAs t/README says,use \"<<-\" instead of \"<<\"\nto strip leading TABs used for indentation. Change the \"<<\" to \"<<-\"\n\nfor example:\n-cat >test-patch <<\\EOF\n-diff a/file b/file\n\n test_expect_success 'apply at the beginning' '\n+ cat >test-patch <<-\\EOF\n+ diff a/file b/file\n+ --- a/file\n\nAs t/README says,chain test assertions.Chain this test assertions\nwith &&.\n\nFor example:\n\n-cat >test-patch <<\\EOF\n-diff --git a/file b/file\n\n+ cat >test-patch <<-\\EOF &&\n+ diff --git a/file b/file\n\nThis script is written in old style,where there are something like\n\n echo x >file &&\n echo y >>file &&\n echo z >>file\n\n Change it to this stlye :\n {\n echo x &&\n echo y &&\n echo z\n } >file\n\nIn order to escape for executable lines inside the test_expect_success.\nChange ' in executable lines to '\\'' in order to escape.\n\nSigned-off-by: Shuqi Liang \n---\n t/t4113-apply-ending.sh | 61 ++++++++++++++++++++---------------------\n 1 file changed, 30 insertions(+), 31 deletions(-)\n\ndiff --git a/t/t4113-apply-ending.sh b/t/t4113-apply-ending.sh\nindex a470c9ce7b..c70429bd07 100755\n--- a/t/t4113-apply-ending.sh\n+++ b/t/t4113-apply-ending.sh\n@@ -8,46 +8,45 @@ test_description='git apply trying to add an ending line.\n '\n . ./test-lib.sh\n \n-# setup\n-\n-cat >test-patch <<\\EOF\n-diff --git a/file b/file\n---- a/file\n-+++ b/file\n-@@ -1,2 +1,3 @@\n- a\n- b\n-+c\n-EOF\n-\n-echo 'a' >file\n-echo 'b' >>file\n-echo 'c' >>file\n-\n test_expect_success setup '\n+\tcat >test-patch <<-\\EOF &&\n+\tdiff --git a/file b/file\n+\t--- a/file\n+\t+++ b/file\n+\t@@ -1,2 +1,3 @@\n+\t a\n+\t b\n+\t+c\n+\tEOF\n+\n+\t{\n+\techo '\\''a'\\'' &&\n+\techo '\\''b'\\'' &&\n+\techo '\\''c'\\''\n+\t} >file &&\n \tgit update-index --add file\n '\n-# test\n \n test_expect_success 'apply at the end' '\n \ttest_must_fail git apply --index test-patch\n '\n-cat >test-patch <<\\EOF\n-diff a/file b/file\n---- a/file\n-+++ b/file\n-@@ -1,2 +1,3 @@\n-+a\n- b\n- c\n-EOF\n-\n-echo >file 'a\n-b\n-c'\n-git update-index file\n \n test_expect_success 'apply at the beginning' '\n+\tcat >test-patch <<-\\EOF &&\n+\tdiff a/file b/file\n+\t--- a/file\n+\t+++ b/file\n+\t@@ -1,2 +1,3 @@\n+\t+a\n+\t b\n+\t c\n+\tEOF\n+\n+\techo >file '\\''a\n+\tb\n+\tc'\\'' &&\n+\tgit update-index file &&\n \ttest_must_fail git apply --index test-patch\n '\n+\n test_done\n-- \n2.39.0\n\n"},{"id":"471922","messageId":"CAMO4yUHZ=aUa5pOCKH4F91YhUSgCRuHNXBBOyTF5tzNY=oHsXw@mail.gmail.com","threadId":"59169","inReplyTo":"230208.865ycc1tqc.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH v4 3/3] t4113: put executable lines to test_expect_success","fromName":"Shuqi Liang","fromEmail":"cheskaqiqi@gmail.com","sentAt":"2023-02-10T15:29:45Z","receivedAt":"2023-02-10T15:30:03Z","isPatch":true,"sender":{"key":"cheskaqiqi@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109261504?v=4"},"body":"Hi Ævar ,\n\nSorry if I sent this email twice. I forget to CC git@vger.kernel.org.\n\nOn Wed, Feb 8, 2023 at 2:50 AM Ævar Arnfjörð Bjarmason wrote:\n\n> I'm not sure because you're just posting snippets, if you have problems\n> in the future it would be best to post the full diff to \"master\" that\n> you're having issues with, e.g. an RFC per Documentation/SubmittingPatches.\n\nYeah, posting snippets doesn't describe the problem very well. Thanks\nfor the advice.\n\n\n\n> But I think this is because the test itself is using '-quotes, so you\n> need to use '\\'' if you want to single quote, and \" for double quotes,\n> and \\\" if the test were in double quotes.\n\n\n> But the issues you're having here aren't with Git, but the very basics\n> of POSIX shell syntax.\n\n> I think it would be good for you to read some basic documentation on\n> POSIX shells, their syntax, common POSIX commands etc. Your local \"man\n> sh\" is probably a good place to start, but there's also books, online\n> tutorials etc.\n\nThanks, will do . I'll try to avoid making mistakes on POSIX shells\nquestions and really learn the basic POSIX shells syntax.\n\n> In this case the syntax you're trying to get working is something we\n> usually try to avoid in either case, i.e. even if it involves an\n> external process we usually do:\n\n> cat >out <<-\\EOF\n> a\n> b\n> c\n> EOF\n>\n> Rather than:\n>\n> echo \"a\n> b\n> c\" >out\n>\n> If you are using \"echo\" I saw another change of yours had e.g.:\n>\n> echo x >f &&\n> echo y >>f &&\n> echo z >>f\n>\n> It's better to e.g. (assuming use of \"echo\", or other built-ins or\n> commands):\n>\n> {\n> echo x &&\n> echo y &&\n> echo z\n> } >f\n\nSeen I pass the test s, I'v a submit V5 patch instead of RFC, Would\nyou mind taking a look at this for me? Looking forward to reply.\n\n-----------------------------\nThanks\n\nShuqi\n\nOn Wed, Feb 8, 2023 at 2:50 AM Ævar Arnfjörð Bjarmason wrote:\n>\n>\n> On Wed, Feb 08 2023, Shuqi Liang wrote:\n>\n> > On Tue, Feb 7, 2023 at 3:12 AM Ævar Arnfjörð Bjarmason wrote:\n> >\n> >> But this is almost certainly that you're trying to insert leading\n> >> whitespace into a line that's in a <<-EOF here-doc, the \"-\" part of that\n> >> means that your leading whitespace is being stripped.\n> >>\n> >> A typical idiom for that is have a marker for the start of line, and\n> >> strip the whitespace with \"sed\". See this for existing examples:\n> >>\n> >> git grep 'sed.*\\^.*>.*EOF'\n> >\n> >\n> > I try to use Z as the marker in front of 'a' and 'b' and use sed -e\n> > \"s/Z/ /g\" in order to replace Z with white space but it still can not\n> > pass the test.\n> >\n> > Then I realize even if I don't add tab in front of the line but with\n> > space in front of 'a' and 'b' like the original test script. It still\n> > says it can't read \"b\" and \"c” ：\n> >\n> > test_expect_success 'apply at the beginning' '\n> > cat >test-patch<<\\EOF &&\n> > diff a/file b/file\n> > --- a/file\n> > +++ b/file\n> > @@ -1,2 +1,3 @@\n> > +a\n> > b\n> > c\n> > EOF\n> >\n> > echo >file 'a\n> > b\n> > c'&&\n> > git update-index file&&\n> > test_must_fail git apply --index test-patch\n> > '\n> > Maybe the error is not caused by whitespace?\n> >\n> > Then I try to change:\n> >\n> > echo >file 'a\n> > b\n> > c'\n> >\n> > To:\n> > echo >file \"a\n> > b\n> > c\"\n> >\n> > Then everything passes the test. I think double quotes allow for\n> > variable substitution and command substitution, while single quotes\n> > preserve the literal value of all characters within the quotes. In\n> > this case, the string contains no variables or commands, so either\n> > type of quote would work. Is there something wrong with my idea? Is it\n> > good to modify code like that?\n> >\n> > Looking forward to your reply!\n>\n> I'm not sure because you're just posting snippets, if you have problems\n> in the future it would be best to post the full diff to \"master\" that\n> you're having issues with, e.g. an RFC per Documentation/SubmittingPatches.\n>\n> But I think this is because the test itself is using '-quotes, so you\n> need to use '\\'' if you want to single quote, and \" for double quotes,\n> and \\\" if the test were in double quotes.\n>\n> But the issues you're having here aren't with Git, but the very basics\n> of POSIX shell syntax.\n>\n> I think it would be good for you to read some basic documentation on\n> POSIX shells, their syntax, common POSIX commands etc. Your local \"man\n> sh\" is probably a good place to start, but there's also books, online\n> tutorials etc.\n>\n> In this case the syntax you're trying to get working is something we\n> usually try to avoid in either case, i.e. even if it involves an\n> external process we usually do:\n>\n> cat >out <<-\\EOF\n> a\n> b\n> c\n> EOF\n>\n> Rather than:\n>\n> echo \"a\n> b\n> c\" >out\n>\n> If you are using \"echo\" I saw another change of yours had e.g.:\n>\n> echo x >f &&\n> echo y >>f &&\n> echo z >>f\n>\n> It's better to e.g. (assuming use of \"echo\", or other built-ins or\n> commands):\n>\n> {\n> echo x &&\n> echo y &&\n> echo z\n> } >f\n"},{"id":"472088","messageId":"CAMO4yUHqogcHaQa8=LCFyze=dZTtrPPKdcscQHMvaNRX8w7i2w@mail.gmail.com","threadId":"59169","inReplyTo":"xmqqlela2z3p.fsf@gitster.g","subject":"Re: [PATCH v4 3/3] t4113: put executable lines to test_expect_success","fromName":"Shuqi Liang","fromEmail":"cheskaqiqi@gmail.com","sentAt":"2023-02-14T22:17:36Z","receivedAt":"2023-02-14T22:17:57Z","isPatch":true,"sender":{"key":"cheskaqiqi@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109261504?v=4"},"body":"Hi Junio,\n\nI didn't see this change in \" What's cooking in git.git\". I'm not sure\nif the V5 patch is overlooked. I didn't receive any review after V5.\nIs there anything wrong in V5 that needs to be corrected?\n\nThanks,\nShuqi\n"},{"id":"472091","messageId":"xmqq1qmr51a4.fsf@gitster.g","threadId":"59169","inReplyTo":"CAMO4yUHqogcHaQa8=LCFyze=dZTtrPPKdcscQHMvaNRX8w7i2w@mail.gmail.com","subject":"Re: [PATCH v4 3/3] t4113: put executable lines to test_expect_success","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-02-14T22:29:23Z","receivedAt":"2023-02-14T22:29:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Shuqi Liang writes:\n\n> I didn't see this change in \" What's cooking in git.git\". I'm not sure\n> if the V5 patch is overlooked. I didn't receive any review after V5.\n> Is there anything wrong in V5 that needs to be corrected?\n\nI dunno (yet). These days a day did not have enough time to be\nlooking at all the patches on the list, and patches that are more\nabout practice than fixing real bugs or adding real features tend to\nbe placed on the back burner.\n\nTHanks for pinging.\n\n\n"},{"id":"472101","messageId":"CAPig+cR9pjxnr6PzyTiPULVvKW6Rc+GMeMTGKKp4F8+cWuZdgg@mail.gmail.com","threadId":"59169","inReplyTo":"20230209154417.10763-3-cheskaqiqi@gmail.com","subject":"Re: [PATCH v5 2/3] t4113: indent with tab","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2023-02-15T00:10:51Z","receivedAt":"2023-02-15T00:11:15Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Feb 9, 2023 at 10:50 AM Shuqi Liang wrote:\n> As Documentation/CodingGuidelines says, the shell scripts\n> are to use tabs for indentation, but this script\n> uses 4-column indent with space. Fix it in use tabs for indentation.\n\ns/in use/to use/\n\n> Signed-off-by: Shuqi Liang \n\nThe patch itself looks fine.\n"},{"id":"472102","messageId":"CAMO4yUEAQ9FQu1j4E7hzHtVap25UySOOnvvJyox1Tptr0bK3ZA@mail.gmail.com","threadId":"59169","inReplyTo":"CAPig+cR9pjxnr6PzyTiPULVvKW6Rc+GMeMTGKKp4F8+cWuZdgg@mail.gmail.com","subject":"Re: [PATCH v5 2/3] t4113: indent with tab","fromName":"Shuqi Liang","fromEmail":"cheskaqiqi@gmail.com","sentAt":"2023-02-15T00:18:30Z","receivedAt":"2023-02-15T00:18:50Z","isPatch":true,"sender":{"key":"cheskaqiqi@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109261504?v=4"},"body":"Hi, Eric\n\nOn Tue, Feb 14, 2023 at 7:11 PM Eric Sunshine wrote:\n\n> The patch itself looks fine.\n\nThank you for your recognition!\n\n\n------\nThanks\nShuqi\n"},{"id":"472103","messageId":"CAPig+cTCd4kTM5xVpZv7joiXS+QZfLKVprY_1cpjg9T1_N6Pmw@mail.gmail.com","threadId":"59169","inReplyTo":"230207.86h6vx51x3.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH v4 3/3] t4113: put executable lines to test_expect_success","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2023-02-15T00:26:34Z","receivedAt":"2023-02-15T00:26:50Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Feb 7, 2023 at 3:19 AM Ævar Arnfjörð Bjarmason wrote:\n> On Mon, Feb 06 2023, Shuqi Liang wrote:\n> > On Mon, Feb 6, 2023 at 5:44 PM Junio C Hamano wrote:\n> >> This creates a \"test-patch\" file with lines 'a' and 'b' that are\n> >> common context lines without any whitespace before them, no? The\n> >> original left the necessary single space in front of them (see the\n> >> line removed above).\n> >\n> > I try to change the code to(left the necessary single space in front\n> > of 'a' and 'b':\n> >\n> > @@ -1,2 +1,3 @@\n> > - a\n> > - b\n> > + a\n> > + b\n> > +c\n> > EOF\n> >\n> > t4113-apply-ending.sh (Wstat: 256 Tests: 0 Failed: 0)\n> > Result: FAIL.\n> >\n> > I'm stumped as to why it's still failing. I've tried searching for\n> > answers on StackOverflow, but I still can't figure it out.\n>\n> But this is almost certainly that you're trying to insert leading\n> whitespace into a line that's in a <<-EOF here-doc, the \"-\" part of that\n> means that your leading whitespace is being stripped.\n\nAlmost. The `<<-` operator actually only strips leading TABs; other\nwhitespace following the TABs is left intact.\n"},{"id":"472104","messageId":"CAPig+cTMBpau+G6bJSbYScOT38CU9BCi8RQ554jZYMUd+TheTQ@mail.gmail.com","threadId":"59169","inReplyTo":"230208.865ycc1tqc.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH v4 3/3] t4113: put executable lines to test_expect_success","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2023-02-15T00:34:46Z","receivedAt":"2023-02-15T00:35:02Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, Feb 8, 2023 at 2:57 AM Ævar Arnfjörð Bjarmason wrote:\n> In this case the syntax you're trying to get working is something we\n> usually try to avoid in either case, i.e. even if it involves an\n> external process we usually do:\n>\n> cat >out <<-\\EOF\n> a\n> b\n> c\n> EOF\n>\n> Rather than:\n>\n> echo \"a\n> b\n> c\" >out\n\nThe here-doc (<<-\\EOF) form is definitely a good idea when the code is\npart of an indented test body, whereas the multi-line double-quoted\nstring will be problematic since lines \"b\" and \"c\" will be indented\nwith TAB, which is undesirable here.\n\nEven better for such a simple case would be:\n\n test_write_lines a b c >out &&\n\nIn fact, Junio made this suggestion in the form of a code snipped much\nearlier in this thread.\n\n> If you are using \"echo\" I saw another change of yours had e.g.:\n>\n> echo x >f &&\n> echo y >>f &&\n> echo z >>f\n>\n> It's better to e.g. (assuming use of \"echo\", or other built-ins or\n> commands):\n>\n> {\n> echo x &&\n> echo y &&\n> echo z\n> } >f\n\nThis is also an improvement, though test_write_lines would (again) be\neven better for such a simple case:\n\n test_write_lines x y z >f &&\n"},{"id":"472105","messageId":"CAPig+cTKCMwCQ2TxXyc1KquZmB0QWQAuuag52a1qmeYh6XPXYQ@mail.gmail.com","threadId":"59169","inReplyTo":"20230209154417.10763-4-cheskaqiqi@gmail.com","subject":"Re: [PATCH v5 3/3] t4113: put executable lines to test_expect_success","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2023-02-15T01:09:04Z","receivedAt":"2023-02-15T01:11:24Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Feb 9, 2023 at 11:00 AM Shuqi Liang wrote:\n> As t/README says, put all code inside test_expect_success and\n> other assertions. This script is written in old style,where there are\n> some executable lines outside test_expect_success. Put the executable\n> lines inside the test_expect_success.\n\nAlthough it's true that t/README explains why code should be placed\ninside tests, you can help readers out by simply explaining the reason\nhere in the commit message. For instance, you might replace the above\nparagraph with:\n\n Some old test scripts have setup code outside of tests. This\n is problematic since any failures of the setup code will go\n unnoticed. Therefore, move setup code into the tests themselves\n so that failures are properly flagged.\n\nAs for the rest of the commit message...\n\n> As t/README says,use \"<<-\" instead of \"<<\"\n> to strip leading TABs used for indentation. Change the \"<<\" to \"<<-\"\n>\n> for example:\n> -cat >test-patch <<\\EOF\n> -diff a/file b/file\n>\n> test_expect_success 'apply at the beginning' '\n> + cat >test-patch <<-\\EOF\n> + diff a/file b/file\n> + --- a/file\n\nCertain changes are considered obvious by reviewers, so you don't need\nto mention them explicitly in the commit message. This is one such\nchange. Any reviewer who sees that you indented the here-doc body to\nmatch the indentation of the rest of the test body will understand why\nyou changed `<<` to `<<-` without the commit message having to explain\nit.\n\n> As t/README says,chain test assertions.Chain this test assertions\n> with &&.\n>\n> For example:\n>\n> -cat >test-patch <<\\EOF\n> -diff --git a/file b/file\n>\n> + cat >test-patch <<-\\EOF &&\n> + diff --git a/file b/file\n\nSame thing. Reviewers understand that all code inside a test body must\nhave an intact &&-chain, so you needn't mention this in the commit\nmessage.\n\n> This script is written in old style,where there are something like\n>\n> echo x >file &&\n> echo y >>file &&\n> echo z >>file\n>\n> Change it to this stlye :\n> {\n> echo x &&\n> echo y &&\n> echo z\n> } >file\n\nThis is similar. This is such a simple style change, and the code\nfragment itself is so tiny, that a reviewer can understand this change\nwithout the commit message spelling it out.\n\n> In order to escape for executable lines inside the test_expect_success.\n> Change ' in executable lines to '\\'' in order to escape.\n\nLikewise.\n\nReviewers appreciate well-explained commit messages, but they also\nappreciate succinctness. Although it may not always be obvious how\nmuch to write in a commit message, you can assume that reviewers will\nunderstand obvious changes simply by reading the patch itself, thus\nyou don't need to mention every little detail in the commit message.\nThe important thing to mention in the commit message is the\nexplanation of _why_ the change is being made, plus any changes which\nmight not be obvious. In this case, all the changes are obvious, so,\nreally, you can collapse this entire commit message to just the first\nparagraph.\n\n> Signed-off-by: Shuqi Liang \n> ---\n> diff --git a/t/t4113-apply-ending.sh b/t/t4113-apply-ending.sh\n> @@ -8,46 +8,45 @@ test_description='git apply trying to add an ending line.\n> -# setup\n> -\n\nGood to see that you got rid of the now-unnecessary comment.\n\n> -cat >test-patch <<\\EOF\n> -diff --git a/file b/file\n> ---- a/file\n> -+++ b/file\n> -@@ -1,2 +1,3 @@\n> - a\n> - b\n> -+c\n> -EOF\n> -\n> -echo 'a' >file\n> -echo 'b' >>file\n> -echo 'c' >>file\n> -\n> test_expect_success setup '\n> + cat >test-patch <<-\\EOF &&\n> + diff --git a/file b/file\n> + --- a/file\n> + +++ b/file\n> + @@ -1,2 +1,3 @@\n> + a\n> + b\n> + +c\n> + EOF\n\nOkay.\n\n> + {\n> + echo '\\''a'\\'' &&\n> + echo '\\''b'\\'' &&\n> + echo '\\''c'\\''\n> + } >file &&\n\nA few comments:\n\nThis is unnecessarily confusing. Although this does work, it would be\nsufficient just to change the single-quotes to double-quotes, like\nthis:\n\n {\n echo \"a\" &&\n echo \"b\" &&\n echo \"c\"\n } >file &&\n\nEven simpler, you could drop the quotes altogether for such a simple case:\n\n {\n echo a &&\n echo b &&\n echo c\n } >file &&\n\nHowever, as mentioned elsewhere in this thread, a really succinct way\nto do this, taking advantage of modern style would be to use\ntest_write_lines(), so the five lines collapse to a single line:\n\n test_write_lines a b c >file &&\n\n> -cat >test-patch <<\\EOF\n> -diff a/file b/file\n> ---- a/file\n> -+++ b/file\n> -@@ -1,2 +1,3 @@\n> -+a\n> - b\n> - c\n> -EOF\n> -\n> -echo >file 'a\n> -b\n> -c'\n> -git update-index file\n>\n> test_expect_success 'apply at the beginning' '\n> + cat >test-patch <<-\\EOF &&\n> + diff a/file b/file\n> + --- a/file\n> + +++ b/file\n> + @@ -1,2 +1,3 @@\n> + +a\n> + b\n> + c\n> + EOF\n> +\n> + echo >file '\\''a\n> + b\n> + c'\\'' &&\n\nSame comment about simply using double-quotes instead of\nsingle-quotes, however, this is also another really good place to use\ntest_write_lines:\n\n test_write_lines a b c >file &&\n"},{"id":"472109","messageId":"20230215023900.11854-1-cheskaqiqi@gmail.com","threadId":"59169","inReplyTo":"20230209154417.10763-1-cheskaqiqi@gmail.com","subject":"[PATCH v6 0/3] t4113: modernize style","fromName":"Shuqi Liang","fromEmail":"cheskaqiqi@gmail.com","sentAt":"2023-02-15T02:38:57Z","receivedAt":"2023-02-15T02:39:57Z","isPatch":true,"sender":{"key":"cheskaqiqi@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109261504?v=4"},"body":"different with 5 :\n\n1.change the commit message to be more succinct.\n\n2.\nuse \n\ntest_write_lines a b c >file && \n\ninstead of :\n \n{\n echo a &&\n echo b &&\n echo c\n} >file &&\n\n\n\n\nShuqi Liang (3):\n t4113: modernize test script\n t4113: indent with tab\n t4113: put executable lines to test_expect_success\n\n t/t4113-apply-ending.sh | 75 +++++++++++++++++++----------------------\n 1 file changed, 34 insertions(+), 41 deletions(-)\n\nRange-diff against v5:\n1: 4d55e522a6 ! 1: cb29da5d42 t4113: modernize test script\n @@ Commit message\n where the test_expect_success command and test title are written on\n separate lines. Change the old style to modern style.\n \n - for example :\n - -test_expect_success setup \\\n - - 'git update-index --add file'\n - -\n - +test_expect_success setup '\n - + git update-index --add file\n - +'\n -\n Signed-off-by: Shuqi Liang \n \n ## t/t4113-apply-ending.sh ##\n2: cb997bc422 = 2: 09efa23ba4 t4113: indent with tab\n3: 726d2fcb47 ! 3: 0169cbd402 t4113: put executable lines to test_expect_success\n @@ Commit message\n t4113: put executable lines to test_expect_success\n \n As t/README says, put all code inside test_expect_success and\n - other assertions. This script is written in old style,where there are\n - some executable lines outside test_expect_success. Put the executable\n - lines inside the test_expect_success.\n -\n - As t/README says,use \"<<-\" instead of \"<<\"\n - to strip leading TABs used for indentation. Change the \"<<\" to \"<<-\"\n -\n - for example:\n - -cat >test-patch <<\\EOF\n - -diff a/file b/file\n -\n - test_expect_success 'apply at the beginning' '\n - + cat >test-patch <<-\\EOF\n - + diff a/file b/file\n - + --- a/file\n -\n - As t/README says,chain test assertions.Chain this test assertions\n - with &&.\n -\n - For example:\n -\n - -cat >test-patch <<\\EOF\n - -diff --git a/file b/file\n -\n - + cat >test-patch <<-\\EOF &&\n - + diff --git a/file b/file\n -\n - This script is written in old style,where there are something like\n -\n - echo x >file &&\n - echo y >>file &&\n - echo z >>file\n -\n - Change it to this stlye :\n - {\n - echo x &&\n - echo y &&\n - echo z\n - } >file\n -\n - In order to escape for executable lines inside the test_expect_success.\n - Change ' in executable lines to '\\'' in order to escape.\n + other assertions. This old test scripts have setup code\n + outside of tests. This is problematic since any failures of the\n + setup code will go unnoticed. Therefore, move setup code into the tests\n + themselves so that failures are properly flagged. t/README also says,\n + use \"<<-\" instead of \"<<\" to strip leading TABs used for indentation.\n + Fix it. We should chain test assertions(t/README). Therefore,Chain\n + this test assertions with &&. What's more,take advantage of modern\n + style. Use test_write_lines instead.\n \n Signed-off-by: Shuqi Liang \n \n @@ -1,2 +1,3 @@\n +\t+c\n +\tEOF\n +\n -+\t{\n -+\techo '\\''a'\\'' &&\n -+\techo '\\''b'\\'' &&\n -+\techo '\\''c'\\''\n -+\t} >file &&\n ++\ttest_write_lines a b c >file &&\n \tgit update-index --add file\n '\n -# test\n @@ -1,2 +1,3 @@\n +\t c\n +\tEOF\n +\n -+\techo >file '\\''a\n -+\tb\n -+\tc'\\'' &&\n ++\ttest_write_lines a b c >file &&\n +\tgit update-index file &&\n \ttest_must_fail git apply --index test-patch\n '\n-- \n2.39.0\n\n"},{"id":"472110","messageId":"20230215023953.11880-1-cheskaqiqi@gmail.com","threadId":"59169","inReplyTo":"20230209154417.10763-1-cheskaqiqi@gmail.com","subject":"[PATCH v6 0/3] t4113: modernize style","fromName":"Shuqi Liang","fromEmail":"cheskaqiqi@gmail.com","sentAt":"2023-02-15T02:39:50Z","receivedAt":"2023-02-15T02:40:16Z","isPatch":true,"sender":{"key":"cheskaqiqi@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109261504?v=4"},"body":"different with 5 :\n\n1.change the commit message to be more succinct.\n\n2.\nuse \n\ntest_write_lines a b c >file && \n\ninstead of :\n \n{\n echo a &&\n echo b &&\n echo c\n} >file &&\n\n\n\n\nShuqi Liang (3):\n t4113: modernize test script\n t4113: indent with tab\n t4113: put executable lines to test_expect_success\n\n t/t4113-apply-ending.sh | 75 +++++++++++++++++++----------------------\n 1 file changed, 34 insertions(+), 41 deletions(-)\n\nRange-diff against v5:\n1: 4d55e522a6 ! 1: cb29da5d42 t4113: modernize test script\n @@ Commit message\n where the test_expect_success command and test title are written on\n separate lines. Change the old style to modern style.\n \n - for example :\n - -test_expect_success setup \\\n - - 'git update-index --add file'\n - -\n - +test_expect_success setup '\n - + git update-index --add file\n - +'\n -\n Signed-off-by: Shuqi Liang \n \n ## t/t4113-apply-ending.sh ##\n2: cb997bc422 = 2: 09efa23ba4 t4113: indent with tab\n3: 726d2fcb47 ! 3: 0169cbd402 t4113: put executable lines to test_expect_success\n @@ Commit message\n t4113: put executable lines to test_expect_success\n \n As t/README says, put all code inside test_expect_success and\n - other assertions. This script is written in old style,where there are\n - some executable lines outside test_expect_success. Put the executable\n - lines inside the test_expect_success.\n -\n - As t/README says,use \"<<-\" instead of \"<<\"\n - to strip leading TABs used for indentation. Change the \"<<\" to \"<<-\"\n -\n - for example:\n - -cat >test-patch <<\\EOF\n - -diff a/file b/file\n -\n - test_expect_success 'apply at the beginning' '\n - + cat >test-patch <<-\\EOF\n - + diff a/file b/file\n - + --- a/file\n -\n - As t/README says,chain test assertions.Chain this test assertions\n - with &&.\n -\n - For example:\n -\n - -cat >test-patch <<\\EOF\n - -diff --git a/file b/file\n -\n - + cat >test-patch <<-\\EOF &&\n - + diff --git a/file b/file\n -\n - This script is written in old style,where there are something like\n -\n - echo x >file &&\n - echo y >>file &&\n - echo z >>file\n -\n - Change it to this stlye :\n - {\n - echo x &&\n - echo y &&\n - echo z\n - } >file\n -\n - In order to escape for executable lines inside the test_expect_success.\n - Change ' in executable lines to '\\'' in order to escape.\n + other assertions. This old test scripts have setup code\n + outside of tests. This is problematic since any failures of the\n + setup code will go unnoticed. Therefore, move setup code into the tests\n + themselves so that failures are properly flagged. t/README also says,\n + use \"<<-\" instead of \"<<\" to strip leading TABs used for indentation.\n + Fix it. We should chain test assertions(t/README). Therefore,Chain\n + this test assertions with &&. What's more,take advantage of modern\n + style. Use test_write_lines instead.\n \n Signed-off-by: Shuqi Liang \n \n @@ -1,2 +1,3 @@\n +\t+c\n +\tEOF\n +\n -+\t{\n -+\techo '\\''a'\\'' &&\n -+\techo '\\''b'\\'' &&\n -+\techo '\\''c'\\''\n -+\t} >file &&\n ++\ttest_write_lines a b c >file &&\n \tgit update-index --add file\n '\n -# test\n @@ -1,2 +1,3 @@\n +\t c\n +\tEOF\n +\n -+\techo >file '\\''a\n -+\tb\n -+\tc'\\'' &&\n ++\ttest_write_lines a b c >file &&\n +\tgit update-index file &&\n \ttest_must_fail git apply --index test-patch\n '\n-- \n2.39.0\n\n"},{"id":"472111","messageId":"20230215023953.11880-2-cheskaqiqi@gmail.com","threadId":"59169","inReplyTo":"20230215023953.11880-1-cheskaqiqi@gmail.com","subject":"[PATCH v6 1/3] t4113: modernize test script","fromName":"Shuqi Liang","fromEmail":"cheskaqiqi@gmail.com","sentAt":"2023-02-15T02:39:51Z","receivedAt":"2023-02-15T02:40:36Z","isPatch":true,"sender":{"key":"cheskaqiqi@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109261504?v=4"},"body":"Test scripts in file in this script are written in old style,\nwhere the test_expect_success command and test title are written on\nseparate lines. Change the old style to modern style.\n\nSigned-off-by: Shuqi Liang \n---\n t/t4113-apply-ending.sh | 18 +++++++++---------\n 1 file changed, 9 insertions(+), 9 deletions(-)\n\ndiff --git a/t/t4113-apply-ending.sh b/t/t4113-apply-ending.sh\nindex 66fa51591e..41526ca805 100755\n--- a/t/t4113-apply-ending.sh\n+++ b/t/t4113-apply-ending.sh\n@@ -24,14 +24,14 @@ echo 'a' >file\n echo 'b' >>file\n echo 'c' >>file\n \n-test_expect_success setup \\\n- 'git update-index --add file'\n-\n+test_expect_success setup '\n+ git update-index --add file\n+'\n # test\n \n-test_expect_success 'apply at the end' \\\n- 'test_must_fail git apply --index test-patch'\n-\n+test_expect_success 'apply at the end' '\n+ test_must_fail git apply --index test-patch\n+'\n cat >test-patch <<\\EOF\n diff a/file b/file\n --- a/file\n@@ -47,7 +47,7 @@ b\n c'\n git update-index file\n \n-test_expect_success 'apply at the beginning' \\\n-\t'test_must_fail git apply --index test-patch'\n-\n+test_expect_success 'apply at the beginning' '\n+\ttest_must_fail git apply --index test-patch\n+'\n test_done\n-- \n2.39.0\n\n"},{"id":"472112","messageId":"20230215023953.11880-3-cheskaqiqi@gmail.com","threadId":"59169","inReplyTo":"20230215023953.11880-1-cheskaqiqi@gmail.com","subject":"[PATCH v6 2/3] t4113: indent with tab","fromName":"Shuqi Liang","fromEmail":"cheskaqiqi@gmail.com","sentAt":"2023-02-15T02:39:52Z","receivedAt":"2023-02-15T02:40:47Z","isPatch":true,"sender":{"key":"cheskaqiqi@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109261504?v=4"},"body":"As Documentation/CodingGuidelines says, the shell scripts\nare to use tabs for indentation, but this script\nuses 4-column indent with space. Fix it in use tabs for indentation.\n\nSigned-off-by: Shuqi Liang \n---\n t/t4113-apply-ending.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t4113-apply-ending.sh b/t/t4113-apply-ending.sh\nindex 41526ca805..a470c9ce7b 100755\n--- a/t/t4113-apply-ending.sh\n+++ b/t/t4113-apply-ending.sh\n@@ -25,12 +25,12 @@ echo 'b' >>file\n echo 'c' >>file\n \n test_expect_success setup '\n- git update-index --add file\n+\tgit update-index --add file\n '\n # test\n \n test_expect_success 'apply at the end' '\n- test_must_fail git apply --index test-patch\n+\ttest_must_fail git apply --index test-patch\n '\n cat >test-patch <<\\EOF\n diff a/file b/file\n-- \n2.39.0\n\n"},{"id":"472113","messageId":"CAMO4yUHx68jSe31gKon6_hXTiqufHnHAFkP32DXmWg7dtagAfQ@mail.gmail.com","threadId":"59169","inReplyTo":"CAPig+cTKCMwCQ2TxXyc1KquZmB0QWQAuuag52a1qmeYh6XPXYQ@mail.gmail.com","subject":"Re: [PATCH v5 3/3] t4113: put executable lines to test_expect_success","fromName":"Shuqi Liang","fromEmail":"cheskaqiqi@gmail.com","sentAt":"2023-02-15T02:40:33Z","receivedAt":"2023-02-15T02:40:51Z","isPatch":true,"sender":{"key":"cheskaqiqi@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109261504?v=4"},"body":"On Tue, Feb 14, 2023 at 8:09 PM Eric Sunshine wrote:\n\n> Although it's true that t/README explains why code should be placed\n> inside tests, you can help readers out by simply explaining the reason\n> here in the commit message. For instance, you might replace the above\n> paragraph with:\n>\n> Some old test scripts have setup code outside of tests. This\n> is problematic since any failures of the setup code will go\n> unnoticed. Therefore, move setup code into the tests themselves\n> so that failures are properly flagged.\n>\nThanks, that really makes the change more clear for the readers. I learn a lot.\n\n> Reviewers appreciate well-explained commit messages, but they also\n> appreciate succinctness. Although it may not always be obvious how\n> much to write in a commit message, you can assume that reviewers will\n> understand obvious changes simply by reading the patch itself, thus\n> you don't need to mention every little detail in the commit message.\n> The important thing to mention in the commit message is the\n> explanation of _why_ the change is being made, plus any changes which\n> might not be obvious. In this case, all the changes are obvious, so,\n> really, you can collapse this entire commit message to just the first\n> paragraph.\n\nYeah, the commit looks very wordy. I'll make it more succinct.\n> test_write_lines a b c >file &&\n\n> Same comment about simply using double quotes instead of\n> single-quotes, however, this is also another really good place to use\n> test_write_lines:\n>\n> test_write_lines a b c >file &&\n\nThanks for the tips!\n\n(Sorry for sending the V6 to you twice I send it by accident .)\n\n-------------------------\nThanks\nShuqi\n"},{"id":"472114","messageId":"20230215023953.11880-4-cheskaqiqi@gmail.com","threadId":"59169","inReplyTo":"20230215023953.11880-1-cheskaqiqi@gmail.com","subject":"[PATCH v6 3/3] t4113: put executable lines to test_expect_success","fromName":"Shuqi Liang","fromEmail":"cheskaqiqi@gmail.com","sentAt":"2023-02-15T02:39:53Z","receivedAt":"2023-02-15T02:40:57Z","isPatch":true,"sender":{"key":"cheskaqiqi@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109261504?v=4"},"body":"As t/README says, put all code inside test_expect_success and\nother assertions. This old test scripts have setup code\noutside of tests. This is problematic since any failures of the\nsetup code will go unnoticed. Therefore, move setup code into the tests\nthemselves so that failures are properly flagged. t/README also says,\nuse \"<<-\" instead of \"<<\" to strip leading TABs used for indentation.\nFix it. We should chain test assertions(t/README). Therefore,Chain\nthis test assertions with &&. What's more,take advantage of modern\nstyle. Use test_write_lines instead.\n\nSigned-off-by: Shuqi Liang \n---\n t/t4113-apply-ending.sh | 55 ++++++++++++++++++-----------------------\n 1 file changed, 24 insertions(+), 31 deletions(-)\n\ndiff --git a/t/t4113-apply-ending.sh b/t/t4113-apply-ending.sh\nindex a470c9ce7b..56fc2f436b 100755\n--- a/t/t4113-apply-ending.sh\n+++ b/t/t4113-apply-ending.sh\n@@ -8,46 +8,39 @@ test_description='git apply trying to add an ending line.\n '\n . ./test-lib.sh\n \n-# setup\n-\n-cat >test-patch <<\\EOF\n-diff --git a/file b/file\n---- a/file\n-+++ b/file\n-@@ -1,2 +1,3 @@\n- a\n- b\n-+c\n-EOF\n-\n-echo 'a' >file\n-echo 'b' >>file\n-echo 'c' >>file\n-\n test_expect_success setup '\n+\tcat >test-patch <<-\\EOF &&\n+\tdiff --git a/file b/file\n+\t--- a/file\n+\t+++ b/file\n+\t@@ -1,2 +1,3 @@\n+\t a\n+\t b\n+\t+c\n+\tEOF\n+\n+\ttest_write_lines a b c >file &&\n \tgit update-index --add file\n '\n-# test\n \n test_expect_success 'apply at the end' '\n \ttest_must_fail git apply --index test-patch\n '\n-cat >test-patch <<\\EOF\n-diff a/file b/file\n---- a/file\n-+++ b/file\n-@@ -1,2 +1,3 @@\n-+a\n- b\n- c\n-EOF\n-\n-echo >file 'a\n-b\n-c'\n-git update-index file\n \n test_expect_success 'apply at the beginning' '\n+\tcat >test-patch <<-\\EOF &&\n+\tdiff a/file b/file\n+\t--- a/file\n+\t+++ b/file\n+\t@@ -1,2 +1,3 @@\n+\t+a\n+\t b\n+\t c\n+\tEOF\n+\n+\ttest_write_lines a b c >file &&\n+\tgit update-index file &&\n \ttest_must_fail git apply --index test-patch\n '\n+\n test_done\n-- \n2.39.0\n\n"}]}