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] Windows development fixes #3458

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

HamsterExAstris
Copy link

@HamsterExAstris HamsterExAstris commented Jan 21, 2024

Prettier forces all files to have Unix (LF) line endings. When a Windows user has core.autocrlf set to true in their global .gitconfig, commits use LF line endings but the files on disk use CRLF. This leads to spurious Prettier differences, causing the commit hook to fail.

This overrides that global setting, using LF endings for text files on disk regardless of the core.autocrlf setting.

It also fixes some tests that assumed Unix directory separators.


Use the following Checklist if you have changed something on the Backend or Frontend:

  • Tested the feature and it's working on a current and clean install.
  • Tested the main App features and they are still working on a current and clean install. (Login, Install, Play, Uninstall, Move games, etc.)
  • Created / Updated Tests (If necessary)
  • Created / Updated documentation (If necessary)

@flavioislima
Copy link
Member

flavioislima commented Jan 21, 2024

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@HamsterExAstris
Copy link
Author

I have read the CLA Document and I hereby sign the CLA

@HamsterExAstris HamsterExAstris changed the title Enforce LF endings on checkout for all platforms. Enforce LF endings on Windows. Jan 21, 2024
@HamsterExAstris HamsterExAstris changed the title Enforce LF endings on Windows. [Fix] Windows development fixes Jan 22, 2024
@@ -29,7 +30,7 @@ describe('Utilities - Rest', () => {
expect(() => {
unlinkFile(__dirname)
}).toThrowError(
`Couldn't remove ${workDir}/src/backend/wine/manager/downloader/__tests__/utilities!`
`Couldn't remove ${workDir}${path.sep}src${path.sep}backend${path.sep}wine${path.sep}manager${path.sep}downloader${path.sep}__tests__${path.sep}utilities!`
Copy link
Collaborator

Choose a reason for hiding this comment

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

we should use path.join here like path.join(workDir, 'src', 'backend', ...) instead interpolating path.sep, it's easier to read and should take care of using the right separator

Copy link
Author

Choose a reason for hiding this comment

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

Thanks, that is much more readable!

unzipDir: installDir,
onProgress: progress
})
).resolves.toStrictEqual(
`Succesfully unzip ${workDir}/src/backend/wine/manager/downloader/__tests__/utilities/../test_data/test.tar.xz to ${workDir}/src/backend/wine/manager/downloader/__tests__/utilities/test_unzip.`
`Succesfully unzip ${path.join(
Copy link
Member

Choose a reason for hiding this comment

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

was this needed to fix the issue with the prettier as well?
seems unrelated.

@flavioislima
Copy link
Member

Question: this supposed to fix the issue where when we try yarn prettier-fix it changed the permission for all files in the repo?

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