-
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 SourcePawn grammar #6635
Conversation
A couple more things: 1I would also like to point out my fully functional tree-sitter grammar for Sourcepawn here. 2The textmate grammar supports JSDoc injections, is there something I should do to add support to that in Linguist as well? 3This repo can be deleted now: https://github.com/github-linguist/sublime-sourcepawn Thanks for taking the time to consider this PR! |
Yes
No
I'm not sure what there would be to add. Linguist only acts as a collection point which bundles them up for the syntax highlighting engine which is what reads the grammars. No processing of the files takes place other than some basic PCRE regex checking and then conversion to JSON if not already in JSON format. We expect the grammar file to be in its final and complete state as no other processing takes place. |
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.
Please use the script/add-grammar
script to replace the grammar as documented in the CONTRIBUTING.md file, and don't manually modify any of the files updated by the script.
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.
Done 👍
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.
🤔 there should be more changes than this. Your repo has several grammar scope names so the grammars.yml
file should have been updated to include these. The cached license file should also have changed as the commits are different. Are you sure you've added all the changed files?
I reran the script just to be sure, here are the commands I ran: > script/add-grammar --replace "sourcepawn-vscode" https://github.com/Sarrus1/sourcepawn-vscode
Checking Docker is installed and running
Deregistering submodule: vendor/grammars/sourcepawn-vscode
Cleared directory 'vendor/grammars/sourcepawn-vscode'
Submodule 'vendor/grammars/sourcepawn-vscode' (https://github.com/Dreae/sourcepawn-vscode) unregistered for path 'vendor/grammars/sourcepawn-vscode'
rm 'vendor/grammars/sourcepawn-vscode'
Registering new submodule: https://github.com/Sarrus1/sourcepawn-vscode.git
Reactivating local git directory for submodule 'vendor/grammars/sourcepawn-vscode'
latest: Pulling from linguist/grammar-compiler
Digest: sha256:fcaf08a6e27d3d88212d9ca8aceee30d4089499a77bb05307871febd8073790f
Status: Image is up to date for linguist/grammar-compiler:latest
docker.io/linguist/grammar-compiler:latest
What's Next?
1. Sign in to your Docker account → docker login
2. View a summary of image vulnerabilities and recommendations → docker scout quickview linguist/grammar-compiler:latest
WARNING: The requested image's platform (linux/amd64) does not match the detected host platform (linux/arm64/v8) and no specific platform was requested
OK! added grammar source 'vendor/grammars/sourcepawn-vscode'
new scope: source.sourcepawn
Caching grammar license
Caching dependency records for linguist
git_submodule
Using AL (96c407663984341b21e574a58ef6ca6f89cc3fc0)
Using Alloy.tmbundle (0a80fc74e921325da23264889771a88a02877414)
Using Assembly-Syntax-Definition (519b7b3c5f8428832dde72a90041d0707f831e5c)
.... # cropping some of it
* 451 git_submodule dependencies
Updating grammar documentation in vendor/README.md
> git status
On branch master
Your branch is behind 'origin/master' by 1 commit, and can be fast-forwarded.
(use "git pull" to update your local branch)
Changes to be committed:
(use "git restore --staged <file>..." to unstage)
modified: .gitmodules
Changes not staged for commit:
(use "git add <file>..." to update what will be committed)
(use "git restore <file>..." to discard changes in working directory)
modified: .gitmodules
modified: Brewfile
modified: vendor/README.md
> git add --all
> git commit -m "fix: replace SourcePawn grammar"
> git push -f The grammar is located in this folder which has a parent directory with an MIT License. Maybe the location is a problem? |
Hello @lildude I think I finally managed to make the script work properly. All the changes should be in order now 👍 |
Hello, bumping this. Is there anything else I can do to improve this PR? (I will start by rebasing) |
There’s no rush and not need to rebase as this PR won’t be merged until close to the next release anyway and that will include a merge update if 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.
Sorry I missed this earlier, but we can not accept this change.
Whilst the grammar files are licensed under MIT, the over-all repo is licensed by GPL-3 which means we'll potentially be shipping code that is covered by this license which we can't do.
I'm also not sure GPL3 allows sub-licensing.
Should I move my grammar to a new repo to solve the issue? |
That would probably be best short of relicensing the current repo, but that's entirely up to you. We can't accept the current repo as it stands with the GPL-3 license. |
Changing the License to MIT is fine with me 👍 |
Yes. Re-running |
All done 👍 |
The changes to |
I did the manual change in a previous commit and just reran the script thinking it would overwrite it. I have now reran the script again and made sure that it edited the files correctly. |
I updated the commit as I had to force push a change on the main branch of the repo. |
I'm not sure what I should do to get the script to recognize that the repo has an MIT license instead of "others". |
I think everything should be in order now! The commit version is correct 👍 |
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.
Replace SourcePawn grammar
Description
Dreae's grammar is no longer maintained and is missing a few Sourcepawn core features. This PR replaces it with mine, which now properly handles the old and new syntax, most importantly type highlighting for variable declarations.
Checklist: