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

Add grammar for Apex #6198

Merged
merged 10 commits into from May 30, 2023
Merged

Conversation

muenzpraeger
Copy link
Contributor

@muenzpraeger muenzpraeger commented Nov 29, 2022

Description

This PR changes the association of Salesforce's Apex language from source.java to source.apex. That includes a switch from using Java language definitions to the official tmLanguage definition here.

Checklist:

@Alhadis Alhadis changed the title feat: update Apex language to use correct tmLanguage + added file types Update Apex language to use correct tmLanguage + added file types Nov 29, 2022
@muenzpraeger
Copy link
Contributor Author

Waiting for forcedotcom/apex-tmLanguage#54 to be merged.

@muenzpraeger muenzpraeger marked this pull request as ready for review March 15, 2023 22:45
@muenzpraeger muenzpraeger requested a review from a team as a code owner March 15, 2023 22:45
Copy link
Member

@lildude lildude left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can see that you've manually updated files rather than following the instructions in the CONTRIBUTING.md file and using the supplied scripts. Please don't.

Please use the scripts provided in this repo to change the grammar source.

See my inline comments for the two extensions you're adding.

If in doubt, or for an easy life, don't add these extensions in this PR and leave it purely as a grammar source change PR. Users can implement an override for these files if they feel heavily wed to getting syntax highlighting.

lib/linguist/languages.yml Outdated Show resolved Hide resolved
lib/linguist/languages.yml Show resolved Hide resolved
@muenzpraeger
Copy link
Contributor Author

@lildude Anything to change on my end for the tests?

@lildude
Copy link
Member

lildude commented Mar 16, 2023

Yup. The order of the submodules:

  1) Failure:
TestPedantic#test_submodules_are_sorted [/home/runner/work/linguist/linguist/test/test_pedantic.rb:68]:
Expected false to be truthy.

This stems from you manually doing things rather than using the scripts (the sorting happens at the end of the script/add-grammar script).

You can run script/sort-submodules and commit the updated .gitmodules file.

@muenzpraeger
Copy link
Contributor Author

muenzpraeger commented Mar 16, 2023 via email

@lildude lildude changed the title Update Apex language to use correct tmLanguage + added file types Add grammar for Apex May 22, 2023
Copy link
Member

@lildude lildude left a 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.

@lildude lildude requested a review from a team as a code owner May 30, 2023 09:17
@lildude lildude added this pull request to the merge queue May 30, 2023
Merged via the queue into github-linguist:master with commit acf043b May 30, 2023
5 checks passed
@Arv18
Copy link

Arv18 commented Jun 28, 2023

I understand that adding the extensions, like in an old pull request #4640 are now not necessary.
Thanks

@lildude
Copy link
Member

lildude commented Jun 29, 2023

@Arv18 If they meet our usage requirements, they can be added. You abandoned the last PR so it was closed (when we still used the stale bot). Feel free to open a new PR and be sure to follow the details in the CONTRIBUTING.md file and fill in the template. Keep in mind, like that last PR, the new PR will not be merged until the usage of the new extensions meets our usage requirements. Until then, you can use an override.

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.

None yet

4 participants