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

Unibeautify integration #7

Closed
lassik opened this issue Jun 26, 2018 · 6 comments
Closed

Unibeautify integration #7

lassik opened this issue Jun 26, 2018 · 6 comments

Comments

@lassik
Copy link
Owner

lassik commented Jun 26, 2018

The Unibeautify folks are very friendly and responsive and would like to hook into Emacs. So let's do it!

Initial discussion here: Unibeautify/cli#17

Ping @Glavin001 :)

@Glavin001
Copy link

I can try to make Unibeautify's installation easier, as described in Unibeautify/cli#17 .

Would you be able to work on the Emacs package? I created a repository and provided you with write access: https://github.com/Unibeautify/emacs
If you're able to install the current Unibeautify CLI, let's make the Emacs package assuming unibeautify executable is already installed and exists. I'll work on improving that process.

@lassik
Copy link
Owner Author

lassik commented Jun 27, 2018

I can try to make Unibeautify's installation easier, as described in Unibeautify/cli#17 .

That's awesome!

Would you be able to work on the Emacs package? I created a repository and provided you with write access: https://github.com/Unibeautify/emacs
If you're able to install the current Unibeautify CLI, let's make the Emacs package assuming unibeautify executable is already installed and exists. I'll work on improving that process.

Thanks for granting access.

I was under the impression that we were adding Unibeautify support to format-all. I still think that would be a better solution than creating a separate package. We have a ready and tested framework into which we can easily add Unibeautify support and there's no need to throw away all the old work on calling other formatters. I believe not all Emacs users want to or are able to install Unibeautify right away so the old code in format-all would continue to serve them. Meanwhile, everyone would get Unibeautify support with the next update to format-all.

Before I started format-all, there were a dozen or so formatting packages for Emacs - a different package for autopep8, clang-format, prettier, rubocop, rufo, etc. All need to be installed separately and have a slightly different user interface. They all do the same thing with small variations but are different packages mainly due to historical intertia. I would like to avoid adding to this duplication of code, UI and effort in the Emacs package ecosystem.

In my opinion a good compromise would be to make Unibeautify the default formatter for format-all. I would happily do this. So if Unibeautify is installed, format-all would use it - if not, it would try to directly call the other external formatters that it already knows about. (There is no incentive for me to add new formatters to format-all once Unibeautify is working well, but I imagine someone would send the odd pull request once in a blue moon.)

