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

Update paths #3

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

DavidArchibald
Copy link

Updates path Regexes to permit these valid paths:

  • foo.js/
  • foo\bar.js

Maybe these aren't possible due to stylistic concerns. Fortunately any of these can be reverted fairly easily.

Disallows these:

  • ./../foo.js
  • ./a/../../lorem.js
  • ./a/b/c/../bar.js - This is actually a false positive, it resolves into a/b/bar.js which doesn't traverse up any directories but probably is an anti-pattern anyways

I actually found this as I was looking for an answer to a question. I want to allow an input module.json/system.json to contain paths to ts, scss, sass, etc. files. Is there a way to keep strict mode and get this?

@typhonrt
Copy link
Contributor

I'll also take a look at this in the Q2 time frame. The one thing I think I noticed is that you made this 2nd PR from the same repo that you made the first PR, so it is including both the lower case code and the regex changes that you intended in this PR. You might consider creating a separate fork per PR. I know that can be annoying as Github doesn't allow you to have multiple forks per organization / account, etc.

See in the files changed that it shows the lower case changes too.

@DavidArchibald
Copy link
Author

Haha, sorry about that, clearly wasn't paying enough attention. I'll quickly do some Git work to remove the lowercase problem.

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.

2 participants