nvim-neorg / neorg

Modernity meets insane extensibility. The future of organizing your life in Neovim.
GNU General Public License v3.0
6.5k stars 214 forks source link

Concealer Issues Thread #292

Open vhyrro opened 2 years ago

vhyrro commented 2 years ago

Hey! This thread exists to basically catch all of the bugs that the new concealer implementation may be causing. Post your problems here! Aand here's the longer version of the above TL;DR:

Good News

Let's start with the great news: we've massively improved the performance of Neorg's concealer.

I wish I were joking but it's gotten to the point where the bottleneck is no longer Neorg but the code that performs treesitter highlighting in neovim 😂. I did a bunch of testing on over 100k+ loc Neorg documents and after doing :TSBufDisable highlight to prevent the lag that TS was causing things still manage to work smoothly on my machine.

The bad news?

It's definitely not bug free.

Now that our concealer does updates incrementally there's definitely a chance that it'll miss a line or two when concealing in certain circumstances. If you have a problem with the concealer, or something doesn't work as intended, please post your issue here!. I'll be trying to fix them as I go.

A feature was dropped...

With the dawn of the more performant concealer we've temporarily dropped the TODO item completion level feature - you know, the one that displayed (0 of 3) [0% complete] on headings when they had a TODO list under them? It's gone for now.

I suppose I owe you an explanation as to why we've decided to do this. Before our update the concealer would reparse the entire buffer every single time you changed some text or left insert mode. As you can imagine this made it really slow. Just to clarify we didn't want to make it this bad-performing in the first place, but there were certain bugs in nvim_buf_attach that prevented us from properly doing incremental updates then.

The thing is now that everything is happening incrementally we cannot properly recalculate TODO item counts without requiring more context than a single line - and requiring more context greatly slows down the concealer again.

We'll be working hard on trying to bring it back, but we really wanted to push these new changes to you guys without having you wait another week.

UPDATE: Forgot to update this thread but that has been reimplemented now as well. If you have problems with the TODO item completion levels then be sure to let me know here too!

Hope you understand!

vhyrro commented 2 years ago

First bug I found myself: markup concealing was not autotriggering on files lower than 1250 lines, that's been fixed in bfe3056feb91d610c8224e8af1923e7c45676fe7 now :)

max397574 commented 2 years ago

Concealing on wrapped links is broken: wrapped: Screenshot 2022-01-03 at 17 19 12

not wrapped: Screenshot 2022-01-03 at 17 18 54

danymat commented 2 years ago

The : on https://is concealed:

Capture d’écran 2022-01-04 à 11 38 56

Without conceal:

Capture d’écran 2022-01-04 à 11 39 22
mrossinek commented 2 years ago

Concealing on wrapped links is broken:

This is potentially tricky with how things are done currently in the concealer. I will need to take a look at Vhyrro's rewritten code though since there have been quite some changes. The problem will be that we need to iterate the char in the TS range one by one rather than just rely on the length of the node.

mrossinek commented 2 years ago

The : on https:// is concealed:

Yes, I have encountered this too. The problem is that the : get's recognized as a link_modifier. I will try if I can fix the TS parser to disregard single link_modifier nodes (i.e. if they do not occur in pairs). That should fix this.

On another note: those screenshot of yours are still using Markdown-like link syntax!

mrossinek commented 2 years ago

Concealing on wrapped links is broken:

This is potentially tricky with how things are done currently in the concealer. I will need to take a look at Vhyrro's rewritten code though since there have been quite some changes. The problem will be that we need to iterate the char in the TS range one by one rather than just rely on the length of the node.

A followup question to this @max397574: is this soft-wrapped? If so, I don't think there is a way for us to handle this. Neovim itself would have to handle wrapping extmarks..

danymat commented 2 years ago

On another note: those screenshot of yours are still using Markdown-like link syntax!

Right, lmao

max397574 commented 2 years ago

Concealing on wrapped links is broken:

This is potentially tricky with how things are done currently in the concealer. I will need to take a look at Vhyrro's rewritten code though since there have been quite some changes. The problem will be that we need to iterate the char in the TS range one by one rather than just rely on the length of the node.

A followup question to this @max397574: is this soft-wrapped? If so, I don't think there is a way for us to handle this. Neovim itself would have to handle wrapping extmarks..

Not sure what soft-wrapped is it's just a line longer than the screen is width

mrossinek commented 2 years ago

The : on https:// is concealed:

Yes, I have encountered this too. The problem is that the : get's recognized as a link_modifier. I will try if I can fix the TS parser to disregard single link_modifier nodes (i.e. if they do not occur in pairs). That should fix this.

@danymat This should be fixed now. Can you please update your TS parser to verify?

danymat commented 2 years ago

@danymat This should be fixed now. Can you please update your TS parser to verify?

It is indeed ! thanks !

mrossinek commented 2 years ago

Not sure what soft-wrapped is it's just a line longer than the screen is width

See :help 'wrap'. If you use :set nowrap then a line which is longer than your window width will not get wrapped but instead extends to the right (you then need to scroll horizontally to see the rest).

vhyrro commented 2 years ago

Seems to be a problem with generic line breaks as well.

