Doesn't highlight at all in Linux gnome-terminal about vim-css-color

[daniel-walker]

A bisect show that this commit breaks it,

commit 3ad55a6
Author: Aristotle Pagaltzis [email protected]
Date: Fri May 20 12:41:28 2011 +0200

self-modify code to avoid the GUI-or-256term check

[daniel-walker]

i was able to fix it by duplicating the functions where the substitution happens, which works but isn't a final fix.

[ap]

Excellent work, thanks! I wouldn’t have known how to track this down.

What exactly did you do in your version? (Can you gist it maybe?)

[daniel-walker]

not sure what gist is.. In the commit that broke the highlighting it was redefining the two functions s:MatchColorValue, and s:MatchColorName .. All I did was actually do the redefine at that point instead of relying on the regular expression. I couldn't find anything wrong with the regular expression, but it seemed that trying to do the redefine that way just didn't work for some reason.

Here's a patch that makes the changes I did (it's pasted tho, so might have corrupted whitespace)

http://pastebin.com/SGqJfeV6

[ap]

“Gist” is the Gist link in the top bar on every GitHub page… it’s GitHub’s own pastebin. (Git-backed, too.)

[ap]

I see what you did. Hmm, can you add a echoerr source before the exe source line and check what the output looks like? (It should produce a big error message with the content of the variable when when you open a CSS file. You will probably have to Ctrl-S your terminal to keep the error message from disappearing after a second.)

[daniel-walker]

Here's the output.. I used an "exe apend("$", source)" instead
..
https://gist.github.com/2283371

[daniel-walker]

I also did some processing on the output, it didn't originally have any newlines (one big line).

[daniel-walker]

I did a little more work on this, and I discovered that the highlighting does work in some cases .. It's spotty tho. If I open a new file with a .css extension and type ".class { color: white; }" I get nothing. If I close it and re-open I still get nothing. But if I type ".class { color: white; color: #fff; }" when i type the "#fff" suddenly it highlight both colors. But if I type ".class { color: green; color: #fff; }" only the "#fff" gets highlighted. If I type ".class { color: green; color: #008000; } again both colors get highlighted.

It's seems like most of the #RRGGBB and #RGB, rgba get highlighted.. Just not the word based ones, unless you type in the same color as #RRGGBB then the words also get highlighted.

[daniel-walker]

I made a couple of patches which 1) fix the issue here 2) speed up loading the script

This is the first,
https://gist.github.com/2314586

This makes it so the script just runs some of the code as precomputed instead of calculating at runtime. It speeds up the loading of the script by about %19 in a vim terminal and %28 in gvim. Not sure why they're different tho.

It also fixes the issue with the self modifying code.

[ap]

You can drop all the exes and just write the commands directly. You can also roll them up by specifying the cterm* and gui* colours in the same highlight.

I’m unhappy about dropping the self-modification code. Vimscript is so slow that it made a measurable difference when the conditional is compiled out, and this plugin stretches the highlighting facilities of Vim beyond what they were meant to cope with.

I’m still mulling this over, am unsatisfied with all the options right now.

[ap]

Also I note that the code produced in https://gist.github.com/2283371 looks correct. It should work…

[daniel-walker]

I sent you an email with the changes, but those changes were prior to me reading this comment. One thing to note is that this change makes loading faster than using the self modifying code. In fact I think this change speeds things up because your not making all those function calls to print out the colors..

I'm not sure what you mean by "... roll them up by specifying the cterm* and gui* colours in the same highlight." could you explain that a little more?

[ap]

Thanks for the patches, nevertheless. I prefer pull requests (I’m not a kernel hacker :-)) but I can deal either way.

The function that gets recompiled is actually called as you type, and will sometimes add new highlights as you type. That is why I want it to contain the least amount of logic possible. Whether vim is running on a terminal or not is a condition that will never ever change during the runtime of the script, it is a waste to keep checking it over and over and over. But – I don’t have to recompile it per se… I could just delay the execution of the definition or even (last resort) just put two copies of it in the code and just pick one of them.

I’ve already pushed a commit that rolls up the highlights – look at 2d3de31 to see what I meant.

[ap]

I’ve pushed two more commits which add the other precomputed tables as in your patches but written a little differently.

I’ve left the self-modifying code in, though. If you test this version, do you still get no highlights?

(If the self-modification is the problem (which I don’t understand why it should be), this should still not work (in which case I still want to know why it doesn’t work when everything looks like it should).)

[ap]

Actually, scratch that. In 7e11e6d I flipped it over so that the xterm code is now the regular version and the code self-modifies under GUI, rather than vice versa. That made the self-patching code easier to read, so a win by itself, but it should now give you no grief as well.

After that, I realised there was another easy optimisation to do, by making s:FGForBG return the right kind of value itself depending on GUI vs console.

So the code is now shorter, easier to read, and much faster.

Please test current master and let me know if it works for you.

[daniel-walker]

Yeah , it seems to work with the current master. I just sent you patches for review, not inclusion.. I didn't want to send a pull request if the code wasn't something you wanted to pull.

The thing with the function that your self-modifing, it doesn't seem all that time critical to me. I guess it's a bonus if you can get the self modification to work.. I didn't want to tackle that problem.

[ap]

It’s true that it’s a lot less critical now that the constants are all precomputed, and if it had not been possible to retain it I would have dropped it next up. I am glad I did not have to, though; excellent that it works for you now.

Thanks a lot for your report and your detective work.