git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v2 0/2] Fix interactive rebase when the editor saves with CR/LF

From
Johannes Schindelin <johannes.schindelin@gmx.de>
Date
Oct 26, 2015, 10:43 UTC
Message-ID
<alpine.DEB.1.00.1510261133030.31610@s15462909.onlinehome-server.info>
In-Reply-To
<xmqqpp02kbif.fsf@gitster.mtv.corp.google.com>
Hi Junio,
On Sun, 25 Oct 2015, Junio C Hamano wrote:
Show 19 quoted lines
> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> 
> > Chad Boles reported that `git rebase -i` recently started producing
> > errors when the editor saves files with DOS line endings. The symptom
> > is:
> >
> > 	Warning: the command isn't recognized in the following line:
> > 	 -
> >
> > 	You can fix this with 'git rebase --edit-todo'.
> > 	Or you can abort the rebase with 'git rebase --abort'.
> >
> > The real bummer is that simply calling `git rebase --continue` "fixes"
> > it.
> 
> Two questions.
> 
>  * What does the DOS editor do to a file with ^M in the middle of a
>    line, e.g. "A^MB^M^J"?

You mean mixed line endings? At least with this Windows 10 WordPad, *all* line endings are normalized to CR/LF. That is, if you edit a file that contains a stray CR, it is presented as a line break.

>  * Is your shell ported correctly to the platform?

I would think so. It is an MSys2 program, therefore it uses all the POSIX niceties, of course.

A simple test with CR/LF line endings in a script reveals that it is pretty solid:

	x=a
	case "$x" in a) echo b;; esac
prints out 'b', as expected.

Please also note that things apparently worked alright before the patch removing the stripspace call was applied.

Show 10 quoted lines
> The latter may need a bit of elaboration.  "read a b c" is supposed
> to read one line of text (where the definition of line is platform
> dependent, your platform may use CRLF to signal the end of an line),
> split the characters on the line (i.e. LF vs CRLF no longer matters
> at this point) at $IFS characters and give them to $a $b and $c. If
> the platform accepts CRLF as the EOL signal, should the program still
> see CR at the end of $c?
> 
> A solution that mucks with IFS smells like fixing a wrong problem
> without addressing the real cause.

Please note that it is a bit unsettling if you use funny language like "smells" here and then accuse me of not having an argument when I point that the same rationale applies to having CR in IFS as applies to having LF in IFS. Yes, that was an implicit argument, but it is a strong one, so I do not think you are well served ignoring it.

Show 9 quoted lines
> Also IFS is used not only by "read", so munging it globally doubly
> feels wrong.
> 
> In addition, you do not want to split at CR; what you want is to
> treat CRLF (i.e. not a lone CR) as the end-of-line separator.
> Adding CR to IFS feels triply wrong.
> 
> By the way, saying "This is right, really" makes you sound as if you
> do not have a real argument.

Again. If CR has no place in IFS, why does LF have a place in IFS? It makes *no* sense to argue for one and against the other.

In any case, I am not interested in arguing for arguing's sake. Tell me what you want instead of the patch to IFS, I will implement it and test whether it fixes the bug and we will move on.

Ciao, Johannes

Previous: Junio C HamanoNext: Junio C Hamano
Message 17 of 31 in “Fix interactive rebase when the editor saves with CR/LF”
  1. 0/2 Fix interactive rebase when the editor saves with CR/LFJohannes Schindelin, Oct 25, 2015
  2. 1/2 Demonstrate rebase fails when the editor saves with CR/LFJohannes Schindelin, Oct 25, 2015
  3. 2/2 sh-setup: explicitly mark CR as a field separatorJohannes Schindelin, Oct 25, 2015
  4. Philip OakleyOct 25, 2015
  5. Johannes SchindelinOct 25, 2015
  6. 0/2 Fix interactive rebase when the editor saves with CR/LFJohannes Schindelin, Oct 25, 2015
  7. 1/2 Demonstrate rebase fails when the editor saves with CR/LFJohannes Schindelin, Oct 25, 2015
  8. 2/2 sh-setup: explicitly mark CR as a field separatorJohannes Schindelin, Oct 25, 2015
  9. Matthieu MoyOct 26, 2015
  10. Junio C HamanoOct 26, 2015
  11. Junio C HamanoOct 26, 2015
  12. Johannes SchindelinOct 27, 2015
  13. Johannes SchindelinOct 27, 2015
  14. Junio C HamanoOct 27, 2015
  15. Matthieu MoyOct 27, 2015
  16. Junio C HamanoOct 25, 2015
  17. Johannes SchindelinOct 26, 2015
  18. Junio C HamanoOct 26, 2015
  19. Johannes SchindelinOct 27, 2015
  20. 0/2 Fix interactive rebase when the editor saves with CR/LFJohannes Schindelin, Oct 27, 2015
  21. 1/2 Demonstrate rebase fails when the editor saves with CR/LFJohannes Schindelin, Oct 27, 2015
  22. 2/2 rebase-i: work around Windows CRLF line endingsJohannes Schindelin, Oct 27, 2015
  23. Johannes SchindelinOct 27, 2015
  24. Junio C HamanoOct 27, 2015
  25. Johannes SchindelinOct 28, 2015
  26. 0/2 Fix interactive rebase when the editor saves with CR/LFJohannes Schindelin, Oct 28, 2015
  27. 1/2 Demonstrate rebase fails when the editor saves with CR/LFJohannes Schindelin, Oct 28, 2015
  28. 2/2 rebase-i: work around Windows CRLF line endingsJohannes Schindelin, Oct 28, 2015
  29. Junio C HamanoOct 28, 2015
  30. Johannes SchindelinOct 25, 2015
  31. shell scripting woes, was Re: [PATCH 2/2] sh-setup: explicitly mark CR as a field separatorJohannes Schindelin, Oct 25, 2015

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.