This is {a
test}[with custom text]
mrossinek commented 2 years ago

Seems to be a problem with generic line breaks as well.

Yep. That's the original problem I discuss here: https://github.com/nvim-neorg/neorg/issues/292#issuecomment-1005205020

karlc1 commented 2 years ago

I just started trying neorg today, so I just assume that this is unintended behavior.
When I create a link, the link marker is still being rendered, but its foreground color is set to the normal bg color, so it looks like a bunch of whitespace before the custom text (but i can still yank the text or see it if highlighted in visual mode)

An inline link {| my marker}[with custom text].

looks like this in the editor

An inline link                        with custom text.

max397574 commented 2 years ago

I think I know what's the problem could you send your neorg config and a screenshot?

karlc1 commented 2 years ago

I cannot take a screenshot at the moment, but it really looks just as described; like a bunch of whitespace of equal length as the marker.

Here is the config https://github.com/karlc1/nvim/blob/main/lua/plugins.lua#L691

vhyrro commented 2 years ago

Just a quick heads up that i've managed to fix some bugs related to the link concealing as discussed earlier above.

Those should now be highlighted properly :)

(also @karlc1 if you haven't already then updating Neorg should fix your problems you've described above) :)

esquires commented 2 years ago

Thanks for the fixes! Here is one more. For the line

Here is {:inbox:}[my link]

with markup_preset set to brave I get the following:

brave

If I then enter insert mode on a line above the link by hitting o (not i and not on the link itself) it seems to get fixed:

brave_fixed

However, if I then enter insert mode on the link line it gets reset to the first markup.

karlc1 commented 2 years ago

@vhyrro I use neovim nightly and neorg on the latest tag, and i still don't think that the links look right.

This line test: {:some_file:}[link!].

Looks like this: test:       some_file       link! (with the 'some_file word in faded color')

I would expect it to look like this: test:   link!

max397574 commented 2 years ago

Check out the markup preset here: https://github.com/nvim-neorg/neorg/wiki/Concealer This is expected

karlc1 commented 2 years ago

I see, did not realize this. The 'brave' preset works as I want it to, thanks for the clarification

esquires commented 2 years ago

I see, did not realize this. The 'brave' preset works as I want it to, thanks for the clarification

Interesting it works on your end with the brave setting. Here is how the brave setting works for me for test: ⁠⁠⁠⁠⁠⁠⁠⁠⁠⁠{:some_file:}[link!].

brave_some_file

Can you post your config? Perhaps you have a setting that I don't that is making it work.

max397574 commented 2 years ago

this is because stuff breaks with the joiner char that's why we introduced the other presets

esquires commented 2 years ago

this is because stuff breaks with the joiner char that's why we introduced the other presets

Thanks for the clarification. is it fixable or is brave not intended for use?

karlc1 commented 2 years ago

@esquires It does look like that for me if I enter insert mode on a line with a link, and then go back to normal mode but stay with the cursor on the same line. However, when I move the cursor to a different line it looks correct again.
I have a pretty basic config i believe https://github.com/karlc1/nvim/blob/main/lua/plugins.lua#L680

esquires commented 2 years ago

Thanks!

vhyrro commented 2 years ago

From what we've found it seems to be very inconsistent across different distros, window managers and configs. We don't really know the source of the problem as to why "brave" doesn't work for some, and because of that it's disabled by default. I use kitty and it breaks there, but mrossinek also uses kitty and it doesn't break for him :sweat_smile:. "brave" uses the word-joiner char to quite literally fake concealing and cause of that things go weird. It's not a Neorg config thing either, we've already tested that :P

AndOrangutan commented 2 years ago

Are there any known working terminals? This isn't the first time I have had issues on Kitty.

mrossinek commented 2 years ago

Are there any known working terminals? This isn't the first time I have had issues on Kitty.

As vhyrro mentioned in the comment before yours, I'm having no issues on Kitty. We ave not been able to track down what differs. My dotfiles are in https://gitlab.com/mrossinek/dotfiles in case you want to go poking around though :+1:

karlc1 commented 2 years ago

Link concealing is broken for me today with the brave preset, even though it worked yesterday and I don't think I changed anything.. Are there any possible alternative solutions to using the joiner char for handling this? I could try looking into it myself since this feels like a deal breaker for me

vhyrro commented 2 years ago

To our knowledge there is no solution other than using the joiner char. Even with FFI concealing doesn't seem to be controllable from Neovim in any easy way. The issue to allow this functionality is part of the "unplanned" milestone unfortunately so unless someone allows for that we're kind of screwed :(

esquires commented 2 years ago

The issue to allow this functionality is part of the "unplanned" milestone unfortunately so unless someone allows for that we're kind of screwed :(

It looks like that issue has a “status:has-workaround” label (looks like it was added when the joiner chart was thought to be a workaround). Based on your description of the effects it is having on neorg can we remove that label?

vhyrro commented 2 years ago

The issue to allow this functionality is part of the "unplanned" milestone unfortunately so unless someone allows for that we're kind of screwed :(

It looks like that issue has a “status:has-workaround” label (looks like it was added when the joiner chart was thought to be a workaround). Based on your description of the effects it is having on neorg can we remove that label?

@esquires mrossinek doesn't have permissions to remove the label so we'd have to let the Neovim guys know and ask them to remove it instead. Perhaps it'll grab someone's attention in the meantime :)

