jwalton512 / vim-blade

Vim syntax highlighting for Blade templates.
226 stars 37 forks source link

Colon after todo comment #57

Open adriaanzon opened 8 years ago

adriaanzon commented 8 years ago

Because I added : to 'syn iskeyword' in 95809e7, todo comments (like TODO, NOTO, FIXME) aren't highlighted anymore, when they're followed by a colon.

This can be fixed by using syn match with a regular expression for bladeTodo, but I think adding support for colons in blade directives shouldn't interfere with other syntax groups.

So should we use a regular expression for bladeKeyword? Or does someone know a better solution? I was thinking about something like this:

syn match bladeKeyword "\v<\@(if|foreach|for|include|etc)>"

then you can add matches for directives with colons in them, without the need for : in 'syn iskeyword'.

adriaanzon commented 7 years ago

The sh syntax file shipped with Vim also introduced this problem at some point, and they fixed it like this:

syn match   shTodo  contained       "\<\%(COMBAK\|FIXME\|TODO\|XXX\)\ze:\=\>"
jwalton512 commented 7 years ago

@adriaanzon that solution seems reasonable to me.

jwalton512 commented 7 years ago

@adriaanzon i started tinkering around with trying to simplify/reorganize the code a bit today. while under the hood, i covered this colon issue.

if you get any time, could you take a look at this branch and let me know if anything seems out of whack to you? https://github.com/jwalton512/vim-blade/tree/rewrite

adriaanzon commented 7 years ago

First of all, I would avoid the multiple filetypes (php.html.blade). We tried it over at vim-vue, but it broke things like FileType vue autocommands, and a lot of little things that aren't really noticeable at first. I already noticed the indentation stopped working on the rewrite branch 😅

I actually had an idea some time ago about creating an autoload file, containing variables of all blade directives, which can be used in syntax, indent, and ftplugin, so we don't have to repeatedly specify the directives in all three of these files. That will also make it easier to add new blade directives: you'll only have to add them in one place. I started working on the idea so I had this draft laying around: https://gist.github.com/adriaanzon/9e51ce5a7ddbe41ea5ff6d38cc244119. Note that it's just a draft, some of it isn't even valid vim script yet :)

I'd also consider adding some vader tests, because not all faults are directly visible in the test.blade.php file.

jwalton512 commented 7 years ago

@adriaanzon argh, but that multiple filetypes feels so much easier 😅 😅 😅 . I like the idea of moving the directives and other things to a separate file.

I had checked out vader once, but never got around to actually writing up the tests, but I agree that would be much better than a test file.

adriaanzon commented 7 years ago

OK, I can work on implementing the variables in autoload directory, to use them in ftplugin/syntax/indent, and write vader tests in the process.

I also updated the gist so it's all valid vim script now :P https://gist.github.com/adriaanzon/9e51ce5a7ddbe41ea5ff6d38cc244119/revisions

jwalton512 commented 7 years ago

@adriaanzon thanks for all your contributions as always. i still haven't abandoned this rewrite branch as of yet. i started going through the indent logic, and got some pieces of it working.

adriaanzon commented 7 years ago

I quickly wrote an implementation here: https://github.com/adriaanzon/vim-blade/commit/5d84a80b3a0b5a24a72d847716baae84f6a839fd. Feel free to use it (or parts of it) in the rewrite branch.