search

neovim / tree-sitter-vimdoc

Tree-sitter parser for Vim help files
Apache License 2.0
103 stars 13 forks source link

remove scanner, rewrite grammar.js #16

Closed justinmk closed 2 years ago

justinmk commented 2 years ago

fix #7 fix #9 fix #10 fix #11 fix #14 fix #12 (non-nested) fix #13 (non-nested) fix #17

justinmk commented 2 years ago

Incompatible language version 14. Expected minimum 13, maximum 13

@vigoux any idea how I can fix this? Is CI installing an older version of tree-sitter CLI?

justinmk commented 2 years ago

code_block is kinda working but still needs work. Other than that, all tests are passing (plus several new tests).

vigoux commented 2 years ago

It is pretty surprising... I'll look into this problem today.

vigoux commented 2 years ago

I pushed a fix for that in latest master.

To ensure consistency of the parser version, I recommend you to:

This should handle all the version management for us now.

clason commented 2 years ago

Yes, looks like CI installs an old parser (0.20.0)?

As ABI 14 is now (0.20.7) the default, it would be better to update the CI.

vigoux commented 2 years ago

The CI uses the locked version of tree-sitter.

In latest master the locked version now supports ABI 14, and furthermore, the default ABI for this parser should now be 14.

clason commented 2 years ago

Any interest in including queries in this repo, too, so CI can catch breaking changes that require query updates on PRs?

(Does this PR require query updates?)

vigoux commented 2 years ago

Would be good too. I'll have to look up the names of the captures for markup languages.

clason commented 2 years ago

You can start with https://github.com/nvim-treesitter/nvim-treesitter/blob/master/queries/help/highlights.scm (sans the quote conceal).

We don't need to follow the strict corset of nvim-treesitter, but it would help; the allowed text captures are here: https://github.com/nvim-treesitter/nvim-treesitter/blob/master/CONTRIBUTING.md#text

justinmk commented 2 years ago

(Does this PR require query updates?)

yeah, mostly the introduction of:

I will probably alias (line_noeol) => (line) and (block_end) => (block) since there isn't much reason to expose those.

For "inner content" I have named the field either "name" or "text", to add some consistency.

If we're open to renaming things, I suggest:

justinmk commented 2 years ago

code_block is working. Only 2 failing tests remaining for "option". Then will add keycode and keychord, then this is ready I think.

clason commented 2 years ago

If we're open to renaming things, I suggest:

We are. If you add the new queries here, I'll just plonk them into nvim-treesitter when bumping this parser. (Makes my life much easier.)

(The parser is currently marked as "experimental", so I have no qualms about breaking things. Could think about removing that label, though, if you deem things stable enough after this PR.)

clason commented 2 years ago

code_block is working.

I know it's a long shot (especially now that there's no custom scanner anymore), but any chance the new code_block can specify the language (either Lua or Vimscript -- or just one of the two -- would be enough)?

vigoux commented 2 years ago

Without changing the "spec" for help files, I think that this cannot be handled in the parser.

The parser user would have to guess the filetype of the codeblock by looking at what it contains otherwise.

justinmk commented 2 years ago

any chance the new code_block can specify the language (either Lua or Vimscript

Isn't that something that can be added later? Could also extend the vimdoc spec as @vigoux mentioned, e.g. vimscript> and lua>.

clason commented 2 years ago

Oh, sure, I just wanted to raise awareness in case it matters.

vigoux commented 2 years ago

By the way, I am okay with renaming things. I think taking a fresh start is great.

clason commented 2 years ago

Isn't that something that can be added later? Could also extend the vimdoc spec as @vigoux mentioned, e.g. vimscript> and lua>.

That is a good point; adding vim> and lua> as valid (concealed) markers to the legacy syntax file would allow us to use these markers in our own help files at least. And once this parser has parity with the syntax file, enable it by default and presto: highlighted code blocks.

vigoux commented 2 years ago

For the record, this seems to fail parsing some options.

justinmk commented 2 years ago

For the record, this seems to fail parsing some options.

Yes, I need to fix some stuff

justinmk commented 2 years ago

options are fixed (locally, WIP), now 2 other issues to fix :D getting close...

justinmk commented 2 years ago

Ok all tests are passing and I think the results are pretty good. I will note the compromises as PR comments.

I didn't rename anything yet, nor add highlights. Should that wait for a followup PR?

clason commented 2 years ago

I didn't rename anything yet, nor add highlights. Should that wait for a followup PR?

Best to rip the bandaid off in one go.

And, yes, please add queries; it's good practice to keep them in sync, otherwise I'll have to manually fix the ones we bundle or it will block all updates in nvim-treesitter. (Can be improved later; the main point is that they don't error out with the new parser.)

justinmk commented 2 years ago

Plan

  1. fix column_heading to not parse special forms like links, tags, etc. It only supports plaintext.
    • skipping this for now, consumers can just check if the parent is column_heading and then treat its children as plaintext.
  2. ✅ rename atoms
  3. ✅ add queries/highlights
  4. future PR: try to eliminate block_end and line_noeol (just for elegance)?
  5. future PR: introduce (keycode), (special) for #1
justinmk commented 2 years ago

Latest push includes renamed forms. See updated comment https://github.com/vigoux/tree-sitter-vimdoc/pull/16#issuecomment-1250864587 for summary.

vigoux commented 2 years ago

You should have commit access now, feel free to merge this PR and take the parser over from here