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

Use slash to allow testing on Windows #215

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

compnerd
Copy link
Contributor

@compnerd compnerd commented May 9, 2021

Normalize the paths to the expected value on Windows when running swift-format. This has no bearing on the action itself, just is a clean up required to get tests to work on Windows.

This removes the trailing `,` to allow the warnings to be listed
similarly.  This is purely a stylistic change intended to help simplify
future changes.
When testing on Windows, the tool currently prints paths with a `/` path
separator rather than the canonical `\` separator for the platform.  The
resulting path is valid and usable.  However, this causes match failures
due to the difference in the path separator.  The `slash` module permits
us to normalize the path for the tests.  This should have no impact when
running on Unix platforms, but allows greater portability to Windows.
Add a dependency on slash for development purposes.
@compnerd
Copy link
Contributor Author

@ocean90 I cannot figure out what the lint issue here is. Could you please help me understand what is going on here?

@ocean90
Copy link
Member

ocean90 commented May 11, 2021

npm run format:fix should fix it for you. I'm assuming it's because of the line length.

"file1.swift",
)}:7:1: warning: [Indentation] replace leading whitespace with 2 spaces`;
const warning4 = `${join(dir, "file1.swift")}:7:23: warning: [Spacing]: add 1 space`;
const warning1 = `${slash(join(dir, "file2.swift"))}:2:22: warning: [DoNotUseSemicolons]: remove ';'`;
Copy link
Member

Choose a reason for hiding this comment

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

Is there a specific reason why we can't use joinDoubleBackslash()?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, joinDoubleBackslash will replace \ with \\. This will replace \ and \\ with /. They are preforming different operations. This is needed because the tooling currently prints out the path with / as the path separator on Windows rather than the canonical \.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ocean90 ping?

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.

None yet

3 participants