calebsmith / vim-lambdify

Vim plugin that conceals lambda/function syntax with lambda characters
BSD 2-Clause "Simplified" License
52 stars 8 forks source link

Not working on JavaScript-Files / Error in javascript.vim #1

Closed webgefrickel closed 9 years ago

webgefrickel commented 9 years ago

When I open up javascript files after having installed this plugin with neobundle, I get the following issue:

Error detected while processing /Users/webgefrickel/dotfiles/vim/bundle/vim-lambdify/after/syntax/javascript.vim:
line    7:
E28: No such highlight group name: javaScriptFunction
Press ENTER or type command to continue

What kind of syntax-file for JS do you use? I don't seem to have the correct highlight-group defined...

BTW: nice idea :-)

calebsmith commented 9 years ago

Nice find. I based it off of the javascript syntax file that came with my local checkout of vim 7.4. I'd bet 7.3 uses a different highlight group name and the fix would be to check for which one to use.

The file I'm referring to is in vim/runtime/syntax/javascript.vim . Lots of folks have 7.3 and that is when the conceal feature first existed so it'd be good to fix this.

Thanks again for finding this issue and I'll try to dig into this soon

calebsmith commented 9 years ago

My first guess might be wrong. It appears the same match group is used in 7.3:

https://github.com/b4winckler/vim/blob/hg/vim73/runtime/syntax/javascript.vim#L62

I need to look a bit closer at this. Thanks for filing the issue

calebsmith commented 9 years ago

Another guess, try replacing every instance of javaScriptFunction with Function in that file. Could you paste or link your vimrc? I tried removing large parts of mine to reproduce the issue but to no avail.

calebsmith commented 9 years ago

I can't seem to find a way to check if a group exists in the syntax help.

I took the liberty of looking at your dotfiles repo real quick. I think that the vim-javascript plugin you're using has different names than the default one I used to pair mine against:

https://github.com/pangloss/vim-javascript/blob/master/syntax/javascript.vim#L264

Based on that, I think it might need to check against Type rather than Function

calebsmith commented 9 years ago

After a bit of investigation and reading, I was able to get it working with or without the pangloss/vim-javascript syntax. I've tried to make it match for lambdas and work regardless of syntax customizations.

I went ahead and dropped this in master and it clears up the issue for me. It's a pretty critical bug so I'm updating the release at vim.org

Thanks a ton for reporting this. I learned a lot in the process of fixing it.

webgefrickel commented 9 years ago

Thank you very much for looking into this issue that fast (and intense)! I very much appreciate it and of course its fine to have a look at my weird dotfiles :-)

I pulled the current master, and the lamdbification works fine now! Woot!

But, sorry to tell you that: syntax highlighting is now completely broken :-/

It now looks like this:

screen shot 2014-09-23 at 15 44 42

but should look like this (well, with the lambdas replaced as above, of course):

screen shot 2014-09-23 at 15 45 17

I am using solarized and vim terminal v 7.4.383 (or macvim -- same issue).

I have absolutely no clue about vim syntax-coloring whatsoever, but I tried with a very minimal vimrc:

set nocompatible
filetype plugin indent on
syntax on

colorscheme darkblue
set background=dark

and the result was the same — should be sth. like:

screen shot 2014-09-23 at 15 55 12

but with the plugin-contents then copied to ~/.vim/after it looked like:

screen shot 2014-09-23 at 15 56 39

I then fiddled around with your plugin and uncommented these lines

syn region jsExpression start=+(+ end=+)+ contains=jsNiceFunction
syn region jsBlock start=+{+ end=+}+ contains=jsNiceFunction

which at least restored most of the syntax-highlighting, but (well, as expected) only conceals the first function right after module.exports = ... Every function declaration in this block will still not be concealed.

Any ideas on this? And thanks again for your time!

calebsmith commented 9 years ago

Wow, thanks for the details. I shouldn't have closed the issue after all.

So it appears my guess at using those regions is overriding the syntax rules from elsewhere. With the default javascript syntax file, those region commands aren't needed, so I've been looking over vim-javascript and trying to grok what is going on and how to apply the conceal rule to functions regardless of if they are contained within blocks or expressions.

I'll have to take a deeper dive into this and fix it for real. I'm guessing there is a way to do something like the above syn region ... code, but have it add onto and not override the syntax rules for it.

Also, thanks for the screenshots. If you do feel like fiddling with it further, the Makefile should help with the common tasks of overwriting your ~/.vim with the local checkout to test it out.

calebsmith commented 9 years ago

I think I found the fix. If you replace the original line with::

syntax keyword jsFunction function conceal cchar=λ

and get rid of the sny region commands, it should work properly with or without vim-javascript loaded. I'll likely push this up to master later.

Will wait to confirm it works for you too before marking a new release.

Thanks for the help

calebsmith commented 9 years ago

I made the above change to master. If you can pull this in, test it out and confirm it works for you too, I'll close the issue. I'm able to see the correct syntax highlighting on my end, as well as the lambdas in all contexts.

HerringtonDarkholme commented 9 years ago

@calebsmith Maybe putting some images on the README is a good idea? I could not imagine what the effect of this plugin until I saw @webgefrickel 's beautiful screenshots ;). One picture worth more than hundred lines of description.

webgefrickel commented 9 years ago

The current version of master is perfect! Very nice :-) Thanks a lot and feel free to use the screenshots :-)

screen shot 2014-09-24 at 13 03 45

calebsmith commented 9 years ago

Excellent. Thanks for testing. I'll tag the release soon.

@HerringtonDarkholme Yes, thanks for the suggestion. I've been thinking that was a good idea, so thank you @webgefrickel for offering a screenshot.