chrisbra / Colorizer

color hex codes and color names
547 stars 30 forks source link

colorizer_use_virtual_text seems to be not working in Neovim #71

Closed snide closed 5 years ago

snide commented 5 years ago

Hello. Thank you for this fantastic little plugin. I was very excited when I saw you had swatch support rather than the full on backgrounds. I've tried adding let g:colorizer_use_virtual_text = 1 to my init.vim but this doesn't seem to do anything.

Some hopefully helpful details:

Anyways, I saw this was a recent addition so figured I'd ask to make sure it was still working from what you can see. Thanks again. Wonderful little plugin.

image

chrisbra commented 5 years ago

Hm, looks like neovims virtual text is not allowed in the sandbox. cc @bfredl

I can work around this at the cost of creating many undo branches. Please check current head, it at least does not error out and works for me (not a usual neovim user).

bfredl commented 5 years ago

@chrisbra If the sandbox is active the API is disabled, not doing so was a security risk. What code is this plugin running inside the sandbox and why? IIUC the sandbox is for unsafe code loaded from files like 'modeline', not for plugin code.

chrisbra commented 5 years ago

@bfredl I contributed Vim patch 7.3.627 specifically for this plugin. It made working on matches fast, since I do not have to loop over the complete buffer text manually to find the matches. It adds the sandbox, since we do not want to change the buffer (but it still allows to create highlight matches for each found match).

Ideally, neovim would allow the same, prevent changing the buffer, but would still allow me to act on each match (even in the sandbox).

bfredl commented 5 years ago

"sandbox" is a security feature for untrusted code (or at least it tries to be), not for preventing a buffer to be incorrectly mutated inside a command or anything like that. By design sandbox must forbid "acting on" anything interesting, otherwise it is not secure.

Sounds like "n" flag substitution should have used "textlock" and not "sandbox".

chrisbra commented 5 years ago

it might indeed, but it works as documented.

So I suppose, this means, it works (or rather not works) as intended?

bfredl commented 5 years ago

it might indeed, but it works as documented.

The documented behavior is what I described. The question is if we want to change it.

So I suppose, this means, it works (or rather not works) as intended?

Intended by whom? If the purpose of |sub-replace-expression| is to run arbitrary code as long as it doesn't mess up the substitution, we should change it to use |textlock|. Unless some people already have internalized it as a "security" feature, we would need a separate flag then to not break this expectation (though I find it unlikely).