The big thing that Unibeautify brings to Emacs (in addition to a wide range of formatters) is configuration. My original goal was to avoid inventing our own Emacs-specific configuration system (this problem is explored in painful detail in issue #2). Unibeautify solves the problem beautifully by providing a .unibeautifyrc file ready to use. format-all doesn't currently have any configuration options at all. We can keep it that way by telling people to simply use Unibeautify and .unibeautifyrc if they want to configure their formatters. Meanwhile, many of the already supported formatters (such as autopep8 and gofmt) are very useful with their default settings so people who want the simplest possible solution can keep doing that if they want to.

One thing I would be willing to consider is changing the name of the Emacs package. I never thought that format-all is a particularly good name - it began by adapting an internet meme ("format all the code") which I got tired of so I shortened it to format-all, lol 😃 We could rename it to beautify, for example. There isn't yet a package with that name in MELPA, the main third-party Emacs package archive. I would also be happy to mention Unibeautify in the one-line package summary, so people looking for beautify or unibeautify on MELPA will easily find it. I personally think that format is a better term than beautify because it's easier to spell and more neutral (people who don't like their team's coding style would think that it uglifies their code) but I don't truly care about this - either word is fine.

I'm not familiar with the Vim community but I imagine a similar approach would serve them well - they already have a widely used plugin called Neoformat. Unibeautify could be added to that one instead of making an entirely new plugin from scratch. But we should ask @wbthomason since he knows about Vim and Neoformat.

@Glavin001
Copy link

In my opinion a good compromise would be to make Unibeautify the default formatter for format-all. I would happily do this. So if Unibeautify is installed, format-all would use it - if not, it would try to directly call the other external formatters that it already knows about. (There is no incentive for me to add new formatters to format-all once Unibeautify is working well, but I imagine someone would send the odd pull request once in a blue moon.)

👍 This does sound like a good compromise 😃 .

One thing I would be willing to consider is changing the name of the Emacs package

No worries here. I think you should continue to maintain your format-all Emacs package.
I want to house all officially supported Unibeautify tools within the organization ( https://github.com/Unibeautify/unibeautify ) with a name containing the Unibeautify brand. With the exception of Atom-Beautify, since it has over 4.7 million downloads with this name.

Ideally, the name and installation is fairly consistent:

# Node.js
npm install unibeautify
# macOS
brew install unibeautify-cli
# VSCode
code --install-extension Glavin001.unibeautify-vscode
# Sublime Text (via Package Control)
# --> Package Control --> Search for "unibeautify" --> Install
# Vim
:Neoformat unibeautify
# etc ...

The Unibeautify Emacs package will be extremely lightweight and simply leverage unibeautify-cli. After the initial Unibeautify Emacs package is developed I do not expect many changes in its future, as the vast majority of work and complexity will be within Unibeautify core and Unibeautify CLI.

Since you already have developed out this excellent package, format-all, there's no point in deprecating it, since it does fit a need and there will be some users who do not need the additional features Unibeautify offers. 👍

@lassik
Copy link
Owner Author

lassik commented Jun 27, 2018

With the exception of Atom-Beautify, since it has over 4.7 million downloads with this name.

Wow, that is very impressive. Atom is really taking off!

I thought about this some more and had to change my mind. I now agree with you that having a separate unibeautify Emacs package is better after all, because it turns out that full Unibeautify support is fundamentally more complex (because it does more useful things to users!) than the zero-configuration stuff that format-all currently does.

For the best Unibeautify support in Emacs, we should:

  • Automatically load Emacs coding style settings from .unibeautifyrc when opening a file.
  • Automatically format code before saving a file.
  • Have a command to format a piece of code in a temp buffer (not saved to any file), sending the coding style settings used in that Emacs buffer to Unibeautify via command line arguments.
  • Have a command to create or update .unibeautifyrc based on current Emacs coding style settings.

All of these are hard to fit in the current format-all package without confusing users.

  • For example, suppose a user sits down on a new computer that has Unibeautify. They are used to running clang-format and expect it to read its settings from the .clang-format file. But now the new computer has Unibeautify and format-all would cause clang-format to be invoked via Unibeautify instead, so clang-format would use the settings from .unibeautifyrc (or default settings if .unibeautifyrc does not exist).

  • Also, auto-formatting on save is something that's easy to do in a dedicated Unibeautify Emacs package (by only applying it to files that have a .unibeautifyrc in their project directory) but cannot be done in format-all without confusing users. Likewise, auto-loading settings from .unibeautifyrc might be confusing in a generic formatting package, but expected in a dedicated Unibeautify package.

I also looked at the current Emacs Lisp code for format-all and the part that does the actual formatting is a very small amount of code. So I could volunteer to kick off the Unibeautify Emacs package based on that code. After all this reflection, I think it's the right thing.

@Glavin001
Copy link

So I could volunteer to kick off the Unibeautify Emacs package based on that code. After all this reflection, I think it's the right thing.

Thank you in advance for your contribution! I am excited to have you on board 🎉 😃 . Together let's build the best user experience for code formatting!

All of your comments above make good sense and we can continue our discussions over at https://github.com/Unibeautify/emacs/issues 👍

@lassik
Copy link
Owner Author

lassik commented Aug 14, 2018

This project is continued at https://github.com/Unibeautify/emacs :)

@lassik lassik closed this as completed Aug 14, 2018
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

2 participants