feat(extmarks): expose spell from extmarks

[vigoux]

Expose the spell checking to the extmarks API.

Frist dip into win_line.

cc @bfredl @clason @lewis6991

[lewis6991]

Nice!

What's the plan with navigation? As is, this only allows #19351 to be simplified.

[vigoux]

So maybe I lack a bit of explanation about the functionning of this PR as is. Here it goes:

1. Extmarks can set a spell flag to notify nvim that spellchecking should be run on the area marked by the extmark
2. If the window where the mark is reported has spell then it is pretty much just like when the legacy syntax engine reported a @Spell region.

There is a few more things to that:

• Each decoration provider has a flag associated to it telling whether it handles spell checking (may be used later for navigation for example)
• For a given extmark there is actually three states:
  1. spell = true: force spell checking here
  2. spell = false: force not spell checking here
  3. unset: default to the old behavior

[lewis6991]

For now I don't think options passed to nvim_set_decoration_provider should influence in any way how extmarks are rendered. Specifically any extmark with spell=true should always have spelling applied and not be predicated on how the provider is set up. This will allow non-provider extmarks to apply spelling. Therefore the only effect of spell=true in nvim_set_decoration_provider should just be to disable legacy spellchecking.

For navigation we just run on_line for any spell-enabled provider and then see if there is an extmark at the location. Currently for navigation, Neovim spell checks every line then ignores the result if there is no @Spell attribute, so we can do exactly the same for extmarks.

[vigoux]

Okay, current state of the PR is:

• New 'spelloptions' flag named 'noplainbuffer' that indicates that spellchecking should not be done on the full buffer (only with either syntax or extmarks)
• spell option to nvim_buf_set_extmark indicating that this extmark should trigger spellchecking
• For decoration providers: half-done but spell flag indicating that this decoration provider provides spell checking annotations.

