{"thread":{"id":"40637","subject":"[PATCH 0/2] Fix interactive rebase when the editor saves with CR/LF","startedAt":"2015-10-25T12:49:50Z","lastAt":"2015-10-28T17:12:00Z","messageCount":31,"participants":["Johannes Schindelin","Philip Oakley","Junio C Hamano","Matthieu Moy"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"272187","messageId":"cover.1445777347.git.johannes.schindelin@gmx.de","threadId":"40637","inReplyTo":null,"subject":"[PATCH 0/2] Fix interactive rebase when the editor saves with CR/LF","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2015-10-25T12:49:50Z","receivedAt":"2015-10-25T12:49:50Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Chad Boles reported that `git rebase -I` recently started producing\nerrors when the editor saves files with DOS line endings. The symptom\nis:\n\n\tWarning: the command isn't recognized in the following line:\n\t -\n\n\tYou can fix this with 'git rebase --edit-todo'.\n\tOr you can abort the rebase with 'git rebase --abort'.\n\nThe real bummer is that simply calling `git rebase --continue` \"fixes\"\nit.\n\nTurns out that we now check whether a single Carriage Return is a valid\ncommand. This new check was introduced recently (1db168ee9, ironically\nnamed \"rebase-i: loosen over-eager check_bad_cmd check\").\n\nThe proposed fix is to teach *all* shell scripts in Git to accept CR as\na field separator. Since LF is already specified as such, it should be\nan uncontentious change.\n\n\nJohannes Schindelin (2):\n  Demonstrate rebase fails when the editor saves with CR/LF\n  sh-setup: explicitly mark CR as a field separator\n\n git-sh-setup.sh               |  2 +-\n t/t3404-rebase-interactive.sh | 12 ++++++++++++\n 2 files changed, 13 insertions(+), 1 deletion(-)\n\n-- \n2.1.4\n"},{"id":"272188","messageId":"90f2eabb6968d9b2fe650d67662ec613a86011cb.1445777347.git.johannes.schindelin@gmx.de","threadId":"40637","inReplyTo":"cover.1445777347.git.johannes.schindelin@gmx.de","subject":"[PATCH 1/2] Demonstrate rebase fails when the editor saves with CR/LF","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2015-10-25T12:50:18Z","receivedAt":"2015-10-25T12:50:18Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Based on a bug report by Chad Boles.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n t/t3404-rebase-interactive.sh | 12 ++++++++++++\n 1 file changed, 12 insertions(+)\n\ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex 3de0b1d..5dfa16a 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -1261,4 +1261,16 @@ test_expect_success 'static check of bad SHA-1' '\n \ttest E = $(git cat-file commit HEAD | sed -ne \\$p)\n '\n \n+test_expect_failure 'editor saves as CR/LF' '\n+\tgit checkout -b with-crlf &&\n+\twrite_script add-crs.sh <<-\\EOF &&\n+\tsed -e \"s/\\$/Q/\" <\"$1\" | tr Q \"\" >\"$1\".new &&\n+\tmv -f \"$1\".new \"$1\"\n+\tEOF\n+\t(\n+\t\ttest_set_editor \"$(pwd)/add-crs.sh\" &&\n+\t\tgit rebase -i HEAD^\n+\t)\n+'\n+\n test_done\n-- \n2.1.4\n"},{"id":"272189","messageId":"214d877df18f3189b1809946817aab9765450ea9.1445777347.git.johannes.schindelin@gmx.de","threadId":"40637","inReplyTo":"cover.1445777347.git.johannes.schindelin@gmx.de","subject":"[PATCH 2/2] sh-setup: explicitly mark CR as a field separator","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2015-10-25T12:50:32Z","receivedAt":"2015-10-25T12:50:32Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"This is the correct thing to do, really: we already specify LF as\nfield separator.\n\nIncidentally, this fixes the problem interactive rebase has when the\neditor wants to save text with CR/LF line endings, as WordPad does\nin Windows 10.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n git-sh-setup.sh               | 2 +-\n t/t3404-rebase-interactive.sh | 2 +-\n 2 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/git-sh-setup.sh b/git-sh-setup.sh\nindex 4691fbc..94dfe04 100644\n--- a/git-sh-setup.sh\n+++ b/git-sh-setup.sh\n@@ -13,7 +13,7 @@ unset CDPATH\n # do not equate an unset IFS with IFS with the default, so here is\n # an explicit SP HT LF.\n IFS=' \t\n-'\n+'\"$(printf '')\"\n \n git_broken_path_fix () {\n \tcase \":$PATH:\" in\ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex 5dfa16a..98eb49a 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -1261,7 +1261,7 @@ test_expect_success 'static check of bad SHA-1' '\n \ttest E = $(git cat-file commit HEAD | sed -ne \\$p)\n '\n \n-test_expect_failure 'editor saves as CR/LF' '\n+test_expect_success 'editor saves as CR/LF' '\n \tgit checkout -b with-crlf &&\n \twrite_script add-crs.sh <<-\\EOF &&\n \tsed -e \"s/\\$/Q/\" <\"$1\" | tr Q \"\" >\"$1\".new &&\n-- \n2.1.4\n"},{"id":"272190","messageId":"36EB0AFDC58D44F3B48BFA044EA15547@PhilipOakley","threadId":"40637","inReplyTo":"214d877df18f3189b1809946817aab9765450ea9.1445777347.git.johannes.schindelin@gmx.de","subject":"Re: [PATCH 2/2] sh-setup: explicitly mark CR as a field separator","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.org","sentAt":null,"receivedAt":"2015-10-25T12:50:32Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"From: \"Johannes Schindelin\" <johannes.schindelin@gmx.de>\n> This is the correct thing to do, really: we already specify LF as\n> field separator.\n>\n> Incidentally, this fixes the problem interactive rebase has when the\n> editor wants to save text with CR/LF line endings, as WordPad does\n> in Windows 10.\n>\n> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n> ---\n> git-sh-setup.sh               | 2 +-\n> t/t3404-rebase-interactive.sh | 2 +-\n> 2 files changed, 2 insertions(+), 2 deletions(-)\n>\n> diff --git a/git-sh-setup.sh b/git-sh-setup.sh\n> index 4691fbc..94dfe04 100644\n> --- a/git-sh-setup.sh\n> +++ b/git-sh-setup.sh\n> @@ -13,7 +13,7 @@ unset CDPATH\n> # do not equate an unset IFS with IFS with the default, so here is\n> # an explicit SP HT LF.\n\nDoesn't this comment also need an update, given the patch title?\n\n> IFS='\n> -'\n> +'\"$(printf '')\"\n>\n> git_broken_path_fix () {\n>  case \":$PATH:\" in\n> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\n> index 5dfa16a..98eb49a 100755\n> --- a/t/t3404-rebase-interactive.sh\n> +++ b/t/t3404-rebase-interactive.sh\n> @@ -1261,7 +1261,7 @@ test_expect_success 'static check of bad SHA-1' '\n>  test E = $(git cat-file commit HEAD | sed -ne \\$p)\n> '\n>\n> -test_expect_failure 'editor saves as CR/LF' '\n> +test_expect_success 'editor saves as CR/LF' '\n>  git checkout -b with-crlf &&\n>  write_script add-crs.sh <<-\\EOF &&\n>  sed -e \"s/\\$/Q/\" <\"$1\" | tr Q \"\" >\"$1\".new &&\n> -- \n> 2.1.4\n--\nPhilip \n"},{"id":"272191","messageId":"alpine.DEB.1.00.1510251422310.31610@s15462909.onlinehome-server.info","threadId":"40637","inReplyTo":"20151025131059.GA370025@vauxhall.crustytoothpaste.net","subject":"Re: [PATCH 2/2] sh-setup: explicitly mark CR as a field separator","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2015-10-25T13:25:46Z","receivedAt":"2015-10-25T13:25:46Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Brian,\n\nre-Cc:ing the Git mailing list.\n\nOn Sun, 25 Oct 2015, brian m. carlson wrote:\n\n> On Sun, Oct 25, 2015 at 01:50:32PM +0100, Johannes Schindelin wrote:\n> > This is the correct thing to do, really: we already specify LF as\n> > field separator.\n> > \n> > Incidentally, this fixes the problem interactive rebase has when the\n> > editor wants to save text with CR/LF line endings, as WordPad does\n> > in Windows 10.\n> > \n> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n> > ---\n> >  git-sh-setup.sh               | 2 +-\n> >  t/t3404-rebase-interactive.sh | 2 +-\n> >  2 files changed, 2 insertions(+), 2 deletions(-)\n> > \n> > diff --git a/git-sh-setup.sh b/git-sh-setup.sh\n> > index 4691fbc..94dfe04 100644\n> > --- a/git-sh-setup.sh\n> > +++ b/git-sh-setup.sh\n> > @@ -13,7 +13,7 @@ unset CDPATH\n> >  # do not equate an unset IFS with IFS with the default, so here is\n> >  # an explicit SP HT LF.\n> >  IFS=' \t\n> > -'\n> > +'\"$(printf '')\"\n> \n> On Linux, printf '' produces no output.  From my understanding of the\n> POSIX spec, this is the correct behavior.   Does it behave differently\n> on Windows?  If so, it might be nice to explain that in the commit\n> messgae.\n\nHrm. This is apparently a bug in the script I tried to write for three\ndays now (because the mailing list-based code submission is really *so*\nmuch more tedious than the Pull Request-based process I got so used to).\n\nThe '' should really read '\\r'.\n\nCiao,\nJohannes\n"},{"id":"272192","messageId":"alpine.DEB.1.00.1510251426150.31610@s15462909.onlinehome-server.info","threadId":"40637","inReplyTo":"36EB0AFDC58D44F3B48BFA044EA15547@PhilipOakley","subject":"Re: [PATCH 2/2] sh-setup: explicitly mark CR as a field separator","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2015-10-25T13:26:56Z","receivedAt":"2015-10-25T13:26:56Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Philip,\n\nOn Sun, 25 Oct 2015, Philip Oakley wrote:\n\n> From: \"Johannes Schindelin\" <johannes.schindelin@gmx.de>\n> > @@ -13,7 +13,7 @@ unset CDPATH\n> > # do not equate an unset IFS with IFS with the default, so here is\n> > # an explicit SP HT LF.\n> \n> Doesn't this comment also need an update, given the patch title?\n\nYes, you are right.\n\nCiao,\nDscho\n"},{"id":"272194","messageId":"alpine.DEB.1.00.1510251453420.31610@s15462909.onlinehome-server.info","threadId":"40637","inReplyTo":"alpine.DEB.1.00.1510251422310.31610@s15462909.onlinehome-server.info","subject":"shell scripting woes, was Re: [PATCH 2/2] sh-setup: explicitly mark CR as a field separator","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2015-10-25T13:56:39Z","receivedAt":"2015-10-25T13:56:39Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Brian,\n\nOn Sun, 25 Oct 2015, Johannes Schindelin wrote:\n\n> On Sun, 25 Oct 2015, brian m. carlson wrote:\n> \n> > On Sun, Oct 25, 2015 at 01:50:32PM +0100, Johannes Schindelin wrote:\n> > > This is the correct thing to do, really: we already specify LF as\n> > > field separator.\n> > > \n> > > Incidentally, this fixes the problem interactive rebase has when the\n> > > editor wants to save text with CR/LF line endings, as WordPad does\n> > > in Windows 10.\n> > > \n> > > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n> > > ---\n> > >  git-sh-setup.sh               | 2 +-\n> > >  t/t3404-rebase-interactive.sh | 2 +-\n> > >  2 files changed, 2 insertions(+), 2 deletions(-)\n> > > \n> > > diff --git a/git-sh-setup.sh b/git-sh-setup.sh\n> > > index 4691fbc..94dfe04 100644\n> > > --- a/git-sh-setup.sh\n> > > +++ b/git-sh-setup.sh\n> > > @@ -13,7 +13,7 @@ unset CDPATH\n> > >  # do not equate an unset IFS with IFS with the default, so here is\n> > >  # an explicit SP HT LF.\n> > >  IFS=' \t\n> > > -'\n> > > +'\"$(printf '')\"\n> > \n> > On Linux, printf '' produces no output.  From my understanding of the\n> > POSIX spec, this is the correct behavior.   Does it behave differently\n> > on Windows?  If so, it might be nice to explain that in the commit\n> > messgae.\n> \n> Hrm. This is apparently a bug in the script I tried to write for three\n> days now (because the mailing list-based code submission is really *so*\n> much more tedious than the Pull Request-based process I got so used to).\n> \n> The '' should really read '\\r'.\n\nAargh. It is 'dash'. Which is the default /bin/sh on Ubuntu. Try to run\nthe following script through dash and bash, compare the outputs, and weep:\n\n-- snip --\nx=\"$(echo \"printf '\\\\n'\")\"\necho \"$x\"\n-- snap --\n\nCiao,\nJohannes\n\nP.S.: At least here, the output differs as following:\n\nBash:\nprintf '\\n'\n\nDash:\nprintf '\n'\n\n(!!!)\n"},{"id":"272196","messageId":"cover.1445782122.git.johannes.schindelin@gmx.de","threadId":"40637","inReplyTo":"cover.1445777347.git.johannes.schindelin@gmx.de","subject":"[PATCH v2 0/2] Fix interactive rebase when the editor saves with CR/LF","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2015-10-25T14:10:11Z","receivedAt":"2015-10-25T14:10:11Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Chad Boles reported that `git rebase -i` recently started producing\nerrors when the editor saves files with DOS line endings. The symptom\nis:\n\n\tWarning: the command isn't recognized in the following line:\n\t -\n\n\tYou can fix this with 'git rebase --edit-todo'.\n\tOr you can abort the rebase with 'git rebase --abort'.\n\nThe real bummer is that simply calling `git rebase --continue` \"fixes\"\nit.\n\nTurns out that we now check whether a single Carriage Return is a valid\ncommand. This new check was introduced recently (1db168ee9, ironically\nnamed \"rebase-i: loosen over-eager check_bad_cmd check\").\n\nThe proposed fix is to teach *all* shell scripts in Git to accept CR as\na field separator. Since LF is already specified as such, it should be\nan uncontentious change.\n\n\nJohannes Schindelin (2):\n  Demonstrate rebase fails when the editor saves with CR/LF\n  sh-setup: explicitly mark CR as a field separator\n\n git-sh-setup.sh               |  4 ++--\n t/t3404-rebase-interactive.sh | 12 ++++++++++++\n 2 files changed, 14 insertions(+), 2 deletions(-)\n\nInterdiff vs v1:\n\ndiff --git a/git-sh-setup.sh b/git-sh-setup.sh\nindex 94dfe04..e34673d 100644\n--- a/git-sh-setup.sh\n+++ b/git-sh-setup.sh\n@@ -11,7 +11,7 @@ unset CDPATH\n \n # Similarly for IFS, but some shells (e.g. FreeBSD 7.2) are buggy and\n # do not equate an unset IFS with IFS with the default, so here is\n-# an explicit SP HT LF.\n+# an explicit SP HT LF CR.\n IFS=' \t\n-'\"$(printf '\n')\"\n+'\"$(printf '\\r')\"\n \n\n-- \n2.1.4\n"},{"id":"272195","messageId":"90f2eabb6968d9b2fe650d67662ec613a86011cb.1445782122.git.johannes.schindelin@gmx.de","threadId":"40637","inReplyTo":"cover.1445782122.git.johannes.schindelin@gmx.de","subject":"[PATCH v2 1/2] Demonstrate rebase fails when the editor saves with CR/LF","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2015-10-25T14:10:19Z","receivedAt":"2015-10-25T14:10:19Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Based on a bug report by Chad Boles.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n t/t3404-rebase-interactive.sh | 12 ++++++++++++\n 1 file changed, 12 insertions(+)\n\ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex 3de0b1d..5dfa16a 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -1261,4 +1261,16 @@ test_expect_success 'static check of bad SHA-1' '\n \ttest E = $(git cat-file commit HEAD | sed -ne \\$p)\n '\n \n+test_expect_failure 'editor saves as CR/LF' '\n+\tgit checkout -b with-crlf &&\n+\twrite_script add-crs.sh <<-\\EOF &&\n+\tsed -e \"s/\\$/Q/\" <\"$1\" | tr Q \"\\\\015\" >\"$1\".new &&\n+\tmv -f \"$1\".new \"$1\"\n+\tEOF\n+\t(\n+\t\ttest_set_editor \"$(pwd)/add-crs.sh\" &&\n+\t\tgit rebase -i HEAD^\n+\t)\n+'\n+\n test_done\n-- \n2.1.4\n"},{"id":"272197","messageId":"2b089201404299257f23b3931499ea16202f0f65.1445782122.git.johannes.schindelin@gmx.de","threadId":"40637","inReplyTo":"cover.1445782122.git.johannes.schindelin@gmx.de","subject":"[PATCH v2 2/2] sh-setup: explicitly mark CR as a field separator","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2015-10-25T14:10:25Z","receivedAt":"2015-10-25T14:10:25Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"This is the correct thing to do, really: we already specify LF as\nfield separator.\n\nIncidentally, this fixes the problem interactive rebase has when the\neditor wants to save text with CR/LF line endings, as WordPad does\nin Windows 10.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n git-sh-setup.sh               | 4 ++--\n t/t3404-rebase-interactive.sh | 2 +-\n 2 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/git-sh-setup.sh b/git-sh-setup.sh\nindex 4691fbc..e34673d 100644\n--- a/git-sh-setup.sh\n+++ b/git-sh-setup.sh\n@@ -11,9 +11,9 @@ unset CDPATH\n \n # Similarly for IFS, but some shells (e.g. FreeBSD 7.2) are buggy and\n # do not equate an unset IFS with IFS with the default, so here is\n-# an explicit SP HT LF.\n+# an explicit SP HT LF CR.\n IFS=' \t\n-'\n+'\"$(printf '\\r')\"\n \n git_broken_path_fix () {\n \tcase \":$PATH:\" in\ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex 5dfa16a..98eb49a 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -1261,7 +1261,7 @@ test_expect_success 'static check of bad SHA-1' '\n \ttest E = $(git cat-file commit HEAD | sed -ne \\$p)\n '\n \n-test_expect_failure 'editor saves as CR/LF' '\n+test_expect_success 'editor saves as CR/LF' '\n \tgit checkout -b with-crlf &&\n \twrite_script add-crs.sh <<-\\EOF &&\n \tsed -e \"s/\\$/Q/\" <\"$1\" | tr Q \"\\\\015\" >\"$1\".new &&\n-- \n2.1.4\n"},{"id":"272209","messageId":"xmqqpp02kbif.fsf@gitster.mtv.corp.google.com","threadId":"40637","inReplyTo":"cover.1445782122.git.johannes.schindelin@gmx.de","subject":"Re: [PATCH v2 0/2] Fix interactive rebase when the editor saves with CR/LF","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-10-25T19:12:56Z","receivedAt":"2015-10-25T19:12:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <johannes.schindelin@gmx.de> writes:\n\n> Chad Boles reported that `git rebase -i` recently started producing\n> errors when the editor saves files with DOS line endings. The symptom\n> is:\n>\n> \tWarning: the command isn't recognized in the following line:\n> \t -\n>\n> \tYou can fix this with 'git rebase --edit-todo'.\n> \tOr you can abort the rebase with 'git rebase --abort'.\n>\n> The real bummer is that simply calling `git rebase --continue` \"fixes\"\n> it.\n\nTwo questions.\n\n * What does the DOS editor do to a file with ^M in the middle of a\n   line, e.g. \"A^MB^M^J\"?\n\n * Is your shell ported correctly to the platform?\n\nThe latter may need a bit of elaboration.  \"read a b c\" is supposed\nto read one line of text (where the definition of line is platform\ndependent, your platform may use CRLF to signal the end of an line),\nsplit the characters on the line (i.e. LF vs CRLF no longer matters\nat this point) at $IFS characters and give them to $a $b and $c. If\nthe platform accepts CRLF as the EOL signal, should the program still\nsee CR at the end of $c?\n\nA solution that mucks with IFS smells like fixing a wrong problem\nwithout addressing the real cause.\n\nAlso IFS is used not only by \"read\", so munging it globally doubly\nfeels wrong.\n\nIn addition, you do not want to split at CR; what you want is to\ntreat CRLF (i.e. not a lone CR) as the end-of-line separator.\nAdding CR to IFS feels triply wrong.\n\nBy the way, saying \"This is right, really\" makes you sound as if you\ndo not have a real argument.\n"},{"id":"272231","messageId":"vpqlhaqas8g.fsf@grenoble-inp.fr","threadId":"40637","inReplyTo":"2b089201404299257f23b3931499ea16202f0f65.1445782122.git.johannes.schindelin@gmx.de","subject":"Re: [PATCH v2 2/2] sh-setup: explicitly mark CR as a field separator","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-10-26T09:34:07Z","receivedAt":"2015-10-26T09:34:07Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Johannes Schindelin <johannes.schindelin@gmx.de> writes:\n\n> This is the correct thing to do, really: we already specify LF as\n> field separator.\n\nI'm almost convinced that this is the right thing to do in the long run\n(\"almost\" because I'm not sure, not because I have arguments against). I\nagree with Junio that the commit message should be more convincing, but\nindeed, accepting LF and not CR is strange.\n\nHowever, is this the right thing to do in the maintainance branch? It\ndoes fix the issue, but does so in a rather intrusive way, so I'd need\nmore arguments to be convinced that this is safe to merge in maint. Or\nhave a local fix for rebase to be merged in maint, and apply this in\nmaster for the next feature release.\n\nSorry for being negative, and especially sorry since I'm partly guilty\nfor the breakage. I just want to be sure that we don't break anything\nwhile repairing it (we already introduced this breakage while repairing\nanother one...).\n\n>  # Similarly for IFS, but some shells (e.g. FreeBSD 7.2) are buggy and\n>  # do not equate an unset IFS with IFS with the default, so here is\n> -# an explicit SP HT LF.\n> +# an explicit SP HT LF CR.\n>  IFS=' \t\n> -'\n> +'\"$(printf '\\r')\"\n\nWhile we're there, it may be better to have a single \"printf ' \\t\\n\\r'\"\nto avoid the whitespace magic in the source code.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"272232","messageId":"alpine.DEB.1.00.1510261133030.31610@s15462909.onlinehome-server.info","threadId":"40637","inReplyTo":"xmqqpp02kbif.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2 0/2] Fix interactive rebase when the editor saves with CR/LF","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2015-10-26T10:43:44Z","receivedAt":"2015-10-26T10:43:44Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Sun, 25 Oct 2015, Junio C Hamano wrote:\n\n> Johannes Schindelin <johannes.schindelin@gmx.de> writes:\n> \n> > Chad Boles reported that `git rebase -i` recently started producing\n> > errors when the editor saves files with DOS line endings. The symptom\n> > is:\n> >\n> > \tWarning: the command isn't recognized in the following line:\n> > \t -\n> >\n> > \tYou can fix this with 'git rebase --edit-todo'.\n> > \tOr you can abort the rebase with 'git rebase --abort'.\n> >\n> > The real bummer is that simply calling `git rebase --continue` \"fixes\"\n> > it.\n> \n> Two questions.\n> \n>  * What does the DOS editor do to a file with ^M in the middle of a\n>    line, e.g. \"A^MB^M^J\"?\n\nYou mean mixed line endings? At least with this Windows 10 WordPad, *all*\nline endings are normalized to CR/LF. That is, if you edit a file that\ncontains a stray CR, it is presented as a line break.\n\n>  * Is your shell ported correctly to the platform?\n\nI would think so. It is an MSys2 program, therefore it uses all the POSIX\nniceties, of course.\n\nA simple test with CR/LF line endings in a script reveals that it is\npretty solid:\n\n\tx=a\n\tcase \"$x\" in a) echo b;; esac\n\nprints out 'b', as expected.\n\nPlease also note that things apparently worked alright before the patch\nremoving the stripspace call was applied.\n\n> The latter may need a bit of elaboration.  \"read a b c\" is supposed\n> to read one line of text (where the definition of line is platform\n> dependent, your platform may use CRLF to signal the end of an line),\n> split the characters on the line (i.e. LF vs CRLF no longer matters\n> at this point) at $IFS characters and give them to $a $b and $c. If\n> the platform accepts CRLF as the EOL signal, should the program still\n> see CR at the end of $c?\n> \n> A solution that mucks with IFS smells like fixing a wrong problem\n> without addressing the real cause.\n\nPlease note that it is a bit unsettling if you use funny language like\n\"smells\" here and then accuse me of not having an argument when I point\nthat the same rationale applies to having CR in IFS as applies to having\nLF in IFS. Yes, that was an implicit argument, but it is a strong one, so\nI do not think you are well served ignoring it.\n\n> Also IFS is used not only by \"read\", so munging it globally doubly\n> feels wrong.\n> \n> In addition, you do not want to split at CR; what you want is to\n> treat CRLF (i.e. not a lone CR) as the end-of-line separator.\n> Adding CR to IFS feels triply wrong.\n> \n> By the way, saying \"This is right, really\" makes you sound as if you\n> do not have a real argument.\n\nAgain. If CR has no place in IFS, why does LF have a place in IFS? It\nmakes *no* sense to argue for one and against the other.\n\nIn any case, I am not interested in arguing for arguing's sake. Tell me\nwhat you want instead of the patch to IFS, I will implement it and test\nwhether it fixes the bug and we will move on.\n\nCiao,\nJohannes\n"},{"id":"272250","messageId":"xmqq8u6piir8.fsf@gitster.mtv.corp.google.com","threadId":"40637","inReplyTo":"vpqlhaqas8g.fsf@grenoble-inp.fr","subject":"Re: [PATCH v2 2/2] sh-setup: explicitly mark CR as a field separator","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-10-26T18:31:39Z","receivedAt":"2015-10-26T18:31:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:\n\n> Johannes Schindelin <johannes.schindelin@gmx.de> writes:\n>\n>> This is the correct thing to do, really: we already specify LF as\n>> field separator.\n>\n> I'm almost convinced that this is the right thing to do in the long run\n> (\"almost\" because I'm not sure, not because I have arguments against). I\n> agree with Junio that the commit message should be more convincing, but\n> indeed, accepting LF and not CR is strange.\n\nIf there were a single character that denotes CRLF, I'd say that\nincluding such a character in IFS would make sense on a system with\nCRLF EOL convention.  But that is not the case.\n\nOn a platform with LF EOL convention, having LF in IFS makes sense,\ndue to two reasons.\n\n * read is not the only user of IFS.  Expressing \"list of things\"\n   (pre bashism \"shell array\" days) by concatenating elements into a\n   single string variable, separated with LF, and later iterating\n   over them is a very common use case, e.g.\n\n\tLF='\n        '\n\tlist=\"$thing1\"\n\tlist=\"$list$LF$thing2\"\n\tlist=\"$list$LF$thing3\"\n        ...\n\n\tIFS=$LF\n\tfor thing in $list\n        do\n        \t...\n\n   And including LF by default in IFS, especially when \"things\" can\n   contain SP/HT, is handy.\n\n * It does not hurt on a platform with LF EOL convention to include\n   LF in IFS, because you cannot have a \"lone LF\" in the middle of a\n   line.  Presence of a single LF alone will terminate the current\n   line and the bytes that follow it will be a next line.\n\nNo similarity argument can be made for a lone CR on a platform that\nuses CRLF EOL convention.  A \"lone CR\" can appear in the middle of a\nline without terminating the current line, as only a CR that is\nimmediately followed by a LF is the end of line, so you cannot make\nthe \"It does not hurt\" argument for including CR to IFS.  If you\nhave a variable in which \"A^MB\" is there, \"set $variable\" would\nsplit them into two and assign B to $2, which is not what the\nscripts would expect.\n\n> However, is this the right thing to do in the maintainance branch? It\n> does fix the issue, but does so in a rather intrusive way, so I'd need\n> more arguments to be convinced that this is safe to merge in maint.\n\nIf this were the \"right\" thing in general for shell scripts on\nsystems with CRLF EOL convention, the implementation of the shell\nlanguage on such a platform would be doing that upon startup (or\nupon \"unset IFS\"), and we wouldn't be having this discussion.  Don't\nyou think it is strange that individual applications like Git have\nto set IFS to include CR?  I see it as a strong sign that this is\nnot a correct thing to do.\n\nIntrusive does not begin to describe it.\n\nI think the \"right\" thing for a shell implementation on a CRLF\nplatform to do is twofold.\n\n * Read\n   http://pubs.opengroup.org/onlinepubs/9699919799/utilities/read.html\n   and notice that the operation is defined as a two-step process.\n   It is supposed to read an input line and then \"The terminating\n   <newline> (if any) shall be removed from the input\".  And then\n   \"the result shall be split into fields\" (at IFS boundaries).\n\n   As POSIX does not say <newline> has to be a single octet with\n   value 10 (the same way you are allowed to show CRLF when you call\n   printf(\"\\n\")), a shell implementation would read CRLF terminated\n   lines, and remove the terminating CRLF before doing the field\n   splitting using IFS.\n\n   This is why I said in my message to Dscho that IFS does not get\n   into the picture.\n\n * Implement the \"field splitting\" (at IFS boundaries) with a small\n   twist.  When the script/user included '\\n' in IFS, split at LF\n   but remove CR if there is one that immediately precedes the LF.\n\nThe latter would help if you did the previous \"read is not the only\nuser of IFS\" example like this instead:\n\n        LF=$(printf '\\n.')\n        LF=${LF%.}\n\tlist=\"$thing1\"\n\tlist=\"$list$LF$thing2\"\n\tlist=\"$list$LF$thing3\"\n        ...\n\n\tIFS=$LF\n\tfor thing in $list\n        do\n        \t...\n\nand the platform 'printf' gave CRLF for \"\\n\" (newline), resulting\nthe list to be concatenated with CRLF, not LF.\n\nAnd this is why I asked Dscho if the shell is done correctly _for_\nthe platform.\n\n> Sorry for being negative, and especially sorry since I'm partly guilty\n> for the breakage. I just want to be sure that we don't break anything\n> while repairing it (we already introduced this breakage while repairing\n> another one...).\n\nSomething along the line of the following would be tolerable, even\nthough I think in the longer term, not just in Git land but in the\nlarger ecosystem to use POSIXy tools on Windows, the best solution\nis to fix the shell so that it matches the expectation of the users\nof its platform.\n\nI say \"something along the line of\" here because I do not know how\nthe problematic shell behaves on the assignment command that stuffs\na lone CR into a variable.  It _might_ need a similar protection\nagainst the shell feature \"the last EOL is removed from the result\nof command expansion\", as I did in the above example, depending on\nhow incoherent the implementation is.  The implementation seems to\naccept CRLF and LF in shell scripts proper just fine, but apparently\nits implementation of \"read\" does not honor such platform EOL\nconvention, which caused this issue, and I don't know what it does\nin the codepath that implements command expansion.\n\n git-rebase--interactive.sh | 5 +++++\n 1 file changed, 5 insertions(+)\n\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex c42ba34..8f13a35 100644\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -881,6 +881,7 @@ check_commit_sha () {\n check_bad_cmd_and_sha () {\n \tretval=0\n \tlineno=0\n+\tcr=$(printf \"%c\" 13)\n \twhile read -r command rest\n \tdo\n \t\tlineno=$(( $lineno + 1 ))\n@@ -888,6 +889,10 @@ check_bad_cmd_and_sha () {\n \t\t\"$comment_char\"*|''|noop|x|exec)\n \t\t\t# Doesn't expect a SHA-1\n \t\t\t;;\n+\t\t\"$cr\")\n+\t\t\t# Windows port of shell not stripping the newline\n+\t\t\t# at the end of an empty line correctly.\n+\t\t\t;;\n \t\tpick|p|drop|d|reword|r|edit|e|squash|s|fixup|f)\n \t\t\tif ! check_commit_sha \"${rest%%[ \t]*}\" \"$lineno\" \"$1\"\n \t\t\tthen\n"},{"id":"272253","messageId":"xmqqsi4xh298.fsf@gitster.mtv.corp.google.com","threadId":"40637","inReplyTo":"alpine.DEB.1.00.1510261133030.31610@s15462909.onlinehome-server.info","subject":"Re: [PATCH v2 0/2] Fix interactive rebase when the editor saves with CR/LF","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-10-26T19:13:23Z","receivedAt":"2015-10-26T19:13:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> A simple test with CR/LF line endings in a script reveals that it is\n> pretty solid:\n>\n> \tx=a\n> \tcase \"$x\" in a) echo b;; esac\n>\n> prints out 'b', as expected.\n\nI do not see what this has to do with anything.\n\nThe shell language parser when parsing a script may do the right\nthing, but the bug I was alluding to was that your 'read' does not\nseem to be removing the terminating <newline> (which is CRLF on your\nplatform) after reading a line before splitting the contents on the\nline at IFS boundaries.\n\n> Again. If CR has no place in IFS, why does LF have a place in IFS? It\n> makes *no* sense to argue for one and against the other.\n\nSee my message to Matthieu.\n"},{"id":"272261","messageId":"xmqqbnblgzrk.fsf@gitster.mtv.corp.google.com","threadId":"40637","inReplyTo":"xmqq8u6piir8.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2 2/2] sh-setup: explicitly mark CR as a field separator","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-10-26T20:07:11Z","receivedAt":"2015-10-26T20:07:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Something along the line of the following would be tolerable, even\n> though I think in the longer term, not just in Git land but in the\n> larger ecosystem to use POSIXy tools on Windows, the best solution\n> is to fix the shell so that it matches the expectation of the users\n> of its platform.\n>\n> I say \"something along the line of\" here because I do not know how\n> the problematic shell behaves on the assignment command that stuffs\n> a lone CR into a variable.  It _might_ need a similar protection\n> against the shell feature \"the last EOL is removed from the result\n> of command expansion\", as I did in the above example, depending on\n> how incoherent the implementation is.  The implementation seems to\n> accept CRLF and LF in shell scripts proper just fine, but apparently\n> its implementation of \"read\" does not honor such platform EOL\n> convention, which caused this issue, and I don't know what it does\n> in the codepath that implements command expansion.\n\nI still have to say \"something along the lines of\" as I do not have\n(and I do not wish to have, to be honest) an access to test this\nwith the problematic shell implementation, but here is an updated\npatch.\n\n-- >8 --\nSubject: [PATCH] rebase-i: work around Windows CRLF line endings\n\nEditors on Windows can and do save text files with CRLF line\nendings, which is the convention on the platform.  We are seeing\nreports that the \"read\" command in a port of bash to the environment\nhowever does not strip the CRLF at the end, not adjusting for the\nsame convention on the platform.\n\nThis breaks the recently added sanity checks for the insn sheet fed\nto \"rebase -i\"; instead of an empty line (hence nothing in $command),\nthe script was getting a lone CR in there.\n\nSpecial case a lone CR and treat it the same way as an empty line to\nwork this around.\n\nThe test was stolen from Dscho.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n git-rebase--interactive.sh    | 13 +++++++++++++\n t/t3404-rebase-interactive.sh | 12 ++++++++++++\n 2 files changed, 25 insertions(+)\n\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex c42ba34..3911711 100644\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -77,6 +77,10 @@ amend=\"$state_dir\"/amend\n rewritten_list=\"$state_dir\"/rewritten-list\n rewritten_pending=\"$state_dir\"/rewritten-pending\n \n+# Work around a Windows port of shell that does not strip\n+# the newline at the end of a line correctly.\n+cr=$(printf \"\\015\")\n+\n strategy_args=\n if test -n \"$do_merge\"\n then\n@@ -518,6 +522,11 @@ do_next () {\n \t\"$comment_char\"*|''|noop|drop|d)\n \t\tmark_action_done\n \t\t;;\n+\t\"$cr\")\n+\t\t# Windows port of shell not stripping the newline\n+\t\t# at the end of an empty line correctly.\n+\t\tmark_action_done\n+\t\t;;\n \tpick|p)\n \t\tcomment_for_reflog pick\n \n@@ -888,6 +897,10 @@ check_bad_cmd_and_sha () {\n \t\t\"$comment_char\"*|''|noop|x|exec)\n \t\t\t# Doesn't expect a SHA-1\n \t\t\t;;\n+\t\t\"$cr\")\n+\t\t\t# Windows port of shell not stripping the newline\n+\t\t\t# at the end of an empty line correctly.\n+\t\t\t;;\n \t\tpick|p|drop|d|reword|r|edit|e|squash|s|fixup|f)\n \t\t\tif ! check_commit_sha \"${rest%%[ \t]*}\" \"$lineno\" \"$1\"\n \t\t\tthen\ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex 88d7d53..2f97a3d 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -1240,4 +1240,16 @@ test_expect_success 'static check of bad SHA-1' '\n \ttest E = $(git cat-file commit HEAD | sed -ne \\$p)\n '\n \n+test_expect_success 'editor saves as CR/LF' '\n+\tgit checkout -b with-crlf &&\n+\twrite_script add-crs.sh <<-\\EOF &&\n+\tsed -e \"s/\\$/Q/\" <\"$1\" | tr Q \"\\\\015\" >\"$1\".new &&\n+\tmv -f \"$1\".new \"$1\"\n+\tEOF\n+\t(\n+\t\ttest_set_editor \"$(pwd)/add-crs.sh\" &&\n+\t\tgit rebase -i HEAD^\n+\t)\n+'\n+\n test_done\n-- \n2.6.2-405-g6877da6\n"},{"id":"272294","messageId":"alpine.DEB.1.00.1510270950230.31610@s15462909.onlinehome-server.info","threadId":"40637","inReplyTo":"xmqq8u6piir8.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2 2/2] sh-setup: explicitly mark CR as a field separator","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2015-10-27T09:19:21Z","receivedAt":"2015-10-27T09:19:21Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Mon, 26 Oct 2015, Junio C Hamano wrote:\n\n> I asked Dscho if the shell is done correctly _for_ the platform.\n\nThis assumes that the platform is either CR/LF or LF. That is incorrect.\nWindows does *not* dictate the line endings to be CR/LF. Certain\napplications do.\n\nThe shell is an MSys2 shell, and MSys2 convention is to use LF. Hence the\nshell is done correctly for the platform.\n\nYet on Windows (and in cross-platform projects also on Linux and MacOSX),\nyou have to deal with the fact that text files produced outside the cozy\nlittle shell can have different line endings.\n\nAs I said, I am uninterested in arguing for arguing's sake. Otherwise I\nwould shoot down your argument that LF in IFS \"does not hurt\" but CR\nsomehow magically does. I am sure it would be a helluva discussion and\nfun. But I do not have the time.\n\nWhat I really needed you to do indicate the way forward.\n\nThe patch you provided in the mail I am replying to fails to fix the bug.\n\nCiao,\nJohannes\n"},{"id":"272295","messageId":"alpine.DEB.1.00.1510271031000.31610@s15462909.onlinehome-server.info","threadId":"40637","inReplyTo":"xmqqsi4xh298.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2 0/2] Fix interactive rebase when the editor saves with CR/LF","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2015-10-27T09:34:17Z","receivedAt":"2015-10-27T09:34:17Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Mon, 26 Oct 2015, Junio C Hamano wrote:\n\n> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n> \n> > A simple test with CR/LF line endings in a script reveals that it is\n> > pretty solid:\n> >\n> > \tx=a\n> > \tcase \"$x\" in a) echo b;; esac\n> >\n> > prints out 'b', as expected.\n> \n> I do not see what this has to do with anything.\n\nYou probably missed the context. Brian pointed out that my patch said\n\"printf ''\" when it should have said \"printf '\\n'\". I responded that my\ncommit says the right thing, but somewhere along the lines of transforming\nbeautiful Git commits into mails it must have been lost.\n\nThe explanation you quoted is the essence of the problem of my script to\nprepare mails for submission to the mailing list: the script works in\nBash, but fails in Dash. And as my script has the shebang \"#!/bin/sh\" and\nas Ubuntu defaults to using the Dash as its /bin/sh, we now have the full\nexplanation why my first mail showed an incorrect patch.\n\nCiao,\nJohannes\n"},{"id":"272297","messageId":"alpine.DEB.1.00.1510271019490.31610@s15462909.onlinehome-server.info","threadId":"40637","inReplyTo":"xmqqbnblgzrk.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2 2/2] sh-setup: explicitly mark CR as a field separator","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2015-10-27T09:46:47Z","receivedAt":"2015-10-27T09:46:47Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Mon, 26 Oct 2015, Junio C Hamano wrote:\n\n> Subject: [PATCH] rebase-i: work around Windows CRLF line endings\n> \n> Editors on Windows can and do save text files with CRLF line\n> endings, which is the convention on the platform.  We are seeing\n> reports that the \"read\" command in a port of bash to the environment\n> however does not strip the CRLF at the end, not adjusting for the\n> same convention on the platform.\n> \n> This breaks the recently added sanity checks for the insn sheet fed\n> to \"rebase -i\"; instead of an empty line (hence nothing in $command),\n> the script was getting a lone CR in there.\n> \n> Special case a lone CR and treat it the same way as an empty line to\n> work this around.\n\nYou do not need me to tell you that it is likely that the same issue will\narise in other places, but if you are content with this work-around, so be\nit.\n\n\n> The test was stolen from Dscho.\n\nThere was no need to steal this, as I made a specific effort to introduce\nthe test in a separate commit. There was also no need to remove the\nattribution for finding the bug.\n\nLet's not do that.\n\nI will submit v3 in a second, including your work-around in favor of the\npatch you hate so much.\n\nCiao,\nJohannes\n"},{"id":"272298","messageId":"cover.1445939154.git.johannes.schindelin@gmx.de","threadId":"40637","inReplyTo":"cover.1445782122.git.johannes.schindelin@gmx.de","subject":"[PATCH v3 0/2] Fix interactive rebase when the editor saves with CR/LF","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2015-10-27T09:47:10Z","receivedAt":"2015-10-27T09:47:10Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Chad Boles reported that `git rebase -i` recently started producing\nerrors when the editor saves files with DOS line endings. The symptom\nis:\n\n\tWarning: the command isn't recognized in the following line:\n\t -\n\n\tYou can fix this with 'git rebase --edit-todo'.\n\tOr you can abort the rebase with 'git rebase --abort'.\n\nThe real bummer is that simply calling `git rebase --continue` \"fixes\"\nit.\n\nTurns out that we now check whether a single Carriage Return is a valid\ncommand. This new check was introduced recently (1db168ee9, ironically\nnamed \"rebase-i: loosen over-eager check_bad_cmd check\").\n\nThe proposed fix is to teach *all* shell scripts in Git to accept CR as\na field separator. Since LF is already specified as such, it should be\nan uncontentious change.\n\n\nJohannes Schindelin (1):\n  Demonstrate rebase fails when the editor saves with CR/LF\n\nJunio C Hamano (1):\n  rebase-i: work around Windows CRLF line endings\n\n git-rebase--interactive.sh    | 13 +++++++++++++\n t/t3404-rebase-interactive.sh | 12 ++++++++++++\n 2 files changed, 25 insertions(+)\n\nInterdiff vs v2:\n\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex d65c06e..daadf2d 100644\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -77,6 +77,10 @@ amend=\"$state_dir\"/amend\n rewritten_list=\"$state_dir\"/rewritten-list\n rewritten_pending=\"$state_dir\"/rewritten-pending\n \n+# Work around a Windows port of shell that does not strip\n+# the newline at the end of a line correctly.\n+cr=$(printf \"\\015\")\n+\n strategy_args=\n if test -n \"$do_merge\"\n then\n@@ -518,6 +522,11 @@ do_next () {\n \t\"$comment_char\"*|''|noop|drop|d)\n \t\tmark_action_done\n \t\t;;\n+\t\"$cr\")\n+\t\t# Windows port of shell not stripping the newline\n+\t\t# at the end of an empty line correctly.\n+\t\tmark_action_done\n+\t\t;;\n \tpick|p)\n \t\tcomment_for_reflog pick\n \n@@ -896,6 +905,10 @@ check_bad_cmd_and_sha () {\n \t\t\"$comment_char\"*|''|noop|x|exec)\n \t\t\t# Doesn't expect a SHA-1\n \t\t\t;;\n+\t\t\"$cr\")\n+\t\t\t# Windows port of shell not stripping the newline\n+\t\t\t# at the end of an empty line correctly.\n+\t\t\t;;\n \t\tpick|p|drop|d|reword|r|edit|e|squash|s|fixup|f)\n \t\t\tif ! check_commit_sha \"${rest%%[ \t]*}\" \"$lineno\" \"$1\"\n \t\t\tthen\ndiff --git a/git-sh-setup.sh b/git-sh-setup.sh\nindex e34673d..4691fbc 100644\n--- a/git-sh-setup.sh\n+++ b/git-sh-setup.sh\n@@ -11,9 +11,9 @@ unset CDPATH\n \n # Similarly for IFS, but some shells (e.g. FreeBSD 7.2) are buggy and\n # do not equate an unset IFS with IFS with the default, so here is\n-# an explicit SP HT LF CR.\n+# an explicit SP HT LF.\n IFS=' \t\n-'\"$(printf '\\r')\"\n+'\n \n git_broken_path_fix () {\n \tcase \":$PATH:\" in\n\n-- \n2.1.4\n"},{"id":"272299","messageId":"3d37a1348065d5902c829481db2124a52be0ca1a.1445939154.git.johannes.schindelin@gmx.de","threadId":"40637","inReplyTo":"cover.1445939154.git.johannes.schindelin@gmx.de","subject":"[PATCH v3 1/2] Demonstrate rebase fails when the editor saves with CR/LF","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2015-10-27T09:47:16Z","receivedAt":"2015-10-27T09:47:16Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Based on a bug report by Chad Boles.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n t/t3404-rebase-interactive.sh | 12 ++++++++++++\n 1 file changed, 12 insertions(+)\n\ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex 3de0b1d..5dfa16a 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -1261,4 +1261,16 @@ test_expect_success 'static check of bad SHA-1' '\n \ttest E = $(git cat-file commit HEAD | sed -ne \\$p)\n '\n \n+test_expect_failure 'editor saves as CR/LF' '\n+\tgit checkout -b with-crlf &&\n+\twrite_script add-crs.sh <<-\\EOF &&\n+\tsed -e \"s/\\$/Q/\" <\"$1\" | tr Q \"\\\\015\" >\"$1\".new &&\n+\tmv -f \"$1\".new \"$1\"\n+\tEOF\n+\t(\n+\t\ttest_set_editor \"$(pwd)/add-crs.sh\" &&\n+\t\tgit rebase -i HEAD^\n+\t)\n+'\n+\n test_done\n-- \n2.1.4\n"},{"id":"272300","messageId":"adcab7dcf9d802b3703b9ec9291132259a092a04.1445939154.git.johannes.schindelin@gmx.de","threadId":"40637","inReplyTo":"cover.1445939154.git.johannes.schindelin@gmx.de","subject":"[PATCH v3 2/2] rebase-i: work around Windows CRLF line endings","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2015-10-27T09:47:21Z","receivedAt":"2015-10-27T09:47:21Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Junio C Hamano <gitster@pobox.com>\n\nEditors on Windows can and do save text files with CRLF line\nendings, which is the convention on the platform.  We are seeing\nreports that the \"read\" command in a port of bash to the environment\nhowever does not strip the CRLF at the end, not adjusting for the\nsame convention on the platform.\n\nThis breaks the recently added sanity checks for the insn sheet fed\nto \"rebase -i\"; instead of an empty line (hence nothing in $command),\nthe script was getting a lone CR in there.\n\nSpecial case a lone CR and treat it the same way as an empty line to\nwork this around.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n git-rebase--interactive.sh    | 13 +++++++++++++\n t/t3404-rebase-interactive.sh |  2 +-\n 2 files changed, 14 insertions(+), 1 deletion(-)\n\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex d65c06e..daadf2d 100644\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -77,6 +77,10 @@ amend=\"$state_dir\"/amend\n rewritten_list=\"$state_dir\"/rewritten-list\n rewritten_pending=\"$state_dir\"/rewritten-pending\n \n+# Work around a Windows port of shell that does not strip\n+# the newline at the end of a line correctly.\n+cr=$(printf \"\\015\")\n+\n strategy_args=\n if test -n \"$do_merge\"\n then\n@@ -518,6 +522,11 @@ do_next () {\n \t\"$comment_char\"*|''|noop|drop|d)\n \t\tmark_action_done\n \t\t;;\n+\t\"$cr\")\n+\t\t# Windows port of shell not stripping the newline\n+\t\t# at the end of an empty line correctly.\n+\t\tmark_action_done\n+\t\t;;\n \tpick|p)\n \t\tcomment_for_reflog pick\n \n@@ -896,6 +905,10 @@ check_bad_cmd_and_sha () {\n \t\t\"$comment_char\"*|''|noop|x|exec)\n \t\t\t# Doesn't expect a SHA-1\n \t\t\t;;\n+\t\t\"$cr\")\n+\t\t\t# Windows port of shell not stripping the newline\n+\t\t\t# at the end of an empty line correctly.\n+\t\t\t;;\n \t\tpick|p|drop|d|reword|r|edit|e|squash|s|fixup|f)\n \t\t\tif ! check_commit_sha \"${rest%%[ \t]*}\" \"$lineno\" \"$1\"\n \t\t\tthen\ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex 5dfa16a..98eb49a 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -1261,7 +1261,7 @@ test_expect_success 'static check of bad SHA-1' '\n \ttest E = $(git cat-file commit HEAD | sed -ne \\$p)\n '\n \n-test_expect_failure 'editor saves as CR/LF' '\n+test_expect_success 'editor saves as CR/LF' '\n \tgit checkout -b with-crlf &&\n \twrite_script add-crs.sh <<-\\EOF &&\n \tsed -e \"s/\\$/Q/\" <\"$1\" | tr Q \"\\\\015\" >\"$1\".new &&\n-- \n2.1.4\n"},{"id":"272301","messageId":"alpine.DEB.1.00.1510271054380.31610@s15462909.onlinehome-server.info","threadId":"40637","inReplyTo":"adcab7dcf9d802b3703b9ec9291132259a092a04.1445939154.git.johannes.schindelin@gmx.de","subject":"Re: [PATCH v3 2/2] rebase-i: work around Windows CRLF line endings","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2015-10-27T09:55:44Z","receivedAt":"2015-10-27T09:55:44Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 27 Oct 2015, Johannes Schindelin wrote:\n\n> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\n> index 5dfa16a..98eb49a 100755\n> --- a/t/t3404-rebase-interactive.sh\n> +++ b/t/t3404-rebase-interactive.sh\n> @@ -1261,7 +1261,7 @@ test_expect_success 'static check of bad SHA-1' '\n>  \ttest E = $(git cat-file commit HEAD | sed -ne \\$p)\n>  '\n>  \n> -test_expect_failure 'editor saves as CR/LF' '\n> +test_expect_success 'editor saves as CR/LF' '\n>  \tgit checkout -b with-crlf &&\n>  \twrite_script add-crs.sh <<-\\EOF &&\n>  \tsed -e \"s/\\$/Q/\" <\"$1\" | tr Q \"\\\\015\" >\"$1\".new &&\n\nMy apologies: I forgot to spell out explicitly that this passes in Git for\nWindows 2.x' SDK: It does.\n\nCiao,\nJohannes\n"},{"id":"272302","messageId":"vpqd1w07hr7.fsf@grenoble-inp.fr","threadId":"40637","inReplyTo":"xmqq8u6piir8.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2 2/2] sh-setup: explicitly mark CR as a field separator","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-10-27T10:01:00Z","receivedAt":"2015-10-27T10:01:00Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:\n>\n>> Johannes Schindelin <johannes.schindelin@gmx.de> writes:\n>>\n>>> This is the correct thing to do, really: we already specify LF as\n>>> field separator.\n>>\n>> I'm almost convinced that this is the right thing to do in the long run\n>> (\"almost\" because I'm not sure, not because I have arguments against). I\n>> agree with Junio that the commit message should be more convincing, but\n>> indeed, accepting LF and not CR is strange.\n>\n> If there were a single character that denotes CRLF, I'd say that\n> including such a character in IFS would make sense on a system with\n> CRLF EOL convention.  But that is not the case.\n\nIt's not, but there are platforms where the newline convention is\nCR-only. AFAIK, they are all \"old\" architectures (like Mac OS < X), but\nthere are still CR-only files lying around.\n\n>  * read is not the only user of IFS.  Expressing \"list of things\"\n>    (pre bashism \"shell array\" days) by concatenating elements into a\n>    single string variable, separated with LF, and later iterating\n>    over them is a very common use case, e.g.\n>\n> \tLF='\n>         '\n> \tlist=\"$thing1\"\n> \tlist=\"$list$LF$thing2\"\n> \tlist=\"$list$LF$thing3\"\n>         ...\n>\n> \tIFS=$LF\n> \tfor thing in $list\n>         do\n>         \t...\n>\n>    And including LF by default in IFS, especially when \"things\" can\n>    contain SP/HT, is handy.\n\nI don't get the argument. You're talking about a case where you\nexplicitly set IFS, and the patch is about changing the default. The\nuse-case above would work exactly like before if we modify the default\nIFS.\n\n> If you\n> have a variable in which \"A^MB\" is there, \"set $variable\" would\n> split them into two and assign B to $2, which is not what the\n> scripts would expect.\n\nThe same goes if you replace ^M with a tab or a space. Using unquoted\n\"set $variable\" is sane only if you are sure that $variable does not\ncontain unexpected special characters. I can't imagine a case where one\nwould accept space, tab or LF as separator and would need to accept CR\nas a non-separator.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"272314","messageId":"xmqqd1w0dy0k.fsf@gitster.mtv.corp.google.com","threadId":"40637","inReplyTo":"alpine.DEB.1.00.1510271054380.31610@s15462909.onlinehome-server.info","subject":"Re: [PATCH v3 2/2] rebase-i: work around Windows CRLF line endings","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-10-27T17:25:31Z","receivedAt":"2015-10-27T17:25:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> On Tue, 27 Oct 2015, Johannes Schindelin wrote:\n>\n>> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\n>> index 5dfa16a..98eb49a 100755\n>> --- a/t/t3404-rebase-interactive.sh\n>> +++ b/t/t3404-rebase-interactive.sh\n>> @@ -1261,7 +1261,7 @@ test_expect_success 'static check of bad SHA-1' '\n>>  \ttest E = $(git cat-file commit HEAD | sed -ne \\$p)\n>>  '\n>>  \n>> -test_expect_failure 'editor saves as CR/LF' '\n>> +test_expect_success 'editor saves as CR/LF' '\n>>  \tgit checkout -b with-crlf &&\n>>  \twrite_script add-crs.sh <<-\\EOF &&\n>>  \tsed -e \"s/\\$/Q/\" <\"$1\" | tr Q \"\\\\015\" >\"$1\".new &&\n>\n> My apologies: I forgot to spell out explicitly that this passes in Git for\n> Windows 2.x' SDK: It does.\n\nCan you add that to the log message?  Your cover letter may also\nwant to be updated for those who fish for the last version posted\nin the list archive.\n\nThanks.\n"},{"id":"272316","messageId":"xmqqsi4wcigx.fsf@gitster.mtv.corp.google.com","threadId":"40637","inReplyTo":"alpine.DEB.1.00.1510270950230.31610@s15462909.onlinehome-server.info","subject":"Re: [PATCH v2 2/2] sh-setup: explicitly mark CR as a field separator","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-10-27T17:46:38Z","receivedAt":"2015-10-27T17:46:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> This assumes that the platform is either CR/LF or LF. That is incorrect.\n> Windows does *not* dictate the line endings to be CR/LF. Certain\n> applications do.\n>\n> The shell is an MSys2 shell, and MSys2 convention is to use LF. Hence the\n> shell is done correctly for the platform.\n>\n> Yet on Windows (and in cross-platform projects also on Linux and MacOSX),\n> you have to deal with the fact that text files produced outside the cozy\n> little shell can have different line endings.\n\nI know that.  That is why I said this is platform specific.  When\nsomebody on Ubuntu comes and says \"XYZ does not work when I saved my\ntext file with CRLF\", I can say \"don't do that then\".  I've been\nassuming that you cannot say the same to your users, in order to be\nnicer to Windows.  You say MSys2 convention is to use LF, but what\ndo you exactly mean by that?\n\nI am hoping that you do not mean \"we treat a file created by a DOS\neditor as a text file with CR appended at the end of each line\".\nI'd instead expect that a file the user wrote ABC on the first line\nwill give you ABC in $word if the user did this:\n\n\t#!/bin/sh\n        read word <file\n\nand not \"ABC^M\".  That is, \"we may produce only LF files, we consume\nLF files just fine, but we also treat CRLF files as text as their\nusers intended.\"\n\n> As I said, I am uninterested in arguing for arguing's sake.\n\nI already explained why this particular thing does not have anything\nto do with IFS but comes from the way your \"read\" behaves, which is\nnot in line with the way users on your platform expects.  Is that so\nhard to understand?\n\nAn argument you make to support adding stuff to IFS _is_ an argument\nfor arguing's sake.\n\n> What I really needed you to do indicate the way forward.\n\nYes, and I already did, didn't I?\n\nI already said that mucking with IFS is a wrong thing to do, and\nindicated that that is not the way forward.\n\nI also already said that fixing your read is the only sensible thing\nto do in the longer term iff you claim to support those who use\n\"certain applications\" that leave CRLF line terminator.\n\n * http://pubs.opengroup.org/onlinepubs/9699919799/utilities/read.html\n   defines the operation as a two-step process.  It is supposed to read\n   an input line and then \"The terminating <newline> (if any) shall\n   be removed from the input\".  And then \"the result shall be split into\n   fields\" (at IFS boundaries).\n\n   As POSIX does not say <newline> has to be a single octet with\n   value 10 (the same way you are allowed to show CRLF when you call\n   printf(\"\\n\"), and you do not see CR left at the end when you use\n   gets(3)).\n\n   So on a platform that (optionally) accepts CRLF as a line\n   terminator, \"read\" should read up to LF for a line, and if there\n   is a CR immediately before that LF, remove that CR, too.  Now you\n   got the result of \"terminating <newline> (if any) shall be\n   removed\".  Then look at \"the result\" and split into fields using\n   IFS.  That way, you still process LF terminated files correctly,\n   and you would never show a single ^M for an empty line, which\n   triggered this thread.\n\n * Optionally, you may want to implement the \"field splitting\" (at\n   IFS boundaries) with a small twist.  When the script/user\n   included LF in IFS, split at LF but remove CR if there is one\n   that immediately precedes the LF.\n\nAnd also I gave you a more localized (i.e. less likely to negatively\naffect unrelated parts of the system) workaround that we could use,\nuntil your \"read\" is fixed.\n\nMind you, I am not happy with that \"$cr may appear at the end of the\nlast token on the line\" workaround.  Other places in the code (both\ninside and outside Git scripted porcelains) that calls \"read\" to\nread from text files can suffer from the same issue, until it is\nfixed.  Covering up the bug in \"read\" by throwing extra characters\nto IFS, especially when read is not the only user of IFS, is not a\nfix.\n"},{"id":"272409","messageId":"cover.1446043983.git.johannes.schindelin@gmx.de","threadId":"40637","inReplyTo":"cover.1445939154.git.johannes.schindelin@gmx.de","subject":"[PATCH v4 0/2] Fix interactive rebase when the editor saves with CR/LF","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2015-10-28T14:54:14Z","receivedAt":"2015-10-28T14:54:14Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Chad Boles reported that `git rebase -i` recently started producing\nerrors when the editor saves files with DOS line endings. The symptom\nis:\n\n\tWarning: the command isn't recognized in the following line:\n\t -\n\n\tYou can fix this with 'git rebase --edit-todo'.\n\tOr you can abort the rebase with 'git rebase --abort'.\n\nThe real bummer is that simply calling `git rebase --continue` \"fixes\"\nit.\n\nTurns out that we now check whether a single Carriage Return is a valid\ncommand. This new check was introduced recently (1db168ee9, ironically\nnamed \"rebase-i: loosen over-eager check_bad_cmd check\").\n\nThe fix provided by Junio works around this issue by testing for an\nexplicit trailing carriage return and handles it like an empty line.\n\nUnfortunately, this is the best we can do for now as there is\ndisagreement about a more general fix.\n\nThis iteration clarifies the comments in git-rebase--interactive,\nupdates the commit message to state that this has been tested with Git\nfor Windows, and replaces the description of the proposed fix with a\ndescription of the actual work-around provided by Junio.\n\n\nJohannes Schindelin (1):\n  Demonstrate rebase fails when the editor saves with CR/LF\n\nJunio C Hamano (1):\n  rebase-i: work around Windows CRLF line endings\n\n git-rebase--interactive.sh    | 12 ++++++++++++\n t/t3404-rebase-interactive.sh | 12 ++++++++++++\n 2 files changed, 24 insertions(+)\n\nInterdiff vs v3:\n\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex daadf2d..34cfe66 100644\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -77,8 +77,7 @@ amend=\"$state_dir\"/amend\n rewritten_list=\"$state_dir\"/rewritten-list\n rewritten_pending=\"$state_dir\"/rewritten-pending\n \n-# Work around a Windows port of shell that does not strip\n-# the newline at the end of a line correctly.\n+# Work around Git for Windows' Bash that strips only LFs but no CRs.\n cr=$(printf \"\\015\")\n \n strategy_args=\n@@ -523,8 +522,8 @@ do_next () {\n \t\tmark_action_done\n \t\t;;\n \t\"$cr\")\n-\t\t# Windows port of shell not stripping the newline\n-\t\t# at the end of an empty line correctly.\n+\t\t# Work around Carriage Returns not being stripped (e.g. with\n+\t\t# Git for Windows' Bash).\n \t\tmark_action_done\n \t\t;;\n \tpick|p)\n@@ -906,8 +905,8 @@ check_bad_cmd_and_sha () {\n \t\t\t# Doesn't expect a SHA-1\n \t\t\t;;\n \t\t\"$cr\")\n-\t\t\t# Windows port of shell not stripping the newline\n-\t\t\t# at the end of an empty line correctly.\n+\t\t\t# Work around Carriage Returns not being stripped\n+\t\t\t# (e.g. with Git for Windows' Bash).\n \t\t\t;;\n \t\tpick|p|drop|d|reword|r|edit|e|squash|s|fixup|f)\n \t\t\tif ! check_commit_sha \"${rest%%[ \t]*}\" \"$lineno\" \"$1\"\n\n-- \n2.1.4\n"},{"id":"272410","messageId":"3d37a1348065d5902c829481db2124a52be0ca1a.1446043983.git.johannes.schindelin@gmx.de","threadId":"40637","inReplyTo":"cover.1446043983.git.johannes.schindelin@gmx.de","subject":"[PATCH v4 1/2] Demonstrate rebase fails when the editor saves with CR/LF","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2015-10-28T14:54:21Z","receivedAt":"2015-10-28T14:54:21Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Based on a bug report by Chad Boles.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n t/t3404-rebase-interactive.sh | 12 ++++++++++++\n 1 file changed, 12 insertions(+)\n\ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex 3de0b1d..5dfa16a 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -1261,4 +1261,16 @@ test_expect_success 'static check of bad SHA-1' '\n \ttest E = $(git cat-file commit HEAD | sed -ne \\$p)\n '\n \n+test_expect_failure 'editor saves as CR/LF' '\n+\tgit checkout -b with-crlf &&\n+\twrite_script add-crs.sh <<-\\EOF &&\n+\tsed -e \"s/\\$/Q/\" <\"$1\" | tr Q \"\\\\015\" >\"$1\".new &&\n+\tmv -f \"$1\".new \"$1\"\n+\tEOF\n+\t(\n+\t\ttest_set_editor \"$(pwd)/add-crs.sh\" &&\n+\t\tgit rebase -i HEAD^\n+\t)\n+'\n+\n test_done\n-- \n2.1.4\n"},{"id":"272411","messageId":"ecea378790275a22167f2434ba714a565971e622.1446043983.git.johannes.schindelin@gmx.de","threadId":"40637","inReplyTo":"cover.1446043983.git.johannes.schindelin@gmx.de","subject":"[PATCH v4 2/2] rebase-i: work around Windows CRLF line endings","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2015-10-28T14:54:48Z","receivedAt":"2015-10-28T14:54:48Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Junio C Hamano <gitster@pobox.com>\n\nEditors on Windows can and do save text files with CRLF line\nendings, which is the convention on the platform.  We are seeing\nreports that the \"read\" command in a port of bash to the environment\nhowever does not strip the CRLF at the end, not adjusting for the\nsame convention on the platform.\n\nThis breaks the recently added sanity checks for the insn sheet fed\nto \"rebase -i\"; instead of an empty line (hence nothing in $command),\nthe script was getting a lone CR in there.\n\nSpecial case a lone CR and treat it the same way as an empty line to\nwork this around.\n\nThis patch passes the test with Git for Windows, where the issue was\nseen first.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n git-rebase--interactive.sh    | 12 ++++++++++++\n t/t3404-rebase-interactive.sh |  2 +-\n 2 files changed, 13 insertions(+), 1 deletion(-)\n\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex d65c06e..34cfe66 100644\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -77,6 +77,9 @@ amend=\"$state_dir\"/amend\n rewritten_list=\"$state_dir\"/rewritten-list\n rewritten_pending=\"$state_dir\"/rewritten-pending\n \n+# Work around Git for Windows' Bash that strips only LFs but no CRs.\n+cr=$(printf \"\\015\")\n+\n strategy_args=\n if test -n \"$do_merge\"\n then\n@@ -518,6 +521,11 @@ do_next () {\n \t\"$comment_char\"*|''|noop|drop|d)\n \t\tmark_action_done\n \t\t;;\n+\t\"$cr\")\n+\t\t# Work around Carriage Returns not being stripped (e.g. with\n+\t\t# Git for Windows' Bash).\n+\t\tmark_action_done\n+\t\t;;\n \tpick|p)\n \t\tcomment_for_reflog pick\n \n@@ -896,6 +904,10 @@ check_bad_cmd_and_sha () {\n \t\t\"$comment_char\"*|''|noop|x|exec)\n \t\t\t# Doesn't expect a SHA-1\n \t\t\t;;\n+\t\t\"$cr\")\n+\t\t\t# Work around Carriage Returns not being stripped\n+\t\t\t# (e.g. with Git for Windows' Bash).\n+\t\t\t;;\n \t\tpick|p|drop|d|reword|r|edit|e|squash|s|fixup|f)\n \t\t\tif ! check_commit_sha \"${rest%%[ \t]*}\" \"$lineno\" \"$1\"\n \t\t\tthen\ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex 5dfa16a..98eb49a 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -1261,7 +1261,7 @@ test_expect_success 'static check of bad SHA-1' '\n \ttest E = $(git cat-file commit HEAD | sed -ne \\$p)\n '\n \n-test_expect_failure 'editor saves as CR/LF' '\n+test_expect_success 'editor saves as CR/LF' '\n \tgit checkout -b with-crlf &&\n \twrite_script add-crs.sh <<-\\EOF &&\n \tsed -e \"s/\\$/Q/\" <\"$1\" | tr Q \"\\\\015\" >\"$1\".new &&\n-- \n2.1.4\n"},{"id":"272412","messageId":"alpine.DEB.1.00.1510281556230.31610@s15462909.onlinehome-server.info","threadId":"40637","inReplyTo":"xmqqd1w0dy0k.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v3 2/2] rebase-i: work around Windows CRLF line endings","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2015-10-28T14:56:55Z","receivedAt":"2015-10-28T14:56:55Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Tue, 27 Oct 2015, Junio C Hamano wrote:\n\n> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n> \n> > My apologies: I forgot to spell out explicitly that this passes in Git\n> > for Windows 2.x' SDK: It does.\n> \n> Can you add that to the log message?  Your cover letter may also want to\n> be updated for those who fish for the last version posted in the list\n> archive.\n\nDone,\nJohannes\n"},{"id":"272420","messageId":"xmqqk2q6lxy7.fsf@gitster.mtv.corp.google.com","threadId":"40637","inReplyTo":"cover.1446043983.git.johannes.schindelin@gmx.de","subject":"Re: [PATCH v4 0/2] Fix interactive rebase when the editor saves with CR/LF","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-10-28T17:12:00Z","receivedAt":"2015-10-28T17:12:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <johannes.schindelin@gmx.de> writes:\n\n> Turns out that we now check whether a single Carriage Return is a valid\n> command. This new check was introduced recently (1db168ee9, ironically\n> named \"rebase-i: loosen over-eager check_bad_cmd check\").\n\nWill queue.\n\nThe root cause is not really \"a new check added recently\".  Earlier\nthe edited result was fed to stripspace and because \"stripspace\"\ndoes what a dos-to-unix filter that turns CRLF into LF in addition\nto cleaning up spaces and comments, we did not see the problematic\nbehaviour from \"read\" in this codepath.\n\nThanks.\n"}]}