Don't use idents as ignore dirs or asset dirs #1
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Member
|
I see we documented that identifiers are also valid entries in ignore dirs, but it probably doesn't make sense to do? It feels kind of misleading to have a variable looking piece of syntax act like a string. |
Member
Author
|
Yeah, you could be right about that. It's the same for the assets directory, too. Do you think it would be better to disallow everything except literal strings for both? |
Member
|
Yes. Cc @dodomorandi |
ThinkerDreamer
changed the title
ThinkerDreamer
force-pushed
the
handle-idents-as-ignore-dirs
branch
from
01a73c2 to
03ceaac
Compare
Member
|
The fix commit should be |
ThinkerDreamer
force-pushed
the
handle-idents-as-ignore-dirs
branch
from
03ceaac to
e034453
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I found that we weren't actually allowing valid Rust idents as ignored directories, and I remember we didn't test this in the integration tests because we didn't have any valid identifiers to use in that scope, so I wrote some unit tests inside the macro crate to test it there.