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 support for Vento language #6733

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

Conversation

oscarotero
Copy link

Hi. In this PR I'm adding support for Vento templates.

Description

Vento (.vto extension) is a template engine with support for Deno and Node. You can see more info in the documentation site: https://vento.js.org/

Checklist:

@oscarotero oscarotero requested a review from a team as a code owner March 1, 2024 18:47
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.

See inline comments. This language is also a long way from meeting our popularity requirements so won't be merged any time soon.

color: "#ff0080"
extensions:
- ".vto"
- ".vento"
Copy link
Member

@lildude lildude Mar 2, 2024

Choose a reason for hiding this comment

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

Samples and representative search links need to be provided for all extensions being added. Note, all extensions need to meet our usage requirements before inclusion so if one is less popular than another, it's probably best to remove it and people can rely on an override to force the language until it meets the usage requirement for adding at a later date.

Copy link
Author

Choose a reason for hiding this comment

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

Okay, I guess .vto is the right extension. Will remove .vento.

extensions:
- ".vto"
- ".vento"
tm_scope: text.html.js
Copy link
Member

Choose a reason for hiding this comment

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

This is not the scope from the grammar. It should be.

Copy link
Author

Choose a reason for hiding this comment

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

I run the script/add-grammar passing the vscode plugin https://github.com/oscarotero/vscode-vento as argument. I guess it's not 100% compatible and needs some changes.

</div>
</main>
</div>
{{ /layout }}
Copy link
Member

Choose a reason for hiding this comment

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

This is not the sample you reference in the PR template. Either update the template of this sample.

Copy link
Author

Choose a reason for hiding this comment

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

Okay, I'll search better examples.

@@ -1187,6 +1187,9 @@
[submodule "vendor/grammars/typst-grammar"]
path = vendor/grammars/typst-grammar
url = https://github.com/michidk/typst-grammar.git
[submodule "vendor/grammars/vento"]
path = vendor/grammars/vento
url = https://github.com/oscarotero/vento.git
Copy link
Member

@lildude lildude Mar 2, 2024

Choose a reason for hiding this comment

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

There should only be one entry per language in this file and it should be for the grammar as added by the add-grammar script. The script also sorts the file. This is not the grammar repo so doesn't belong in this file.

Copy link
Author

Choose a reason for hiding this comment

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

My bad. I run script/add-grammar twice. The first one passing this repository which gives me an error. So I run again with the vscode repository which contains the grammar. I guess this line was added in the first run. Will remove it.

@@ -579,6 +579,7 @@ This is a list of grammars that Linguist selects to provide syntax highlighting
- **Vala:** [technosophos/Vala-TMBundle](https://github.com/technosophos/Vala-TMBundle)
- **Valve Data Format:** [Nixinova/NovaGrammars](https://github.com/Nixinova/NovaGrammars)
- **Velocity Template Language:** [animecyc/AtomLanguageVelocity](https://github.com/animecyc/AtomLanguageVelocity)
- **Vento:** [gregory-m/ejs-tmbundle](https://github.com/gregory-m/ejs-tmbundle)
Copy link
Member

Choose a reason for hiding this comment

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

🤨 I have no idea how you did this. This file is automatically updated when you run the add-grammar script.

Copy link
Author

Choose a reason for hiding this comment

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

Me neither :)
I will edit it with the correct repository URL.

@oscarotero
Copy link
Author

Thanks, I know it's not so popular yet, but if it's okay, I will keep this PR opened until it meets your requirements.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants