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

Deny invalid project name chars '\' & '/'' #5980

Closed
wants to merge 1 commit into from

Conversation

bew
Copy link
Contributor

@bew bew commented Apr 21, 2018

Fixes #4817

@straight-shoota
Copy link
Member

There are other characters that can't appear in a file name (differs slightly depending on OS). I suggest to have a method in File (or later Path) to check if a string is a valid path component.

This SO answer looks quite comprehensive: https://stackoverflow.com/a/31976060/7401689

@bew
Copy link
Contributor Author

bew commented Apr 21, 2018

Also, instead of having a blacklist, we could go the other way and have a whitelist?
This is the project name, this name will be used (with some modifications) for the module name in the generated code

@bew
Copy link
Contributor Author

bew commented Jun 4, 2018

Thinking more about this, the whitelist solution will never be complete, as almost any unicode char is allowed in a module name (like A€あ).

So either:

  • the tool only handles a subset of what's possible, by allowing only alphanumeric + dash & underscore
  • we try to list all the unsupported chars (like >, :, /, {, }, ....) and test each of them to make sure the name is fully correct.

@straight-shoota
Copy link
Member

I don't see a reason to artificially restrict the usage of arbitrary unicode characters in file names. So it should only check against a blacklist of (potentially) invalid characters.

@petoem
Copy link
Contributor

petoem commented Jun 12, 2018

@straight-shoota Nice idea! We can add a list of invalid chars to File or later Path and have a method check if a string is valid.
dotnet has GetInvalidFileNameChars and GetInvalidPathChars, both for unix and windows.

@straight-shoota
Copy link
Member

Closing in favor of #8737
Thank you nevertheless @bew

@bew bew deleted the deny-invalid-project-name branch February 18, 2020 16:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:feature pr:needs-work A PR requires modifications by the author. topic:tools
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bad fs structure with crystal init lib /path/to/project
3 participants