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.

Sign up for GitHub

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

Jump to bottom

LF all the things #2378

Merged
merged 11 commits into from
Apr 26, 2018
Merged

LF all the things #2378

merged 11 commits into from
Apr 26, 2018

Conversation

Stanzilla
Copy link
Collaborator

the problem is since there never was a gitattributes file in the repo and no editorconfig enforced, every windows commiter potenially changed some files to CRLF, so this changes all crlf files back to lf.

not sure if best practice or not

@staticfox
Copy link
Member

staticfox commented Apr 22, 2018 via email

@Stanzilla
Copy link
Collaborator Author

that is actually something you can't even tell clang-format to do. At least I haven't been able to find out how.

@staticfox
Copy link
Member

staticfox commented Apr 22, 2018 via email

@Stanzilla
Copy link
Collaborator Author

I am no expert on .gitgnore but it is currently set up like this:

* text=auto
*.cpp text eol=lf
*.h text eol=lf

is this enough?

@IceN9ne
Copy link
Collaborator

IceN9ne commented Apr 23, 2018

What about utf-8 all the things too? It might be a good opportunity to do that as well if we want that. We have an assortment of binary, us-ascii, and utf-8 charsets for our .cpp and .h files.

@wodim
Copy link
Member

wodim commented Apr 23, 2018

We have an assortment of binary, us-ascii, and utf-8 charsets for our .cpp and .h files.

How is that possible? Remember that file is not a good assessment tool for this

@DarthGandalf
Copy link
Member

Note that ASCII is a subset of UTF-8

@wodim
Copy link
Member

wodim commented Apr 23, 2018

^ A file that doesn't contain accents or weird stuff could pass as a us-ascii file

@Stanzilla
Copy link
Collaborator Author

Can we settle the LF thing first, please? :D My .gitattributes does not cover all files we have in the repo, do we need more? Something like https://github.com/alexkaratarakis/gitattributes

@craftwar
Copy link
Contributor

* text=auto
You need only this line. Git will push all text files as LF to repo and checkout according to OS.

reference cross-platform project
https://github.com/obsproject/obs-studio

@Stanzilla
Copy link
Collaborator Author

Stanzilla commented Apr 24, 2018 •
edited
Loading

@craftwar that is what I thought before, then someone told me otherwise: vercel/hyper#2795 not sure if true or not. What yours will do is let your local Git installation decide on what to do, based on the core.autoCRLF setting, afaik.

@craftwar
Copy link
Contributor

@Stanzilla according to git document https://git-scm.com/docs/gitattributes
I think git will detect if it's a text or binary automatically.

add
*.nsi text
git or github think it's binary file

@wodim
Copy link
Member

wodim commented Apr 24, 2018

nsi are binary files because they are not written in utf-8. Don't convert them to utf-8!!!

@Stanzilla
Copy link
Collaborator Author

So what we should do is tell Git which files are binary so it ignores it, like that https://github.com/alexkaratarakis/gitattributes/blob/master/C%2B%2B.gitattributes

@wodim
Copy link
Member

wodim commented Apr 24, 2018

These should be tested. I am not sure if nsis works with utf-8 now, that's why I insisted on not converting them

@Stanzilla
Copy link
Collaborator Author

sure, just waiting for appveyor

@staticfox
Copy link
Member

This UTF-8 stuff probably should have been left to another PR since this PR was intended to be specific towards line endings. Now it seems like we're dealing with 2 unrelated issues.

@Stanzilla
Copy link
Collaborator Author

Yeah agree, but it's done now, I could still split it off if wanted

@staticfox
Copy link
Member

Eh, it's fine. Moving forward, let's just keep PRs specific towards the topic. Otherwise things will get backlogged due to unrelated debates.

@Stanzilla
Copy link
Collaborator Author

Build is green, any last words?

@Stanzilla Stanzilla merged commit 2ecd28f into kvirc:master Apr 26, 2018
@Stanzilla Stanzilla deleted the lf branch April 26, 2018 16:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@Stanzilla @staticfox @IceN9ne @wodim @DarthGandalf @craftwar @Dessa