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

Support legal file names with a dollar notation like $.js or a$.js #4792

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

aleen42
Copy link

@aleen42 aleen42 commented Jan 6, 2023

This PR contains:

  • bugfix
  • feature
  • refactor
  • documentation
  • other

Are tests included?

  • yes (bugfixes and features will not be merged without tests)
  • no

Breaking Changes?

  • yes (breaking changes will not be merged unless absolutely necessary)
  • no

List any relevant issue numbers:

Description

Hope not to sanitize file names like $.js, or a$.js which is legal in all OS systems.

@lukastaegert
Copy link
Member

legal in all OS systems

Others disagree: https://www.mtu.edu/umc/services/websites/writing/characters-avoid/

Why does everyone make PRs to change the defaults instead of passing your own sanitizer function via config?

@aleen42
Copy link
Author

aleen42 commented Jan 6, 2023

@lukastaegert at least I have tested under Windows, Linux, and Linux-based MacOS. $ may be a common signature in a file name, and I think it is better to allow it as a default behaviour, rather than making everyone disable sanitiser or setting a filter. Especially when meeting some developers trying to migrate setting from Webpack to Rollup. It is strange behaviour that everyone needs to trace where the name is changed by someone.

@lukastaegert
Copy link
Member

@lukastaegert
Copy link
Member

And there are very good reasons not to have $ in a file name under MacOS and Linux, namely accidental environment variable interpolation in scripts. But settings the above option to false should fix everything.

@aleen42
Copy link
Author

aleen42 commented Jan 9, 2023

@lukastaegert I knew the option. But environment variables only take effect when naming with $x, it is safe for $ only and x$, and I think it will be smarter when handling these two situations:

touch $.js # OK in MacOS
touch x$.js # OK in MacOS
touch $x.js # not OK

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.

None yet

2 participants