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 #2433: spaces not being allowed in Paths on Windows #2434

Merged
merged 2 commits into from
Nov 16, 2021

Conversation

jchyb
Copy link
Contributor

@jchyb jchyb commented Nov 12, 2021

According to the Windows documentation, merely characters with codes 0-31 are considered to be the control characters forbidden for paths. Since spaces (of code 32) are allowed, they were removed from the check.

A test for the issue was also added.

Acording to Windows documentation, merely characters with codes 0-31 are
considered to be the forbidden control characters for paths. Since
spaces (of code 32) are allowed, they were removed from the check.

A test for the issue was also added.
def isSpaceOrReservedChar(c: Char) = {
c.isSpaceChar || pathReservedChars.contains(c)
def isControlOrReservedChar(c: Char) = {
c < '\u0020' || pathReservedChars.contains(c)
Copy link
Member

@ekrich ekrich Nov 12, 2021

Choose a reason for hiding this comment

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

I would consider either val space = '\u0020' or leaving a comment at this location to make this a touch more readable. Other than that this looks great.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, good idea

Copy link
Member

@ekrich ekrich left a comment

Choose a reason for hiding this comment

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

LGTM, a better change than my idea.

@WojciechMazur WojciechMazur merged commit 3c37e76 into scala-native:master Nov 16, 2021
WojciechMazur pushed a commit to WojciechMazur/scala-native that referenced this pull request Nov 27, 2021
…cala-native#2434)

* Fix spaces not being allowed on Paths on Windows

Acording to Windows documentation, merely characters with codes 0-31 are
considered to be the forbidden control characters for paths. Since
spaces (of code 32) are allowed, they were removed from the check.

A test for the issue was also added.

* Make char exclusion more clear

(cherry picked from commit 3c37e76)
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