plugins/treesitter: switch to mkNeovimPlugin

[khaneliman]

Had a setting I wanted to take advantage of that wasn't supported in its current iteration... figured instead of changing that specific setting I'd help migrate it to the new schema.

[khaneliman]

Here's another round of feedback.

I appreciate this is a fairly complex PR so please ask if there's anything you'd like additional guidance on. Maybe reach out on matrix if that's easier?

Yeah, but it's been nice to get more familiar with the way a lot of these helper methods and library functions work in this project. Sorry if I'm slow with this one, on vacation and just doing bits here and there. I'll reach out on Matrix when I run into some stuff I forgot you all have that setup, too.

[MattSturgeon]

CI error:

error: build of '/nix/store/pqzsfmbgw1izanmx1vr4hqffjz81mvg3-neovim-0.10.0.drv' on 'ssh-ng://[email protected]' failed: error: path '/nix/store/25gm2jy676a7j1ynn5bw1ann1jy9z0xp-init.lua' is not valid

https://buildbot.nix-community.org/#/builders/190/builds/1211

Not sure why it only shows up on one specific system though?

---

As the rename aliases seem to be working, can probably update the these to use the new .settings option paths now.

---

Might not feel like it, but this PR is coming together nicely and I think you're on the home stretch now!

[MattSturgeon]

Looks like our current test doesn't have test-cases for any of the settings options.

Normally plugins of this scale have an "empty" test (with just enable = true), a "defaults" test (where all settings are explicitly set to their plugin-defaults), an "example" test (usually similar to settingsExample) and zero-or-more more specific tests that test special or unusual aspects of the plugin (or integration with other plugins).

• obsidian has two examples (but no "defaults")
• oil is the same
• ccc has an extensive "defaults" case and an example case too
• yanky has all the standard tests, and also a "with-telescope" integration test

To make the "defaults" case, I usually start by copying the defaults documented upstream and then reformatting it as nix code using some vim macros and/or :s commands.

---

Looks like the "custom" test-case has always been commented out, ever since the treesitter test was introduced by @GaetanLepage in #235. (The old tests before that refactor didn't have a treesitter test).

I think we should either remove it or attempt to uncomment it.

---

I think once the test is up to scratch and the last couple outstanding threads are resolved, this PR should be finished, baring any small nit-picks in the final review.

Well done for persisting with this one! :tada:

[GaetanLepage]

Looks like the "custom" test-case has always been commented out, ever since the treesitter test was introduced by @GaetanLepage in #235. (The old tests before that refactor didn't have a treesitter test).

I think we should either remove it or attempt to uncomment it.

I agree, we should clarify this before merging. Of course, uncommenting is the thing to try first.

[khaneliman]

LGTM, thanks for your effort, dedication and patience! 🎉

Np, appreciate you being so helpful with this!

I'll leave this open to give others a chance to review, then merge if there's no additional feedback.

Pushed a quick update for the assertion -> warning

nix build .\#checks.aarch64-darwin.tests.passthru.entries.plugins-languages-treesitter-treesitter-test
trace: warning: Nixvim (plugins.treesitter): You have disabled `nixGrammars`, but `plugins.treesitter.gccPackage` is `null`.
This package is required to build the grammars at runtime.

trace: warning: Nixvim (plugins.treesitter): You have disabled `nixGrammars`, but `plugins.treesitter.nodejsPackage` is `null`.
This package is required to build the grammars at runtime.

trace: warning: Nixvim (plugins.treesitter): You have disabled `nixGrammars`, but `plugins.treesitter.treesitterPackage` is `null`.
This package is required to build the grammars at runtime

[MattSturgeon]

@mergifyio queue

[mergify[bot]]

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at 97fa47376b73d774b081c8e5dd54fcfd0ad278cb

[gkze]

This broke highlighting for me

before

after

Not sure how to debug...

[MattSturgeon]

This broke highlighting for me

Can you link your config? Ideally the minimal treesitter config to reproduce the issue.

[khaneliman]

Looks like the module previously forced the highlight setting to enabled when the plugin was enabled instead of respecting the plugins default of everything disabled by default. Just add settings.highlight.enable = true.

[gkze]

Nice that did it thank you