vhyrro commented 2 years ago

Also new news regarding the concealer: I've just changed some concealer internals that prevent the weird flashing of code blocks and of regular icons when doing a lot of edits at once. It really makes the experience much more enjoyable.

A thing to note - however - is that in the process I might have broken something that I'm unaware of. In my testing things seem to work fine but I'd rather let you guys know about it either way; be on the lookout for those pesky bugs!

karlc1 commented 2 years ago

@vhyrro is regex based link concealing out of the question?

vhyrro commented 2 years ago

There are many moving parts to links (and especially attached modifiers) that can't easily be expressed with regex :/

If there continues to be no progress in regards to extmark based concealing though I guess we might have to go for it, even if it'll be a lot more primitive.

I just hope anyone thinks of implementing controllable concealing through lua scripting by the time we switch back. I'm really not experienced enough to implement such a thing myself in the Neovim core.

dstadelm commented 2 years ago

set conceallevel does not impact concealing whatever conceallevel I set concealing always looks the same. Only in insert mode the current line is not concealed. E.g. concealcursor=v does not change the behaviour.

Am I missing something from the setup to make this working?

OliverWarrington commented 2 years ago

On brave preset, all my links are replaced with <2060>, which I think is the word-joiner unicode character.

Screenshot 2022-03-14 at 12 09 22
max397574 commented 2 years ago

I have multiple ideas what the problem could be:

I don't exactly know where unicode characters are saved or generated from

OliverWarrington commented 2 years ago

Thanks for the ideas @max397574

I'm on kitty and other unicode is rendered. I have the same result with Fira Code & Hack nerd fonts. I have no idea how to check if my unicode version is outdated but I'm on a new machine and running macos Monterey, so I would be surprised!

I have multiple ideas what the problem could be:

  • terminal doesn't render the unicode
  • outdated unicode version
  • font?

I don't exactly know where unicode characters are saved or generated from

vhyrro commented 2 years ago

Good news, good news everywhere! Two things are happening which I think will excite a large chunk of you:

vhyrro commented 2 years ago

We've just merged the latest TS based concealing into main! Make sure you're running on the latest Neovim nightly for it to activate :D

Note that there are two markup presets now: only conceal (the default), and dimmed.

I will be honest, it does have bugs currently, it's not perfect, but I request that you be patient until tomorrow (as it's too late for me today), and report all the bugs that you can amass on this thread. I'll be trying to fix all of them in one swoop then. Thanks for sticking with me!

vhyrro commented 2 years ago

I've managed to fix a small chunk of problems so far - the main problem right now is performance and also icons overlapping with concealed text (this is gonna be a painful one to fix). I'm around 50% of the way there I'd say with the fixes, I hope to push all changes tomorrow.

Thanks again for being patient! Being up to date with the bleeding edge neovim stuff unfortunately comes with a price :)

vhyrro commented 2 years ago

Yet another change (soz but we had to). We have completely removed markup presets and markup parsing from core.norg.concealer, favouring embedded conceals within highlights.scm instead. To enable conceal, set your conceallevel option to something greater than 0 (preferably 2). To disable it, set conceallevel to 0 again. Makes more sense, doesn't it?

Doing this also comes with a massive performance benefit (no need to apply several extra extmarks just to conceal something), it's just a native part of Neovim core, plus it doesn't give annoying error messages for people < 0.7 :)

There's one last big "main" problem that I won't be able to fix today - icons still stay after being concealed. I need to figure that one out.

roland-5 commented 2 years ago

I updated to latest commits and found this. Was that what your last sentence was about?

Do we need do something like this?

["core.norg.concealer"] = {
   config = { 
     conceallevel=2
   }
}
max397574 commented 2 years ago

conceallevel is a vim option so you just do set conceallevel=2 or vim.opt.conceallevel in lua

roland-5 commented 2 years ago

Thanks! I was afraid that I will ask dumb questions. xd Now, that I see my screen record, I'm not sure if it clearly add what is the problem: If I change mode to insert in line with Task list and back to normal mode disappear text with 1 of x and with percentage completion. If I try again insert mode and back to normal, still it's missing. If I change to insert mode in text above it, is the same, but if I change mode to insert in any line below Task list, text with 1 of x and percentage are back to Task list line.

vhyrro commented 2 years ago

Oooh, incredible catch! That's quite the bug to find, since it requires you to explicitly have your cursor before the asterisks of the heading to trigger. Took me a while to reproduce that one lol. I'll see what I can do to fix it

vhyrro commented 2 years ago

Yo @roland-rollo I believe I may have fixed your bug, mind checking it out? :)

roland-5 commented 2 years ago

Sadly, no. But I don't know if it makes sense to fight it further. I checked what you wrote and as you wrote before, you need to be on asterisk, switching between modes, and then is good. Remembering that is not a problem for me. And as I see, I was the only person who noticed it by accident. :P So, I suggest leaving it.