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

Improve grammar installation #230

Closed
wants to merge 1 commit into from

Conversation

TheFedaikin
Copy link
Contributor

Hello package maintainers!

First of all I wanted to say thanks for creating this grammar!

I've been tinkering with rescript lately and had a couple of issues installing it via currently recommended way while using lazy.nvim. I created a local fork to streamline this process and avoid manually adding the queries into my config. While doing so I thought that this can greatly help people who are using modern NeoVim, but maybe are struggling to figure out how to install this grammar. This is how this PR was born.

If this is not welcome, please let me know I'll close this PR immediately and will just keep it for myself.

Description:

  • This PR moves queries to the separate rescript folder for auto-detection. This allows you to streamline installation of grammar and easily enable highlighting via your favorite plugin manager.
  • It also updates README with new instructions and example of simplified installation until this grammar is upstreamed.
  • There are no functional changes here, all the testing is passing.

Partially related to #203.

All the feedback is welcome, especially considering the instructions.

update queries structure, add native nvim-treesitter.parsers instructions
@TheFedaikin TheFedaikin changed the title Update installtion instructions via "nvim-treesitter.parsers", move queries so they can be easily consumed. Update installation instructions via "nvim-treesitter.parsers", move queries so they can be easily consumed. Aug 4, 2023
@TheFedaikin TheFedaikin changed the title Update installation instructions via "nvim-treesitter.parsers", move queries so they can be easily consumed. Improve grammar installation Aug 4, 2023
@nkrkv
Copy link
Collaborator

nkrkv commented Aug 9, 2023

Hello @TheFedaikin. Thanks for the contribution! Although I shallowly understand the good intent, it will likely break the integration of this and the https://github.com/nkrkv/nvim-treesitter-rescript repo. I need some time to carefully review the changes and the consequences. Please, give me some time, I’m OOO until September.

@TheFedaikin
Copy link
Contributor Author

Hey @nkrkv!

In a way I think this basically streamlines the installation and deprecates https://github.com/nkrkv/nvim-treesitter-rescript for everyone who uses NeoVim in an opinionated way, favoring native custom parser with neovim-treesitter.

I completely understand your point of a view and, as I alluded to, in the initial message I have no problem with this being closed, because it's not needed. There's no rush to this, since I can just maintain my fork and install it locally for my needs.

Have a good vacation!


This will make `TSInstall rescript` globally available. For more persistent approach you should add this parser to your Lua configuration.

Default configuration detects `.res` and `.resi` files. You can confirm that it's correctly installed by using [`nvim-treesitter/playground`](https://github.com/nvim-treesitter/playground) and invoking `TSPlaygroundToggle` when you are in the ReScript file.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since Neovim 0.9 the command :InspectTree is available to inspect the tree.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will update it!

install_info = {
url = "https://github.com/nkrkv/tree-sitter-rescript",
branch = "main",
files = { "src/scanner.c" },
Copy link
Collaborator

@aspeddro aspeddro Aug 28, 2023

Choose a reason for hiding this comment

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

I think parser.c should be added?

Suggested change
files = { "src/scanner.c" },
files = { 'src/parser.c', 'src/scanner.c' },

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 think it's needed explicitly when this file is present, since it's not I don't think so. To support this I have my forked branch installed basically like this

  {
    "nvim-treesitter/nvim-treesitter",
    dependencies = {
      {
        "TheFedaikin/tree-sitter-rescript",
      },
    },
    opts = function(_, opts)
      vim.list_extend(opts.ensure_installed, {
        "rescript",
      })

      local parser_config = require("nvim-treesitter.parsers").get_parser_configs()
      parser_config.rescript = {
        install_info = {
          url = "https://github.com/TheFedaikin/tree-sitter-rescript",
          branch = "main",
          files = { "src/scanner.c" },
          generate_requires_npm = false,
          requires_generate_from_grammar = true,
          use_makefile = true, -- macOS specific instruction
        },
      }
    end,
  },

And it works wonders

@aspeddro
Copy link
Collaborator

Closed by #239

@aspeddro aspeddro closed this Mar 20, 2024
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.

3 participants