Skip to content

Conversation

broken-pen
Copy link
Contributor

This passes the spell option to nvim_buf_set_extmark() for all markup groups. In most cases, the passed value is simply nil, i.e. the same as if it were not passed at all.

In the following cases, however, we pass spell=false, i.e. forcefully disable spell-checking:

  • the URL part of [[URL][hyperlinks]];
  • the inside of [[hyperlinks]] without description;
  • ~code~ and =verbatim= spans;
  • in-line LaTeX markup using \(parentheses\).

The reason in all cases is that these spans are extremely likely to contain purposefully non-orthographic spelling, in particular hyperlinks.

@broken-pen
Copy link
Contributor Author

Hmm, something is still strange about this. The change in this PR does indeed remove highlighting for bad words, but the keymaps ]s and [s (go to next/prev misspelled word) still jump to these now-invisible errors. I'll investigate more tomorrow, if I find the time. There might be a bug in how extmarks handle spelling. The feature is still rather new, if I remember correctly.

@broken-pen broken-pen force-pushed the fix/spell-check-in-markup branch 2 times, most recently from 85da2b3 to 338c04c Compare April 26, 2023 21:01
hl_name = 'org_underline',
hl_cmd = 'hi def org_underline term=underline cterm=underline gui=underline',
nestable = true,
spell = nil,
Copy link
Member

Choose a reason for hiding this comment

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

I believe all spell = nil can be removed, since it will default to nil when it tries to find something on table that does not exist.

@kristijanhusak
Copy link
Member

I can confirm that this works, but has that issue you mentioned that it jumps to the works that should not have spell checking. Do you maybe have any news on that?
We could also expand this and move old syntax spell toplevel from syntax/org.vim to syntax/org_legacy.vim and define specific captures to have the spell checking, like (body (paragraph) @spell), etc.

@broken-pen
Copy link
Contributor Author

Sorry, this week has been pretty busy. I only found time to sit down and look into it tonight.

As far as I can tell, this buggy behavior is universal for neovim extmarks. I've opened an issue about this with them (see the mention above), let's see what comes out of that.

Regarding a workaround on the orgmode side, I'm not quite sure how to do that. Using captures sounds promising, but my (extremely limited) understanding is that the TS grammar doesn't currently track inline markup. See this screenshot of the :InspectTree output (requires nvim 0.9.0+):

grafik

Naively, I would've expected a TS nodes like "code" , but instead everything is just "expr" and the actual highlighting is done with extmarks. With this set up, I don't know if it's possible to define specific captures for spell checking.

@kristijanhusak
Copy link
Member

Don't worry, this is open source, no one expects you to be around on demand :)

It seems there is a draft PR that fixes the issue so I hope it will cover this.
Do you know when they added support for spell on extmarks? I wasn't strictly following.

I'll double check this PR and merge it if it's ok. Thanks!

I think you misunderstood what I meant. I meant to remove the old vim syntax spell checking and introduce it on the treesitter level in the queries, something like this https://github.com/nvim-treesitter/nvim-treesitter/blob/master/queries/ecma/highlights.scm#L179.

This can be done separately, so don't worry about it.

This passes the `spell` option to `nvim_buf_set_extmark()` for all
markup groups. In most cases, the passed value is simply `nil`, i.e. the
same as if it were not passed at all.

In the following cases, however, we pass `spell=false`, i.e. forcefully
disable spell-checking:

- the URL part of `[[URL][hyperlinks]]`;
- the inside of `[[hyperlinks]]` without description;
- `~code~` and `=verbatim=` spans;
- in-line LaTeX markup using `\(parentheses\)`.

The reason in all cases is that these spans are extremely likely to
contain purposefully non-orthographic spelling, in particular
hyperlinks.
@broken-pen broken-pen force-pushed the fix/spell-check-in-markup branch from 0664942 to ae9195d Compare April 30, 2023 12:24
@broken-pen
Copy link
Contributor Author

I had to dig a little, but I think spell=false was introduced in neovim/neovim#20178. (It's also mentioned in the release notes for nvim 0.9.0.) The same PR apparently also introduced @nospell captures, but I couldn't make those work in the orgmode queries. Not terribly surprising though, since I'm still very new to the treesitter ecosystem.

In any case, I've removed the spell = nil lines as requested. Let me know if there's anything else, and thanks for the feedback!

Copy link
Member

@kristijanhusak kristijanhusak left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

@kristijanhusak kristijanhusak merged commit d383f5c into nvim-orgmode:master May 1, 2023
@kristijanhusak
Copy link
Member

Just a note that I added support for tree-sitter level spell checking here 4647d20, so spell checking should generally be better than before.

@broken-pen
Copy link
Contributor Author

Very cool! I'll give it a try and report bugs if I find any 👍

@broken-pen broken-pen deleted the fix/spell-check-in-markup branch July 25, 2023 13:41
SlayerOfTheBad pushed a commit to SlayerOfTheBad/orgmode that referenced this pull request Aug 16, 2024
@broken-pen broken-pen mentioned this pull request May 12, 2025
6 tasks
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.

2 participants