Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enforce "\n" line endings on Windows #574

Merged
merged 1 commit into from
Nov 1, 2017

Conversation

haferburg
Copy link
Contributor

With Windows line endings, which seems to be the default on Windows, the
zig compiler won't understand std out of the box. This project should
not rely on git's global core.autocrlf setting.

With Windows line endings, which seems to be the default on Windows, the
zig compiler won't understand std out of the box. This project should
not rely on git's global core.autocrlf setting.
@haferburg
Copy link
Contributor Author

haferburg commented Nov 1, 2017

Hi there, btw! :)

I've been following this project for a long time. Yesterday, I finally spent a couple hours building LLVM and Clang, and I got zig to compile out of the box. Awesome!

But I ran into trouble with hello.zig, because of line endings. First, zig wouldn't compile hello.zig. Fine. But then it wouldn't even compile std/bootstrap.zig or std/panic.zig. Because git automatically converts to Windows line endings on Windows. And I had the same problem when I tried to compile the tetris project.

My favorite solution would be for zig to work with all types of line endings. If you make a Windows executable that is run on Windows by Windows users, I don't see why they should be forced to use non-Windows line endings. Otherwise this is something that every zig project would need to do: Provide a .gitattributes file that makes sure that git won't convert to Windows line endings.

@andrewrk
Copy link
Member

andrewrk commented Nov 1, 2017

Cc @thejoshwolfe

@andrewrk
Copy link
Member

andrewrk commented Nov 1, 2017

One thing we can do is allow crlf but replace them with zig fmt (when it exists)

@andrewrk andrewrk merged commit b35689b into ziglang:master Nov 1, 2017
@andrewrk
Copy link
Member

andrewrk commented Nov 1, 2017

I'm conflicted about this because the autocrlf feature of git is a misfeature. Git is mangling binary files (how does it really know the difference between text and binary?) Git is also mangling text files. Even for projects other than Zig, the autocrlf feature is better left off. It only causes trouble.

@andrewrk
Copy link
Member

andrewrk commented Nov 1, 2017

https://stackoverflow.com/a/2825829/432

Unless you can see specific treatment which must deal with native EOL, you are better off leaving autocrlf to false.

@thejoshwolfe
Copy link
Contributor

@haferburg do you know why autocrlf is enabled for you? usually it's disabled. did you turn it on? did it solve some problem for you?

@haferburg haferburg deleted the line-endings branch November 3, 2017 20:37
@haferburg
Copy link
Contributor Author

@thejoshwolfe According to this http://adaptivepatchwork.com/2012/03/01/mind-the-end-of-your-line/, core.eol = native is the default setting, which means CRLF on Windows.

@thejoshwolfe
Copy link
Contributor

@haferburg yes. the defaults are core.eol = native, and core.autocrlf = false. The latter means that git is not going to convert/corrupt your files by default.

@thejoshwolfe
Copy link
Contributor

looks like if a repo contains a .gitattributes file with * -text, then that disables autocrlf for the repo regardless of anyone's global configuration. i haven't experimented to see if that also disables text diffs though. there maybe be some incantation we can do to make this issue irrelevant for the zig repo.

@haferburg
Copy link
Contributor Author

@thejoshwolfe Hi again! You asked why autocrlf was enabled, and I just found out. When installing the client from https://gitforwindows.org/, there is a wizard that recommends setting autocrlf to true, which is also the default setting in that wizard.

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

Successfully merging this pull request may close these issues.

3 participants