Remove lower-case requirement for language/grammar #4362
Merged
Conversation
This file contains 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
|
One more to remove here: Line 17 in 418a622 |
kleinweby
force-pushed
the
remove-lowercase-grammar-requirement
branch
from
5a06fff to
1956564
Compare
|
Ah thanks. I've removed the missing to lowercase. Two of the tests actually used this lowercase conversion. I've simply changed them. I've also checked |
kleinweby
force-pushed
the
remove-lowercase-grammar-requirement
branch
from
1956564 to
0e029e0
Compare
|
I missed this in the review of the PR that added it but there's a real test failure now from the uppercasing on this line: helix/helix-core/src/syntax.rs Line 2278 in e25af1f |
added 2 commits
October 21, 2022 08:45
Currently we always lower-case the grammar name when loading it. While it is somewhat of an convention to name tree-sitter grammars in lowercase there is no rule to enforce it. This patch removes the lower-casing to allow all possible grammar names. Signed-off-by: Christian Speich <[email protected]>
Just like for grammars we currently force a lower-case of the name for some actions (like filesystem lookup). To make this consistent and less surprising for users, we remove this lower-casing here. Note: it is still the preferred way to name both language and grammar in lower-case Signed-off-by: Christian Speich <[email protected]>
kleinweby
force-pushed
the
remove-lowercase-grammar-requirement
branch
from
0e029e0 to
66ccfcc
Compare
|
Sure, I rebased the PR and lowercased the failing test. |
the-mikedavis
approved these changes
Oct 21, 2022
|
Thanks! |
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.
As described in #4346 both language and grammar names are sometimes lower-cased. For grammars this is incompatible with ones using uppercase names. And for languages this can be a bit surprising. (At least I stumbled across helix ignoring my queries because of this).
Note that the syntax part is not strictly necessary to get grammars as described in the issue to work.
Closes #4346