The goal now is to enable navigation of spellchecking error using decoration providers and static extmarks. For decoration providers, if the spell flag is set then it will be called during the search for a spellchecking mistake (that could be the whole buffer, but I don't see any other way). Static extmarks should follow directly from the use of the decor_state during the search for spellchecking mistakes

[vigoux]

This PR should be ready to merge now, docs has been added, and style fixes too.

Thansk @lewis6991 and @bfredl for the time you took along the way to come to this implementation.

[vigoux]

This should be ready to merge, and is now actually well tested (for the extmarks and decoration provider part, the tree-sitter part will come later).

The discussion that still need to be addressed is whether to use a separate query, or to re-use highlights.scm.

My opinion is that highlights.scm is now the go-to query for decorations-related features, and as spell is now a special kind of decoration, this should be done this way.

[clason]

My opinion is that highlights.scm is now the go-to query for decorations-related features, and as spell is now a special kind of decoration, this should be done this way.

So you want to treat it like @_conceal?

In any case, probably best to get the parser/query PR in first (which we can do even without the opt-in option) so you can change the bundled queries accordingly.

(Note that this will lead to some divergence from upstream.)

[zeertzjq]

= ==33337==ERROR: LeakSanitizer: detected memory leaks
= 
= Direct leak of 2560 byte(s) in 2 object(s) allocated from:
=     #0 0x4e3993 in realloc ??:?
=     #1 0x12ffcb2 in xrealloc /home/runner/work/neovim/neovim/src/nvim/memory.c:164:15
=     #2 0x904c27 in decor_add /home/runner/work/neovim/neovim/src/nvim/decoration.c:255:3
=     #3 0x90e98e in decor_add_ephemeral /home/runner/work/neovim/neovim/src/nvim/decoration.c:542:3
=     #4 0x5f4e57 in nvim_buf_set_extmark /home/runner/work/neovim/neovim/src/nvim/api/extmark.c:786:5
=     #5 0x52db62 in nlua_api_nvim_buf_set_extmark /home/runner/work/neovim/neovim/build/src/nvim/auto/lua_api_c_bindings.generated.c:2122:23
=     #6 0x1fd907b in luaD_precall /home/runner/nvim-deps/build/src/lua/src/ldo.c:320:10
=     #7 0x1fe78d7 in luaV_execute /home/runner/nvim-deps/build/src/lua/src/lvm.c:591:17
=     #8 0x1fd932e in luaD_call /home/runner/nvim-deps/build/src/lua/src/ldo.c:378:5
=     #9 0x1fd57cb in f_call /home/runner/nvim-deps/build/src/lua/src/lapi.c:800:3
=     #10 0x1fd8338 in luaD_rawrunprotected /home/runner/nvim-deps/build/src/lua/src/ldo.c:116:3
=     #11 0x1fd9738 in luaD_pcall /home/runner/nvim-deps/build/src/lua/src/ldo.c:464:12
=     #12 0x1fd588d in lua_pcall /home/runner/nvim-deps/build/src/lua/src/lapi.c:821:12
=     #13 0x10d3d99 in nlua_pcall /home/runner/work/neovim/neovim/src/nvim/lua/executor.c:140:16
=     #14 0x10d677f in nlua_call_ref /home/runner/work/neovim/neovim/src/nvim/lua/executor.c:1519:7
=     #15 0x9128fe in decor_provider_invoke /home/runner/work/neovim/neovim/src/nvim/decoration_provider.c:27:16
=     #16 0x91235f in decor_providers_invoke_spell /home/runner/work/neovim/neovim/src/nvim/decoration_provider.c:68:7
=     #17 0x19a143c in spell_move_to /home/runner/work/neovim/neovim/src/nvim/spell.c:1334:17
=     #18 0x1456019 in nv_brackets /home/runner/work/neovim/neovim/src/nvim/normal.c:5150:11
=     #19 0x142fdeb in normal_execute /home/runner/work/neovim/neovim/src/nvim/normal.c:1172:3
=     #20 0x1a87618 in state_enter /home/runner/work/neovim/neovim/src/nvim/state.c:88:26
=     #21 0x13feba0 in normal_enter /home/runner/work/neovim/neovim/src/nvim/normal.c:471:3
=     #22 0x110488a in main /home/runner/work/neovim/neovim/src/nvim/main.c:583:3
=     #23 0x7f5e2d753082 in __libc_start_main /build/glibc-SzIz7B/glibc-2.31/csu/../csu/libc-start.c:308:16
= 
= Direct leak of 184 byte(s) in 1 object(s) allocated from:
=     #0 0x4e369d in malloc ??:?
=     #1 0x12ff722 in try_malloc /home/runner/work/neovim/neovim/src/nvim/memory.c:80:15
=     #2 0x12ff91c in xmalloc /home/runner/work/neovim/neovim/src/nvim/memory.c:114:15
=     #3 0x6fd936 in api_set_error /home/runner/work/neovim/neovim/src/nvim/api/private/helpers.c:798:14
=     #4 0x10d6875 in nlua_call_ref /home/runner/work/neovim/neovim/src/nvim/lua/executor.c:1524:7
=     #5 0x9128fe in decor_provider_invoke /home/runner/work/neovim/neovim/src/nvim/decoration_provider.c:27:16
=     #6 0x913d0a in decor_providers_start /home/runner/work/neovim/neovim/src/nvim/decoration_provider.c:91:16
=     #7 0x9b13b4 in update_screen /home/runner/work/neovim/neovim/src/nvim/drawscreen.c:542:3
=     #8 0x148ece3 in normal_redraw /home/runner/work/neovim/neovim/src/nvim/normal.c:1289:5
=     #9 0x148c5c6 in normal_check /home/runner/work/neovim/neovim/src/nvim/normal.c:1381:5
=     #10 0x1a86fd6 in state_enter /home/runner/work/neovim/neovim/src/nvim/state.c:30:35
=     #11 0x13feba0 in normal_enter /home/runner/work/neovim/neovim/src/nvim/normal.c:471:3
=     #12 0x110488a in main /home/runner/work/neovim/neovim/src/nvim/main.c:583:3
=     #13 0x7f5e2d753082 in __libc_start_main /build/glibc-SzIz7B/glibc-2.31/csu/../csu/libc-start.c:308:16
= 
= SUMMARY: AddressSanitizer: 2744 byte(s) leaked in 3 allocation(s).

[clason]

If you rebase on master, you can add the @spell annotations to the bundled highlight queries. (Unless you prefer to do that in a followup PR.)

[bfredl]

looks alright with TODO comments for things we want to refactor later. Squash of fixup commits?

[ignamartinoli]

I had an issue open on spellsitter.nvim that wasn't solved. Should I report it to Neovim?

[clason]

If it's https://github.com/lewis6991/spellsitter.nvim/issues/85, then no -- this is an issue with the markdown queries, which are not distributed with Neovim. Make an issue (or better, a PR) to nvim-treesitter.

[ignamartinoli]

@clason alright thanks, same thing should apply for lewis6991/spellsitter.nvim#62 and the comment I added under it?

[clason]

Yes.

[sQVe]

👋🏼,

I'm super confused on how this is supposed to work. I have spelloptions set to camel but for certain filetypes, like markdown, it also adds noplainbuffer now. This essentially removes the spelling for TypeScript.

What am I missing? 🤔

[lewis6991]

noplainbuffer should only be set for treesitter windows (via :setlocal).

[sQVe]

@lewis6991 Yeah, sorry, I messed up my comment. It should say typescript instead of markdown.

If I want to be able to spellcheck typescript files, what should I do? It feels like I'm just missing something simple.

[lewis6991]

To spell check any language, you need to make sure there are @spell queries in the languages highlights.scm.

[sQVe]

@lewis6991 Alright. So in this case I'm guessing typescript doesn't have those queries. How would I best opt out of the noplainbuffer setting?

[lewis6991]

Either remove it in some autocommand, or just simply add a (comment) @spell to highlights.scm

[sQVe]

@lewis6991 Okay! Is it possible to configure the highlights.scm on my machine for a language or do I have to make a PR for it?

Also, really appreciate you taking your time and answering. ❤️

[max397574]

just put it in your config or somewhere else into the runtimepath

[lewis6991]

On mobile so answer is brief.

You should be able to set up a custom highlights.scm, but the best thing would be to raise a PR (to nvim-treesitter) to add the query to query/ecma/highlights.scm

[lewis6991]

Actually don't worry, I'll raise a PR, I'll try and fill in some other languages whilst I'm at it.

[sQVe]

@lewis6991 Cheers 🍻. If you're in Sweden - ping and I'll buy you a beer 🙏🏼 ❤️

[lewis6991]

https://github.com/nvim-treesitter/nvim-treesitter/pull/3549

[e-kwsm]

After 75adfefc85bcf0d62d2c0f51a951e6003b595cea, nvim seems to spell-check the entire contents and recognize some keywords/identifiers misspelled: open the following C file

#include <stdbool.h>
bool func(void);

:set spell and type ]s a few times, then the cursor is navigated to stdbool, bool, and func.

C’s highlights.scm has https://github.com/neovim/neovim/blob/e6c214033a4fadf60faf99e95f8e9787e3c5e630/runtime/queries/c/highlights.scm#L104 and https://github.com/neovim/neovim/blob/e6c214033a4fadf60faf99e95f8e9787e3c5e630/runtime/queries/c/highlights.scm#L152

Am I misunderstanding the feature?

[lewis6991]

No, please raise an issue.

Also check you haven't got legacy syntax highlighting enabled.

[lewis6991]

Locking PR, as getting too many posts. If you have issue with this, either reach out on matrix or raise an issue.