-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Replace Common Lisp grammar source #6728
Conversation
Signed-off-by: Qingpeng Li <43924785+qingpeng9802@users.noreply.github.com>
Signed-off-by: Qingpeng Li <43924785+qingpeng9802@users.noreply.github.com>
This comment was marked as spam.
This comment was marked as spam.
This comment was marked as spam.
This comment was marked as spam.
Signed-off-by: Qingpeng Li <43924785+qingpeng9802@users.noreply.github.com>
Why not use that repo directly? From a quick look it looks like that repo has the grammars in a format we can use in a location we expect to find grammar files… we'll find the *.tmLanguage.json files. I note that the commonlisp grammar hasn't been touched in a while so it's likely your update won't be picked up be linguist too. FYI, we look for these extensions: linguist/tools/grammars/compiler/loader_fs.go Lines 18 to 24 in 559a642
… in these locations: linguist/tools/grammars/compiler/loader.go Lines 119 to 128 in 559a642
|
This grammar aims to collaborate with the semantic token functionality of vscode. If two user groups use the same repo, potential GitHub users may request changes that are not suitable for upstreaming to the vscode repo. The demands of different user groups may break the expectations of the original functionality.
This grammar works fine for the current users from vscode. We respect and value the smoothness of user experience. Considering the lack of language feature support for lisp.tmbundle, we believe that changing it to a market-proof grammar can bring GitHub users more user experience value. |
Cool. I was suggesting using the same repo for both to make things easier for you, but it's fine if you prefer to keep them separate and duplicate things as needed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks.
Note: this PR will not be merged until close to when the next release is made. See here for more details.
Description
Things may be a bit more complicated than usual. This PR would like to replace the old grammar of Common Lisp with my grammar.
Common Lisp and other lisp dialects are currently using
source.lisp
as the scope. My grammar is strictly follow Common Lisp standard so it is not a good idea to use my grammar on other lisp dialects. Thus, the new grammar (source.commonlisp
) is added for Common Lisp only, but the old grammar (source.lisp
) is kept for other lisp dialects.The grammar is originally from my repo vscode-common-lisp.
Checklist: