mihai-vlc / sublime-jsfmt

jsfmt plugin for Sublime Text
MIT License
477 stars 21 forks source link

Improvments Ideas #4

Open mihai-vlc opened 10 years ago

mihai-vlc commented 10 years ago

This seems like a nice plugin to work on.
I played a bit with it and I had a few ideas that I was thinking we can discuss here before making a pull request

Also there seems to be a problem on windows I will try to look into it later today.

What do you think about them ? Any others ?

paulirish commented 10 years ago

I like 1 and 2. We could do those.

paulirish commented 10 years ago

The selection ideas work for me too. Just not sure how well the upstream jsfmt project can support that. But I can see it be valuable yeah.

paulirish commented 10 years ago

Thx very much for taking a look and playing around BTW! :)

mihai-vlc commented 10 years ago

Ok then, I'll start playing with it this evening and make a pull request soon.

mihai-vlc commented 10 years ago

The jsfmt can't run as a commad in the form of jsfmt test.js Here is the reason why:

One solution would be to format run the content in a small js script.
And I made a prototype here The .jsfmtrc config still works as expected however the settings from the sublime-settings file have priority. The big downside here is that we need to have the node_modules/ in the repo and any plugins would have to be installed manually by the user in the packages folder.

Any ideas ?

paulirish commented 10 years ago

oh damn. this improvement is awesome. @sindresorhus was already telling me we'd have to move to the node_bridge and your implementation here looks on point.

The way he handles it in sublime-autoprefixer is including the node modules in the repo and package. I think that's okay. Once this is distributed via package control then it makes it easier on the user side to consume, right?

I believe we'll just have install via PC and the user only has to have node in their PATH, else we're good.

paulirish commented 10 years ago

any plugins would have to be installed manually by the user in the packages folder

There's only five plugins right now. I feel pretty comfortable bundling those (especially as quotes is so very important) in the package. Could we do that?

cc @millermedeiros

paulirish commented 10 years ago

I just pushed your changes up to the rewrite branch. https://github.com/paulirish/sublime-jsfmt/tree/rewrite

we can merge it into master soon

millermedeiros commented 10 years ago

maybe we could change the way esformatter locates the plugins (some option to set the plugin directory? always look for globally installed plugins somehow?). I think a lot of teams will end up implementing their own plugins since there is a limit to what can be done with simple config files (jquery preset is an example), so bundling all the plugins might not be a good option in the long run, specially because of version updates/bugfixes... (maybe just the "essential plugins"?) - to be honest I don't see it as a big problem that the user will need to install the plugins manually, as long as it is documented.

IMO the ideal would be to just use the globally installed jsfmt & esformatter binaries.. that way you don't need to update the plugin at each version change. (and globally installed plugins would also work without any changes)

/cc @jimfleming

jimfleming commented 10 years ago

If I remember right, selections would work if they're complete blocks and thus parseable by esprima. There's an upstream issue[0] for being more resilient to faulty/incomplete input.

@millermedeiros, I like that idea. If we could use the PATH binaries for jsfmt / esformatter (and their respective plugins) if they're present, else fall back to a bundled node_modules / plugins. Easy to get started for new users but less necessity to update the plugin with every version.

I'd also be ok with bundling the common plugins with jsfmt itself. We already bundle the braces plugin.

[0] https://code.google.com/p/esprima/issues/detail?id=130 (also links to autocomplete demo for partial input)

mihai-vlc commented 10 years ago

While the ideal solution to require the global package first and use the local one as a fallback is a good one I'm not sure on how we can implement that and make everything work right.

I think the best solution would be to a bundle with some essential plugins like @millermedeiros suggested and let the user make a manual install if he needs to add extra/custom plugins.

paulirish commented 10 years ago

@ionutvmi the rewrite branch has been merged into the master branch. I really like your approach here.

paulirish commented 10 years ago

I think the best solution would be to a bundle with some essential plugins like @millermedeiros suggested and let the user make a manual install if he needs to add extra/custom plugins.

totally agree.

@millermedeiros what's the best way for a team to define their own preset and manage that easily? Is there a better way than syncing up a bunch of jsfmtrc's per respository?