-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Avoid line ending inconsistencies #3231
Conversation
@ddbeck This article might be helpful: https://www.edwardthomson.com/blog/advent_day_1_gitattributes_for_text_files.html Essentially, if we add a
|
@connorshea thanks for the link! I think that's a good idea too. I've added that to the PR and updated the title to reflect the slightly broadened scope of the changes here. Thank you! |
Co-Authored-By: ddbeck <[email protected]>
This looks like a great idea to me. The code looks good to me, too. |
Fortunately, there's not a new test here. It only changes the output of an existing test. Also fortunately, I did test renormalizing the line endings, and all of the JSON currently in the repo have the proper line endings already. I just re-tested this on |
Ahh yes, this is no new test strictness, but reporting is improved! Previously, this would have been reported as:
And now it is reported as:
Sorry for being slow! Just getting back to work after the holidays 😆 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Love this, it came up a few times in PRs already I think. Great work @ddbeck and thanks for the helpful comments everyone!
Good idea to have some sample output in here, @Elchi3. I don't know why I didn't think of that. 🤦♂️ But yes, that is exactly right. And welcome back! |
Prompted by #3224, this PR replaces line endings and some other invisible characters in the linter diff output with their respective escape sequences.
For example, if you have CRLF line endings in a JSON file, then the resulting linter diff makes it seem as if both the expected and actual lines are the same.
Unfortunately, I suspect my implementation is rather naive. I really want to believe there's some kind of regex trickery that would generalize this to any invisible character. I did some fruitless searching, then gave up and did the simplest thing that worked for me. 🤷♂️