Skip to content
This repository has been archived by the owner on Nov 9, 2017. It is now read-only.

Pt/native crlf (updated test) #87

Closed
wants to merge 2 commits into from
Closed

Pt/native crlf (updated test) #87

wants to merge 2 commits into from

Conversation

bricelam
Copy link

A minor patch to the previous pull request. This version updates the eol=native test to assert that the files use crlf after the conversion.

patthoyts and others added 2 commits August 13, 2013 22:40
The previous patch correctly points out that the NATIVE_CRLF setting is
incorrectly set on Mingw git. However, the Makefile variable is not
propagated to the C preprocessor and results in no change. This patch
pushes the definition to the C code and adds a test to validate that
when core.eol as native is crlf, we actually normalize text files to this
line ending convention when core.autocrlf is false.

Signed-off-by: Pat Thoyts <patthoyts@users.sourceforge.net>
@buildhive
Copy link

MSysGit - the development behind Git for Windows » git #115 SUCCESS
This pull request looks good
(what's this?)

@bricelam
Copy link
Author

"! has_cr" is used in this test suite to assert that the lines do not end in CR and, therefore, must end in LF (preceded by a CR). I agree that it is counter-intuitive, but I just matched the style of the tests above.

To answer your questions, CRLF endings are applied to fileunix, and the CRs are not stripped from filedos. It is just asserted in a very odd way.

@bricelam bricelam mentioned this pull request Aug 14, 2013
@dscho
Copy link
Member

dscho commented Aug 14, 2013

Oh, that is indeed a bummer. We should fix has_cr to succeed if it finds carriage returns.

@dscho
Copy link
Member

dscho commented Aug 14, 2013

Actually, I tested this on Linux, and I fear that has_cr does succeed if I put carriage returns through it:

$ has_cr() { tr '\015' Q | grep Q >/dev/null; }
$ printf '\r\n' | has_cr && echo yep
yep
$ printf '\n' | has_cr && echo yep

@dscho
Copy link
Member

dscho commented Aug 14, 2013

I do realize that above definition lacks a <"$1" but that is intentional, for the test, obviously.

@bricelam
Copy link
Author

I'll look into making the test better.

@bricelam bricelam closed this Aug 14, 2013
@bricelam bricelam deleted the pt/native-crlf branch August 14, 2013 23:52
@bricelam
Copy link
Author

Doh, I just realized I was investigating the wrong failures. I assumed that /share/msysGit/run-tests.sh would also build, but it doesn't. This test passes just fine after building. :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants