Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improvments Ideas #4

Open
mihai-vlc opened this issue Sep 26, 2014 · 13 comments
Open

Improvments Ideas #4

mihai-vlc opened this issue Sep 26, 2014 · 13 comments

Comments

@mihai-vlc
Copy link
Owner

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

  • it should not run on automatically on save by default ! We should have something like Toggle (or Enable/Disable) auto jsfmt on save
  • Command palette to run jsfmt on the current file
  • Command palette to run jsfmt on the current selection
  • Format a new file(that is not saved yet) mostly the same code we will use for the selection
  • Maybe have a config file to allow the user to define custom file extensions

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
Copy link
Collaborator

I like 1 and 2. We could do those.

@paulirish
Copy link
Collaborator

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
Copy link
Collaborator

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

@mihai-vlc
Copy link
Owner Author

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

@mihai-vlc
Copy link
Owner Author

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

  • On pre save
    In the current implementation the commad is called on_pre_save
    this means that the file that gets converted is the one on the disk that does NOT
    contain the latest changes(since it's not saved yet). So we get the wrong result.
  • On post save
    We could move it on post save but the file will be continuously marked as modified.
    The file would get saved on the disk then jsfmt runs replaces the content in sublime
    and the file is marked as changed. This can be very confusing for the users even if we
    get the correct result.

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
Copy link
Collaborator

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
Copy link
Collaborator

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
Copy link
Collaborator

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
Copy link

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
Copy link

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
Copy link
Owner Author

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
Copy link
Collaborator

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

@paulirish
Copy link
Collaborator

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants