-
Notifications
You must be signed in to change notification settings - Fork 108
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
Multiple formatters per language? #6
Comments
Thanks for chiming in and offering to help :) It was only a matter of time before we'd have to deal with this issue. I've been putting it off since configuration is such a quagmire (the gory details are in #2). For this issue,
What do you think about these approaches? Eventually I think we should just have to bank on |
By the way, if there's a specific formatter that you're missing, please mention it in #5 :) Even better if you can provide a PR using the current framework. The data model already supports multiple formatters (see |
For reference, here's how Unibeautify picks a formatter in their YAML config file:
Honestly, I like the way they do it (and the fact that they've done our work for us :p) and I'd just like to adopt that config file immediately if I was confident that it would eventually win the "config file wars" ;) |
Thanks for the responses! I have a few thoughts (responding to your comments from last to first):
In the vein of my last response, I agree with your concern in (1), but I don't think this is a major problem. If the user installs another formatter, so long as there's a way to select the original formatter, there's no unavoidable unexpected behavior. I like the idea of having a default formatter per language and allowing invocation of the command to select another formatter. The plugin could try the formatters in a list defined for each language in sequence, beginning with the default, until one succeeds, and remember which formatter was the first to run correctly (I'll note that this part could have some unexpected behavior if the reason earlier formatters failed was e.g. syntax errors that a later formatter didn't get caught on, so maybe it's best to not store the first to succeed...). To make life easier for those who prefer a non-default formatter, you could make a customization group with the defaults for each language, and persistently set language defaults that way. This mechanism is pretty similar to how Neoformat works - not that they're the gold standard, but (I'm coming from Neovim, if that wasn't clear :p), they do provide a very nice turn-key experience for both out of the box formatting of files and customization of the formatting. tl;dr: I agree with you that the philosophically best thing would be a unified configuration file format, but I think that the best current action is to add Emacs configuration options that can be used to load configuration from a file once a standard format emerges. It's better to have some configurability now and add to the options later (e.g. start with only selecting formatters, then maybe in the future allow settings like tab width, etc.) than it is to wait until the perfect solution is viable to have any configurability, imho. Of course, this is all just my long-winded opinion! Thanks for the great package either way! |
I'm happy to hear that the package is also useful to others :) No problem with the lengthy post, I appreciate the detail! Thought about this some more, I'll cave in and implement the following interim solution if it's ok with you:
I looked at neoformat and unibeautify, and they both allow specifying more than one formatter per file for one language. If I understood correctly, Neoformat picks the first available formatter from that list. I don't know what unibeautify does. Anwyay, I think this is misguided - formatting should be deterministic, and the best way to ensure that is to stick to one particular formatter per file. In theory, two formatters can produce the same result if they are both configured to do so, but in practice formatting is so complex that I expect this will rarely be the case. In fact, different versions of the same formatter are likely to have subtly different output. To clarify, picking the default formatter from a list of several ones may be okay design (for the sake of convenience), but it the user explicitly chooses a formatter, in my opinion they should choose only one. |
That sounds like a very reasonable interim solution. If I can help with a PR, I'd be happy to! Thank you! |
Go ahead with the PR :) If you feel like adding some of your favorite formatters too that'd be awesome! I'm holiday right now and will be offline for a week but I'll be sure to take a look at it when I return. |
Ok! I am in the middle of a crunch period for work, but I'll do what I can afterward! |
Re: My previous comment:
Apparently Unibeautify/atom-beautify have an issue discussing this at length: Glavin001/atom-beautify#457 Implementation: Unibeautify/unibeautify#4 I had misunderstood their idea. When they specify multiple formatters for one language, they want to run all of them every time (presumably in the order given). That makes sense. (Ideally there would be one formatter per language that did everything, but there is not - JavaScript is given as an example where some people use a chain of multiple formatters to format different aspects of the code.) Neoformat is able to run all the given formatters in a chain too. From their README:
I guess we should implement chaining also (but I'd like to leave out the possibility of stopping after the first formatter, per the reasoning in my original comments). The variable could be named There seems to be a somewhat widespread convention of calling the tools beautifiers but I think that's a bit subjective - many people who don't like their project's elected coding style would probably consider them uglifiers :p Formatter is a more neutral term (and also easier to spell). |
I talked to the Unibeautify folks and we are starting to plan Unibeautify integration (#7), which is great, but it makes this multi-formatter issue even more complicated than it already was 😄 We could simply tell people who want to use chained formatters, non-default formatters and/or non-default settings to use Unibeautify and fill in a |
Wow, alright! 😄 Perhaps for now it is best to wait for the Unibeautify changes/integration to solidify, before continuing to implement this feature (if it's even necessary)? |
I agree. Best case, we don't even need to implement user-selectable formatters and can rely entirely on Unibeautify to do it. What we could do is make Unibeautify the default formatter for the languages that it supports, and use the existing formatter specifications as a fallback in case Unibeautify is not installed. That ought to satisfy Unibeautify users, while also giving at least something useful to people who don't want to adopt it (yet). Do you happen to have any contact with the Vim Neoformat folks? I wonder what their stance on Unibeautify is. I guess Vim is the closest major editor to Emacs in terms of user base preferences (lots of old-school Unix heads, as opposed to the web-centric Atom and Sublime etc. :p) |
Multi-formatter support is now implemented in the |
format-all now supports multiple formatters [1], [2], [3]. [1]: lassik/emacs-format-all-the-code@beefe5e [2]: lassik/emacs-format-all-the-code#6 (comment) [3]: lassik/emacs-format-all-the-code#27 (comment)
The equivalent plugin for Neovim, https://github.com/sbdchd/neoformat, configures multiple formatters per language. The main benefit is that the plugin is still functional with a more general set of formatters, but it also enables easy choice of formatter per project, etc.
This seems like it would be easy to implement as an extension to
format-all-formatter-for-mode
- you could return a list of the formatters for a major mode, and iterate through them informat-all-buffer
until one succeeds. If the user invokesformat-all-buffer
with an argument naming a specific formatter, then you would either just run that formatter (if it's in the defined list) or first check that it's relevant to the current major mode and then run it.If you like this feature, I'd be happy to submit a quick PR implementing it as described above!
The text was updated successfully, but these errors were encountered: