ap / vim-css-color

Preview colours in source code while editing
http://www.vim.org/scripts/script.php?script_id=5056
MIT License
1.79k stars 80 forks source link

Error when matches window of `set icm=split` is shown and pressed Ctrl-F #95

Open unclechu opened 6 years ago

unclechu commented 6 years ago

I'm not sure if this feature (set icm=split) exists in regular Vim but it exists in Neovim. Here is my own demonstration of that feature: https://www.youtube.com/watch?v=dY9dME3l-iQ So when that matches window is shown in command-line mode and I press Ctrl-F to edit command in more regular vimmy window css-color is failing with this error: screenshot from 2018-08-15 00-59-56

ap commented 6 years ago

Hi, thanks. Yes, this is evidently a Neovim thing: my Vim 8 hasn’t heard of 'icm'. I don’t know if it can be made to work in that window but oviously it should at the very least not throw big obtrusive errors like that.

unclechu commented 6 years ago

It would be good to silence the error for first, and, if it's possible make plugin work for that window.

justinmk commented 6 years ago

bufname('%') in that window is [Preview]. But this might be a Nvim bug, no autocmds should be enabled in that window. Would be great to get a minimal repro.

unclechu commented 6 years ago

@justinmk well, I'm getting [Command Line] but I'm really confused how I fixed it. Look: https://github.com/ap/vim-css-color/pull/98/files It looks like Neovim doesn't like when conditions separated by | but only in context of icm=split. Weird.

unclechu commented 6 years ago

@ap This PR fixes it https://github.com/ap/vim-css-color/pull/98

unclechu commented 6 years ago

@justinmk About version of Neovim I'm using:

λ dnf info neovim
Last metadata expiration check: 10 days, 19:24:05 ago on Fri 10 Aug 2018 02:34:29 AM MSK.
Installed Packages
Name         : neovim
Version      : 0.3.0
Release      : 2.fc28
Arch         : x86_64
Size         : 19 M
Source       : neovim-0.3.0-2.fc28.src.rpm
Repo         : @System
From repo    : updates
Summary      : Vim-fork focused on extensibility and agility
URL          : http://neovim.io
License      : ASL 2.0
Description  : Neovim is a refactor - and sometimes redactor - in the tradition of
             : Vim, which itself derives from Stevie. It is not a rewrite, but a
             : continuation and extension of Vim. Many rewrites, clones, emulators
             : and imitators exist; some are very clever, but none are Vim. Neovim
             : strives to be a superset of Vim, notwithstanding some intentionally
             : removed misfeatures; excepting those few and carefully-considered
             : excisions, Neovim is Vim. It is built for users who want the good
             : parts of Vim, without compromise, and more.
ap commented 6 years ago

That looks like a clear bug in Neovim. I’ll be filing a bug about it there.

As for the fix, I have to say it would pain me to apply this particular PR.

If that PR were the only solution then frankly I’d be inclined to handle it by waiting for the Neovims to fix it and then just telling anyone who encounters this problem to upgrade their Neovim.

However, the plugin already needs a mechanism to prevent it from activating at all in certain buffers anyway. If it had that, I assume that could be used to keep it from activating in these inccommand splits – at least for now, until the Neovims fix their bug. What are the values of filetype and buftype in that window?

ap commented 6 years ago

In fact, this appears to be neovim/neovim#8796 so there is already a bug and they’re on it.

So it appears that I don’t really need to do anything…

unclechu commented 6 years ago

@ap Well, as I tested filetype kept the same except bufname('%') is [Command Line].

Anyway, by my opinion, considering that there's no delivered Neovim stable release which fixes this bug it would be better to add these simple hacks (especially they aren't that bad, just affecting code styling). Waiting for a fix (which usually takes months for new stable Neovim release) just because you don't wanna add hacks (which could be removed later, when new stable release of Neovim come) for a bug is bad for user experience. Is Neovim users supposed to be annoyed for months by this error messages or even urged to not use this plugin to avoid that? I mean I'm personally don't care, I don't mind maintaining my own fork for myself, but just in general?

ap commented 6 years ago

Please read again. I said I am open to a temporary bandaid, just a different one, if at all possible.

What buftype do you get?

ap commented 6 years ago

(Also: neovim/neovim#8796 is in the Neovim 0.3.2 milestone which is supposed to be due in September, so if their milestone due dates actually track reality to within a reasonable window, then we’re only looking at weeks, not months… which is why I’m even considering doing nothing. (Generally I am entirely on board with working around an upstream bug for the benefit of downstream users. I just also have other lesser priorities.) But, I am also aware that the early September intent might just be a fantasy.)

ap commented 6 years ago

Hmm. I’ve downloaded Neovim to get to the bottom of this myself but so far I can’t reproduce the bug. I’ve opened a CSS file, gotten CSS Color to load and colourise the buffer, set icm=split, and then typed :%s/border, causing (in my case) a [Preview] window to pop up, previewing the results of my command as I continue to type. However, the contents of that preview window are not syntax highlighted at all, for me, and (presumably for that reason) I also don’t get the error.

Am I missing any obvious step? If not, which step is it where you guys get a different result?