mxw / vim-jsx

React JSX syntax highlighting and indenting for vim.
1.59k stars 95 forks source link

Remove pangloss/vim-javascript dependency? #104

Closed pdf closed 8 years ago

pdf commented 8 years ago

Is there any chance of removing the dependency on pangloss/vim-javascript? It doesn't seem to do a very good job, especially with ES6/ES7, whereas stuff like othree's plugins do an excellent job. Unfortunately trying to use this plugin with other javascript-related plugins results in broken indentation and completion.

mxw commented 8 years ago

So, pangloss/vim-javascript is a dependency mostly out of convenience; in particular:

I also don't do much work in JS anymore, so I haven't noticed any issues with vim-javascript (it certainly worked very well for me when I //did// do a lot of JS work).

I'm not sure I want to publicize alternate dependencies (though I'm open to the idea)—but I am happy to support them in practice, in the implementation. You should feel free to submit a PR which makes stuff work with yajs. I'm surprised that indentation is very broken with it—I would expect it to only be a problem at the end of a {} JSX child region, since that's the only thing with an explicit syntax element dependency in the indent file. Unless yajs has wildly different default indentation style (...does it? if so, why?), I think you just need to fix up SynJSXBlockEnd().

Not sure why completion would be broken, unless yajs just isn't robust to intervening syntax elements, since vim-jsx doesn't do anything with omnifunc.

pdf commented 8 years ago

Thanks for the detailed response. The issue is that multiple javascript plugins don't co-exist well, and if you disable vim-javascript, then indentation is broken. As for completion, there's #103 for example.

Ignoring the filetype change, looks like #105 is enough to fix indentation.

amadeus commented 8 years ago

@pdf I'd be curious to hear what vim-javascript doesn't do well with ES6/7. We've made some major changes recently to improve ES6/7, and I'd argue we probably do a better job than most if not all other syntax plugins.

pdf commented 8 years ago

@amadeus it's actually not bad now that I take a closer look, apologies for disparaging your work, that was not my intention! I think it's mainly just different choices that I find jarring or lacking after a long time with an alternative syntax, and I'd really like not to be forced to change globally for a sub-syntax. My goal here is mainly to have vim-jsx not break javascript if vim-javascript is not installed, so that people have a choice.

yajs: screenshot_yajs vim-javascript: screenshot_vim-javascript

amadeus commented 8 years ago

Noooo worries. I didn't take it that way at all. Just poking around to make sure there aren't issues we haven't caught. I think a lot of the differences you see in those screenshots are due to linking groups. We have far better granularity in our matching than yajs, (I think), however your theme doesn't necessarily hook up and highlight those things. Just an example if I re-type a bit of your code using my theme:

example

mxw commented 8 years ago

One of the reasons I like vim-javascript (and the reason I chose it as my dependency (well, okay, the real reason was that I didn't want to test anything else, and at the time I don't think there was anything else worth using)) is that its highlighting is pretty minimalistic. I don't like highlighting delimiters, e.g., because I find it detracts from the "meat" of the code.

Obviously, a sufficiently rich set of syntax elements should make it easy to customize highlighting, so I guess I'm just saying I like vim-javascript's defaults.

Thanks for chiming in, @amadeus! Thanks for keeping vim-javascript up to date, and thereby also validating the choices made by past me :]

amadeus commented 8 years ago

I'd say in the past 3-4 months, we've add a TON of syntax granularity (i.e. we know things like whether the brackets you created are an object, or a switch case, or an if case, or a for loop, or an arrow function, etc), however a good amount of that is not actually linked up to things, or if it is, it's linked to a special group we created, called Noise. Colorschemes don't usually link up to Noise so therefore you won't see many changes. I think we did achieve a solid balance of linking the major things, and then linking the rest to Noise for more generalized matching or giving colorscheme maintainers the ability to get really fine granularity.