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

hard wrap lines + plugins #89

Closed
millermedeiros opened this issue Oct 30, 2013 · 11 comments
Closed

hard wrap lines + plugins #89

millermedeiros opened this issue Oct 30, 2013 · 11 comments

Comments

@millermedeiros
Copy link
Owner

we don't have an option to force line wrap after a certain amount of chars, this can be really useful depending on the code style but it's impossible to implement without loc and range info.

since we do a lot of transformations to the tokens we basically lose the range and loc info - or would need to update it for all nodes/tokens after each manipulation, which is very expensive.

I'm thinking in maybe 2 separate libs, one that can read the AST generated by esformatter.transform and can rebuild the loc and range info and another one that forces the line wraps at the optimal position based on that... We would import these external dependencies and use them (to simplify the process since this is a common need).

Maybe we should even add some way to import "plugins" on the config file and/or as cli argument, that way we don't hardcode anything that is developed as a separate project.

see #73 and #84

/cc @jzaefferer

@millermedeiros
Copy link
Owner Author

EDIT: ignore this comment, we have a better API below


I'm thinking something like this for the plugin API (all optional):

// executed before esformatter.hooks[node.type]
exports.hooksBefore = { ... };
// executed instead of esformatter.hooks[node.type]
exports.hooks = { ... };
// executed after esformatter.hooks[node.type]
exports.hooksAfter = { ... };

// executed before/after the esformatter.transform (before/after hooks)
exports.before = function(ast, options) { ... };
exports.after = function(ast, options) { ... };

and we could add it to the config file like:

{
   "plugins" : [
      "some-esformatter-plugin",
      "esformatter-hardwrap"
   ],
   "some-plugin": {
     "pluginSpecificSetting": true,
     "foo": "bar"
   },
   "lineBreak": ...
}

which would load these plugins from node_modules (similar to grunt.loadNpmTask).

the plugins would be executed based on the order in the array, that would allow some interesting things like multiple pre-processors.

@jzaefferer
Copy link
Collaborator

Since I opened #73 I guess I should support this. So currently I'm more inclined to tell users of the jQuery style guide to always use line breaks for object literals.

Something to revisit later, for now there's certainly more valuable issues to address.

@millermedeiros
Copy link
Owner Author

looking at the way I integrated esindent I think plugins should implement a setOptions method (optional) so that we avoid passing the options multiple times.

the plugins would have access to all the options (so they could potentially update the values). That would increase flexibility a lot.

Many users are asking for features that should not be part of esformatter and I definitely think a plugin API is needed. So I'm increasing the priority of this issue.

@shellscape
Copy link

Has any work been done on the plugin front as yet? (Don't see any commits referencing the issue #)

We're in need of some block formatting that definitely shouldn't be in the main lib. A plugin would definitely be the right way to go with it. I've been discussing contributing back to the project with my team and we're going to move ahead with it, but want to make sure we're not duplicating effort.

@millermedeiros
Copy link
Owner Author

no work on the plugins yet; the proposals above was just a rough idea that I had, not sure if that would be enough and/or if there is a better way to do it. pull requests and feedback are highly appreciated!

@shellscape
Copy link

We've started on a very generic plugin architecture that should allow for some nice post-processing, after the core formatting and transformation has been completed.

Once a plugin system does make it into the project, it'd be great to see a esformatter-contrib-* repo for plugins that could have frequent use. Something to think about.

@shellscape
Copy link

While this pull request (#130) is being considered, anyone needing immediate extension of the core options can write a plugin using our fork to handle whatever missing features they may need. For example plugins, look too https://gist.github.com/shellscape/8443537.

@millermedeiros
Copy link
Owner Author

some notes about what I think would be the ideal plugin API so far (all methods are optional):

// setOptions called once at the beginning to avoid passing options to all methods
// and also encourage plugin authors to cache/manipulate options only once
plugin.setOptions(options);
// string methods are basically `string.replace()` that happens on input/output
plugin.stringBefore(str);
plugin.stringAfter(str);
// called before/after each token manipulation
plugin.tokenBefore(token);
plugin.tokenAfter(token);
// called before/after each node manipulation
plugin.nodeBefore(node);
plugin.nodeAfter(node);
// executed after default changes
plugin.transform(ast);

notice that we don't pass any reference to the lineBreak, whiteSpace, indent modules and that we don't even pass a reference to esformatter itself, that is intentional. We don't want plugins to be tight coupled with a specific esformatter version.. - ideally a plugin should work on all versions, the only thing that should break the plugin is if we decide to change the plugin API and/or we end up changing the AST structure (which should not happen anytime soon).

If the plugin depends on a specific method from esformatter, it should add esformatter to it's package.json file and require('esformatter') themselves. - npm is great at handling dependencies, and we would allow plugins to work with any esformatter version.

I'm also thinking that we should allow some sort of pipe config to manipulate the string itself, just like a regular shell pipe (cat foo.txt | grep bar):

{
  // plugins are executed in the order they appear on the array
  "plugins": ["foo", "bar", "esformatter-gilt-functions"],
  // pipe is a simple way to "pipe" multiple binaries input/output
  "pipe": {
    // scripts listed as "before" will be executed before esformatter
    // and will forward output to next command in the queue
    "before": [
      "strip-debug",
      "./bin/my-custom-script.sh --foo true -zx"
    ],
    // scripts listed as "after" will be executed after esformatter
    "after": [
      "baz --keepLineBreaks"
    ]
  }
}

the idea of the pipe config is just so esformatter could be the entry point for multiple tools and they don't even need to follow the AST and plugin API, as long as they can handle string I/O. - that means you wouldn't need to create a custom plugin for your IDE just because you also want to use another tool together with esformatter.

I also think that the pipe should work like npm scripts (binaries installed locally with npm should be on PATH)

@millermedeiros
Copy link
Owner Author

ahh just reminded that we still need to find a good way to overwrite the indent rules! it would be possible to do it on plugin.transform(ast) but I think it might be too much overhead for the plugin author..

maybe pass the esformatter hooks on setOptions? that way plugin author could use some AOP and monkey-patch the getIndentEdges for the desired hook?

@shellscape
Copy link

It's absolutely fantastic that you're still putting thought into this and that you're trying to accommodate every possible need by plugin authors. But there reaches a point of overkill and over-complication, and I think you've passed that point. By a matter of practice the system on which a plugin runs should be simple.

@millermedeiros
Copy link
Owner Author

@shellscape I was focusing on other tasks/projects the past few months.. today someone asked about plugins and I just summarized all the comments that I added to your PR. My plan is to land official plugin support during June. - feedback and pull requests are highly appreciated!

I have a very ambitious goal with this project and I want the plugins to be flexible and at the same time keeping them as decoupled as possible from esformatter.. - I know it's hard to please everyone and that every choice has drawbacks, but I want to find a good balance and not settle for the first idea. - I tried to keep the configuration simplistic at the beginning and it was way hard to change it later, better to do it right once.

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

Successfully merging a pull request may close this issue.

3 participants