ap / vim-css-color

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

E216: No such event: <buffer> call css_color#reinit() #145

Closed rickalex21 closed 3 years ago

rickalex21 commented 3 years ago

Hello, thanks for making this plugin. I created a similar one but I toggle it on and off with a command. Wouldn't it be much better from a performance point of view to use toggling instead of CursorMoved, CusorModedI ? Maybe some feature options like g:css_enable_toggle = 1. I read someone with a similar issue #126 . When you toggle on, create the groups. When you toggle off delete them. What do you think ?

for color in css_colors
syn clear color
endfor

I'm pretty sure, this has something to do with my configuration because I tried with another user account and I don't have that problem. When I comment line 242 in css_color.vim it works fine:

autocmd ColorScheme <buffer> call css_color#reinit()

Any pointers would be appreciated thanks.

ap commented 3 years ago

Wouldn't it be much better from a performance point of view to use toggling instead of CursorMoved, CusorModedI ?

So every time you type a color definition that didn’t appear in the file so far, you have to toggle the plugin on and off for it to find that color? How are huge files going to be handled – if you have a 100,000-line file loaded, it has to be parsed entirely every time you toggle the plugin on?

Frankly I’d be embarrassed to suggest this as reasonable to anyone. Much less add code to put in an option for this plus the documentation for that option and then having every user confronted with it and supporting it forever. That’s not to say that if you personally want to work that way, it’s not valid, it’s just not something I’d want to suggest to anyone who doesn’t (like you) already want it.

If the plugin already had a reasonable mechanism so you could ask it to not always get enabled, this could be done without another configuration option, since all you need is ⓐ not adding the autocommands and ⓑ a variant of parse_screen that just parses the entire file – packaged together in e.g. css_color#parse_once maybe. I might consider that when I get there.


As far as your actual bug – what does :echo exists('##ColorScheme') say?

rickalex21 commented 3 years ago

So every time you type a color definition that didn’t appear in the file so far, you have to toggle the plugin on and off for it to find that color?

No, I’m talking about toggling the existing colors which are the basic and extended. I don’t know of a situation in which a user would have a 100,000 line file that requires looking at colors? Assuming that you’re talking about css files, most css files are small are split up into parts and sourced from the html but to push the limits I decided to add 36,000+ lines to my personal plugin that I created last year to show you. It's really smooth, the gif is just slow.

Colors Plugin

Really what I’m trying to figure out is why are those functions in your plugin being called several times? Why not just call them once and be done with it instead of using autocommands? Is it because of a syncing problem with syntax? It looks like the groups are being created then matched on a cursor moved per line basis. Are you only parsing part of the file? Then if that's so it would make sense. The decision in your plugin was to 1. highlight the entire file or 2. Use an autocommand to highlight part of it?

    autocmd CursorMoved,CursorMovedI <buffer> call s:parse_screen() | call s:create_matches()
            autocmd BufWinEnter <buffer> call s:create_matches()
            autocmd BufWinLeave <buffer> call s:clear_matches()

Frankly I’d be embarrassed to suggest this as reasonable to anyone.

You’re right, that’s probably not a good idea, options would probably just clutter things up and then you have to make docs for it.

As far as your actual bug

There was a plugin that named an autogroup to ColorScheme. I fixed this problem by renaming the autogroup. I’m sure this post will help someone in the future when they come across this problem.

since all you need is ⓐ not adding the autocommands and ⓑ a variant of parse_screen that just parses the entire file – packaged together in e.g. css_color#parse_once maybe. I might consider that when I get there.

I think this is what I was getting to, not sure.

ap commented 3 years ago

The functions are called repeatedly because the user is presumably, y’know, editing the file? And if they type a colour that wasn’t in the file before… well, how else is that colour supposed to get highlighted?

(It is also the case that the file is only parsed piecemeal, but that could be changed. The fact that the plugin has to keep watching the user’s input is fundamental. (At least given Vim’s static syntax highlighting system. If it was somehow possible to compute on the fly how to highlight something matched by a syntax rule, then much of this plugin would be unnecessary.))

There was a plugin that named an autogroup to ColorScheme.

Ouch. Yeah I’d consider that a bug in that plugin. Collisions like that are always possible, unfortunately, with the way that Vim parses autocommand-related commands, but at least staying away from autocommand names that Vim ships with should be a matter of course.

I’m sure this post will help someone in the future when they come across this problem.

Yeah. It seems like this should be among the first things to check whenever one runs into E216 – and :help E216, well… doesn’t.

rickalex21 commented 3 years ago

well, how else is that colour supposed to get highlighted?

I see what you're saying, instead of making all 16 million + colors which is not practical, the ones that are missing are created on the fly through the autocommand. Another alternative would be to regex only on the current visible screen instead of using the cursor move autocommand but not sure how hard that would be to be implemented or if it would have a dramatic impact in performance.

Ouch. Yeah I’d consider that a bug in that plugin.

Yea, I'm glad I figured that out quick and it will no longer be a problem for me or anyone else.

ap commented 3 years ago

Another alternative would be to regex only on the current visible screen

That’s how it already works.

instead of using the cursor move autocommand

How else does the plugin notice when you’ve scrolled and a different portion of the file is now on screen?

rickalex21 commented 3 years ago

That’s how it already works.

Yea, I see that. Through the use of autocommands.

How else does the plugin notice when you’ve scrolled and a different portion of the file is now on screen?

That’s the question that needs to be answered and if I ever find the answer I will let you know. Even if there was a way you still would need an autocommand or something to know when the user inputs a new color that’s not on the list. I don't know if you have measured to what extent those autocommands are taxing vim performance, hopefully it's not much. It's usually folding functions that can be taxing.

ap commented 3 years ago

The simple answer is to debounce the autocommand. As near as I can tell it should suffice to check whether the screen position and buffer content have changed since last time the autocommand was triggered.

And I have a patch for that.

But I’ve been running it locally for quite a while and have been seeing occasional glitches that feel like occasional over-debouncing. They’ve tended to occur when I’m busy working on something else, so I’ve never sat down to tackle them.

rickalex21 commented 3 years ago

They’ve tended to occur when I’m busy working on something else, so I’ve never sat down to tackle them.

I know how that is. Well, hopefully it will be an easy fix.