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

helix: Support [[grammar]] in languages.toml #3911

Closed
wants to merge 1 commit into from

Conversation

sloane-shark
Copy link
Contributor

@sloane-shark sloane-shark commented Apr 24, 2023

Description

Add support for the [[grammar]] key of languages.toml: https://docs.helix-editor.com/languages.html#tree-sitter-grammar-configuration

Checklist

  • Change is backwards compatible.

  • Code formatted with ./format.

  • Code tested through nix-shell --pure tests -A run.all.

  • Test cases updated/added. See example.

  • Commit messages are formatted like

    {component}: {description}
    
    {long description}
    

    See CONTRIBUTING for more information and recent commit messages for examples.

  • If this PR adds a new module

    • Added myself as module maintainer. See example.

    • Added myself and the module files to .github/CODEOWNERS.

edit: Should fix #2871

@@ -0,0 +1,12 @@
[use-grammars]
except = ["yaml", "json"]
[[grammar]]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

i dont love that using concatTextFile gives us this very slightly ugly result without an additional break between sections, unsure if there is a workaround

@sloane-shark
Copy link
Contributor Author

cc @Philipp-M in case you have any thoughts

Comment on lines +179 to +182
# NB: helix requires that `use-grammars` be the first key in languages.toml if present
# pkgs.formats.toml relies on piping generated json through
# [remarshal](https://github.com/remarshal-project/remarshal),
# so we have to concatenate in order to preserve this ordering
Copy link
Contributor

Choose a reason for hiding this comment

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

Are you sure about that? AFAIK helix uses the serde_derive implementation for it which should respect the TOML standard, no weird TOML breaking custom deserializer (in the way that it relies on ordered tables/objects).

Copy link
Member

@ncfavier ncfavier May 1, 2023

Choose a reason for hiding this comment

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

The docs say

# Note: this key must come **before** the [[language]] and [[grammar]] sections

but indeed this just means that it should be outside of the language and grammar sections, so just regular TOML.

@Philipp-M
Copy link
Contributor

Sorry almost forgot about it.

I'm not sure about the PR (see comment in code), I think it's better to just serialize the whole languages.toml.

I have implemented this some time back in Philipp-M@8bdd7cc.
Actually I have planned to upstream this when a new helix release is out that has merged this: helix-editor/helix#2507

But it doesn't solve the grammars issue see the discussion in #2871

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.

Helix: support configuring grammars
3 participants