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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Junio,

On Sun, 25 Oct 2015, Junio C Hamano wrote:

> Johannes Schindelin <johannes.schindelin@xxxxxx> 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.

> 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.

> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]