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

fix: add .gitattributes file and normalize all the line endings #301

Merged
merged 4 commits into from
Jan 12, 2024

Conversation

adiati98
Copy link
Member

Description

Windows users encounter lint error of Expected linebreaks to be 'LF' but found 'CRLF' because the default behavior of git on Windows systems is to convert LF linebreaks to CRLF when checking out files, but to store the linebreaks as LF when committing a change.

This PR adds a .gitattributes file and normalize all line endings in .ts files to make it easy for developers to contribute from different platforms.

What type of PR is this? (check all applicable)

  • 🍕 Feature
  • 🐛 Bug Fix
  • 📝 Documentation Update
  • 🎨 Style
  • 🧑‍💻 Code Refactor
  • 🔥 Performance Improvements
  • ✅ Test
  • 🤖 Build
  • 🔁 CI
  • 📦 Chore (Release)
  • ⏩ Revert

Related Tickets & Documents

Fixes #299

Mobile & Desktop Screenshots/Recordings

No more Expected linebreaks to be 'LF' but found 'CRLF' error in .ts and .tsx files.

fix lf

Steps to QA

Added to documentation?

  • 📜 README.md
  • 📓 docs.opensauced.pizza
  • 🍕 dev.to/opensauced
  • 📕 storybook
  • 🙅 no documentation needed

[optional] Are there any post-deployment tasks we need to perform?

[optional] What gif best describes this PR or how it makes you feel?

@adiati98 adiati98 changed the title Bug: Add .gitattributes file and normalize all the line endings fix: Add .gitattributes file and normalize all the line endings Dec 20, 2023
@adiati98
Copy link
Member Author

adiati98 commented Dec 20, 2023

@jpmcb if you don't want to add .gitattributes file, I just found another solution to toggle "Select End of Line Sequence" from 'CRLF' to 'LF' on VS Code. I've tested it out and it works.

As Windows still store the linebreaks as 'LF' when committing a change just like on Mac and Linux (see quote from ESLint docs below), toggling the selection can fix this problem.

... the default behavior of git on Windows systems is to convert LF linebreaks to CRLF when checking out files, but to store the linebreaks as LF when committing a change.

Alternative Solution to this PR

The solution above is worth mention in the README of all projects that are using ESLint, or in the OpenSauced's docs under the "Applying Lint Styleguide" section that we can refer to if any Windows user encounter this problem. Because the challenge with us is that we can get more than 80 errors of Expected linebreaks to be 'LF' but found 'CRLF' in a file that can cause us missing other type of lint errors if any.

@nickytonline
Copy link
Member

I tested this locally and ran npm run lint with no issues after adding a test commit. @jpmcb, wouldn't we want this for all textual based files instead of just *.ts and *.tsx?

@adiati98
Copy link
Member Author

@nickytonline On Windows, there would be no problem after committing the files as it stores the linebreaks as 'LF' like on Mac and Linux.

The error occurs when working on changes because Windows converts 'LF' linebreaks to 'CRLF' when checking out files. It's challenging to spot other linting problems when there are more than 80 linebreaks errors in a file.

The reason I add rules specifically for .ts and .tsx is what's written in this thread under the "Recommendation".

... We had a repo with * text=auto, and Git guessed wrong for an image file that it was a text file, causing it to corrupt it as it replaced CR + LF bytes with LF bytes in the object database.

@nickytonline
Copy link
Member

@nickytonline On Windows, there would be no problem after committing the files as it stores the linebreaks as 'LF' like on Mac and Linux.

The error occurs when working on changes because Windows converts 'LF' linebreaks to 'CRLF' when checking out files. It's challenging to spot other linting problems when there are more than 80 linebreaks errors in a file.

The reason I add rules specifically for .ts and .tsx is what's written in this thread under the "Recommendation".

... We had a repo with * text=auto, and Git guessed wrong for an image file that it was a text file, causing it to corrupt it as it replaced CR + LF bytes with LF bytes in the object database.

I didn't necessarily mean the auto option, more along the lines of adding *.json, *.js, *.md and whatever other ones we deem valid for this fix.

@adiati98
Copy link
Member Author

@nickytonline oh, I see. I'll replace the *.ts and *.tsx with *.text tomorrow 😊

Copy link
Member

@jpmcb jpmcb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another way to do this from a windows dev machine without having to add the config to the project:

git config --global core.autocrlf true

which sets the global git config so that all LF endings will be converted to CRLF.


But this seems fair too: my usual preference is for projects to reduce the amount of individual editor config settings they have and be as agnostic as possible. For example, it'd be weird for me to add abunch of lua to this project that may be useful for navigating and work in this project with neovim.

Either way, if we want to add this, it could be helpful for people coming to the project on windows who haven't configured that yet.

.gitattributes Outdated Show resolved Hide resolved
Copy link
Member

@nickytonline nickytonline left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @adiati98! :shipit:

@nickytonline nickytonline changed the title fix: Add .gitattributes file and normalize all the line endings fix: add .gitattributes file and normalize all the line endings Jan 2, 2024
@Anush008 Anush008 merged commit a6279ea into open-sauced:beta Jan 12, 2024
13 checks passed
github-actions bot pushed a commit that referenced this pull request Jan 12, 2024
## [1.14.3-beta.2](v1.14.3-beta.1...v1.14.3-beta.2) (2024-01-12)

### 🐛 Bug Fixes

* add `.gitattributes` file and normalize all the line endings ([#301](#301)) ([a6279ea](a6279ea))
* chunk processing in Chat component ([#289](#289)) ([f1c29ff](f1c29ff))
Copy link

🎉 This PR is included in version 1.14.3-beta.2 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

github-actions bot pushed a commit that referenced this pull request Jan 30, 2024
## [1.15.0](v1.14.2...v1.15.0) (2024-01-30)

### 🐛 Bug Fixes

* add `.gitattributes` file and normalize all the line endings ([#301](#301)) ([a6279ea](a6279ea))
* change the classname and remove extra code ([241c385](241c385))
* chunk processing in Chat component ([#289](#289)) ([f1c29ff](f1c29ff))
* elemenet to the dom if it's not added already ([c654b82](c654b82))
* Link to Usage Guide and Update README ([#298](#298)) ([ba5cef6](ba5cef6))

### 🍕 Features

* enable AI description generator on comments ([#285](#285)) ([a30380b](a30380b))
* Upgrade to v2 API ([693d459](693d459))
Copy link

🎉 This PR is included in version 1.15.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

Bug: Lint error: Expected linebreaks to be 'LF' but found 'CRLF'
4 participants