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

Boxr syntax highlighting #9

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

cybersonic
Copy link

This is my initial pass at syntax highlighting for boxr.
see: #8

@KamasamaK
Copy link
Collaborator

  • Extension structure generally has syntaxes in the root as src is reserved for the TS/JS files.
  • It's strange to see comment under the keywords rule. It's usually its own rule.
  • There is no punctuation being scoped in the grammar (e.g. punctuation.definition.string, punctuation.definition.comment)

@cybersonic
Copy link
Author

Thanks for the feedback! I shall move the syntaxes, and the comment is literally what comes out of the box in the demo project.

@cybersonic
Copy link
Author

@KamasamaK Is this a good guide to add the punctuation.definition stuff?
https://www.sublimetext.com/docs/3/scope_naming.html#punctuation

I am not sure where to look for good resources.

@KamasamaK
Copy link
Collaborator

KamasamaK commented Sep 24, 2020

@cybersonic Yes, that is a good guide for scope naming, but the syntax for SublimeText grammars is a bit different from TextMate. You may also find some of the built-in TM grammars helpful (e.g. shellscript).

@bdw429s
Copy link
Contributor

bdw429s commented Sep 29, 2022

Is this still in progress? @KamasamaK Can we merge what we have?

@KamasamaK
Copy link
Collaborator

I'm not familiar enough with the file format to say whether it's correct, so my issues with it were more general around standard practice for contributing languages.

I haven't tested it, but from what I can tell it won't break anything that already exists and works, so if @cybersonic is unable to make those improvements it should still be fine and probably better than nothing.

@bdw429s
Copy link
Contributor

bdw429s commented Sep 29, 2022

Yeah, I'd prefer to at least merge what we have even if it's not perfect and we can improve on it. I just don't' like pulls sitting forever :)

@lmajano
Copy link
Contributor

lmajano commented Dec 20, 2022

Sooo, should this be merged now? It has sat for a long time @bdw429s @bdw429s ?

Is it working @cybersonic

@cybersonic
Copy link
Author

Looking at the changes that @KamasamaK requested. Will update the PR

@lmajano
Copy link
Contributor

lmajano commented Sep 13, 2023

Do we have anything else in order to merge this @cybersonic @KamasamaK

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

5 participants