search

pangloss / vim-javascript

Vastly improved Javascript indentation and syntax support in Vim.
http://www.vim.org/scripts/script.php?script_id=4452
3.79k stars 358 forks source link

Switch to "very magic" for syntax regular expressions #813

Open TamsynUlthara opened 7 years ago

TamsynUlthara commented 7 years ago

I've recently started hacking up this plugin's syntax definitions to better support Flow typings, and in the process I've found it very difficult to read and write Vim's standard "magic" regular expression syntax.

Would the maintainers consider moving to "very magic" syntax? All it requires is prefixing all regular expression strings with \v, and you get something very close to standard PCREs. I'll volunteer to do the conversion if there's buy-in.

amadeus commented 7 years ago

I like the idea of this, however I fear it could lead down a bit of a rabbit hole.

Also, it would be one of those changes where we would have to halt all fixes and updates to the syntax file until it got merged in since it would be a serious pain to deal with merge conflicts.

Perhaps we start with just using it in the Flow file to make that easier and see?

I tried doing a little googling, but I couldn't seem to find an answer - does supporting very magic mean we could break older versions of Vim? And if so, how far back are we talking?

TamsynUlthara commented 7 years ago

I tried doing a little googling, but I couldn't seem to find an answer - does supporting very magic mean we could break older versions of Vim? And if so, how far back are we talking?

Extended search patterns were added in Vim 6.0, so yes, it would break anything older. Would that be okay?

If so, I'd be happy to run a conversion on the Flow file and introduce my Flow fixes that way.

amadeus commented 7 years ago

I'd say we go with the Flow changes for now, and hold off on making a decision on the rest of the file for a bit.

TamsynUlthara commented 7 years ago

I hate to come off as pedantic, but did you mean the rest of the files, as in changing the entire Flow file is okay? I just want to make sure I'm not doing more than I should there.

amadeus commented 7 years ago

Yup, changing the entire Flow file is fine. Since it's a plugin, I think it's ok to play pretty fast and loose with it since it's not a core JS thing.

Also, somewhat related to this, I put out a call for a test flow file:

https://github.com/pangloss/vim-javascript/issues/783

I think it would be good for us to maintain a test file that's in the repo exemplifying the various Flow possibilities as a quick way to eyeball changes and tweaks. I started putting something together a while back, but I haven't really been able to dedicate serious time to it as of late

TamsynUlthara commented 7 years ago

I wrote a NeoVim unit testing library to help me along when I was writing a couple of Vim plugins. I've been meaning to port it to JavaScript. Would that be helpful here? Granted, it only runs in NeoVim, but it helped me make steady progress while avoiding a slew of regression bugs.

amadeus commented 7 years ago

I personally don't use Neovim, so having that a requirement is something I am not a fan of. However, given this is for a syntax file? How would you propose unit testing it?

bounceme commented 7 years ago

https://github.com/junegunn/vader.vim is quite nice, I've made a few tests using it for neomake. also there seems to be syntax support: https://github.com/junegunn/vader.vim#example (at the bottom). maybe it works for you? I don't really have an opinion

TamsynUlthara commented 7 years ago

@amadeus NeoVim lets you drive it entirely via an RPC API, including standing up new, isolated NeoVim instances and tearing them down. This isn't something Vim can do, as far as I know.

As far as syntax, the README for neovim-unittest has some examples; a more extensive example would be the syntax tests for vim-earlgrey. In a nutshell, you can define a spec to match strings against, and ensure that each part of the string is in the correct syntax group.

I understand if you're not crazy about adopting a tool that doesn't work with vanilla Vim; I'd feel the same way about a tool that didn't work with NeoVim. ;-) Being able to run real unit tests is fantastic when you're dealing with a complex syntax, though.

@bounceme I originally looked into Vader, but I wasn't happy with how it worked. It doesn't automatically start and isolate a new Vim instance for you, I found the syntax awkward, and at least one of its "known issues" was a showstopper for me (i.e., that it doesn't correctly fire certain events).

We're probably going a bit afield from the original scope of this issue, though! If there's still some interest in adopting neovim-unittest, I'm happy to open an new issue for it.