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

Added Xojo module #4542

Merged
merged 2 commits into from
May 31, 2019
Merged

Added Xojo module #4542

merged 2 commits into from
May 31, 2019

Conversation

jimmckay
Copy link
Contributor

@jimmckay jimmckay commented May 30, 2019

Added Xojo module to replace incorrect VBDotNet module

Description

Provides syntax coloring comparable to the coloring seen in the Xojo IDE
I am unsure if this qualifies as adding a language or reclassifying.
Xojo language was previously included, but only pointed at source.vbnet which may have been somewhat compatible in the past, but currently produces unsatisfactory results.

Checklist:

Added Xojo module to replace incorrect VBDotNet module
@Alhadis
Copy link
Collaborator

Alhadis commented May 30, 2019

👋 Hey @jimmckay, thanks for the contribution. Always good to have a dedicated grammar for highlighting!

Could you point me to a screenshot of how the colouring looks in Xojo IDE?

@Alhadis
Copy link
Collaborator

Alhadis commented May 30, 2019

The build is failing because the grammars list is out-of-date:

  1) Failure:
TestGrammars#test_readme_file_is_in_sync [/home/travis/build/github/linguist/test/test_grammars.rb:42]:
Grammar list is out-of-date. Run `script/list-grammars`.
--- expected
+++ actual
@@ -426,7 +426,7 @@
 - **XQuery:** [wcandillon/language-jsoniq](https://github.com/wcandillon/language-jsoniq)
 - **XS:** [textmate/c.tmbundle](https://github.com/textmate/c.tmbundle)
 - **XSLT:** [textmate/xml.tmbundle](https://github.com/textmate/xml.tmbundle)
-- **Xojo:** [angryant0007/VBDotNetSyntax](https://github.com/angryant0007/VBDotNetSyntax)
+- **Xojo:** [jimmckay/XojoSyntaxTM](https://github.com/jimmckay/XojoSyntaxTM)

Did you use the script/add-grammar script to add this grammar, or did you do so manually?

@jimmckay
Copy link
Contributor Author

Here is a screenshot of the sample from Lightshow in Xojo.
xojo-syntax

I did use script/add-grammar to add it, but the VBDotNetSyntax was already present. I'm not sure if that might have broken something.

@jimmckay
Copy link
Contributor Author

jimmckay commented May 30, 2019

Looks like I pasted the wrong url in lightshow for the new syntax file.
Here is the correct url.

@Alhadis
Copy link
Collaborator

Alhadis commented May 30, 2019

Here is the correct url:

Erm, when you do that, could you please use markdown links to embed the URL into the text? Like this:

[Here is this correct url](https://github-lightshow.…/end-of-huge-url.html)

Smaller URLs are fine, but Lightshow permalinks are colossal and can make a thread very difficult to peruse in review. I've amended it for you (as well as the OP), but just a note for next time. =)

I did use script/add-grammar to add it, but the VBDotNetSyntax was already present. I'm not sure if that might have broken something.

Possibly. Were you using the --replace switch to remove the old grammar first? Either way, it sounds like a bug; I'll make a note to investigate later.

You can fix the build failures manually if you prefer not to run the script. It's just a simple matter of editing vendor/README.md with the following change:

--- expected
+++ actual
@@ -426,7 +426,7 @@
 - **XQuery:** [wcandillon/language-jsoniq](https://github.com/wcandillon/language-jsoniq)
 - **XS:** [textmate/c.tmbundle](https://github.com/textmate/c.tmbundle)
 - **XSLT:** [textmate/xml.tmbundle](https://github.com/textmate/xml.tmbundle)
-- **Xojo:** [angryant0007/VBDotNetSyntax](https://github.com/angryant0007/VBDotNetSyntax)
+- **Xojo:** [jimmckay/XojoSyntaxTM](https://github.com/jimmckay/XojoSyntaxTM)

@jimmckay
Copy link
Contributor Author

Thanks @Alhadis , I'm obviously new here. I'll be sure to use markdown in the future.
I fixed the readme.

@Alhadis
Copy link
Collaborator

Alhadis commented May 30, 2019

All good! 😉 We all have to start somewhere.

Thanks for your patience and contribution, and welcome to Linguist! We'll get this merged once it's been given the green light by site-staff.

/cc @lildude

@Alhadis Alhadis requested a review from lildude May 30, 2019 02:09
@jimmckay
Copy link
Contributor Author

Awesome! Thanks for your help!

@lildude lildude merged commit b8d1374 into github-linguist:master May 31, 2019
@github-linguist github-linguist locked as resolved and limited conversation to collaborators Jun 17, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants