Skip to content

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.
compnerd added 2 commits May 9, 2021 15:53
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.

3 participants