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

the future of emacs, python, and black #25

Closed
wbolster opened this issue May 3, 2019 · 10 comments
Closed

the future of emacs, python, and black #25

wbolster opened this issue May 3, 2019 · 10 comments

Comments

@wbolster
Copy link

wbolster commented May 3, 2019

first of all, thanks for writing this package!

i used blacken.el in the past, but i stopped using it recently since i think there are better options available to me… please allow me to explain. :)

a while ago i wrote black-macchiato to allow formatting of partial files using black. (see also #11.)

to integrate black-macchiato with emacs, i wrote black-macchiato.el, which currently lives as a private minimalistic emacs package inside my dotfiles.

as you can see, it looks rather trivial, thanks to reformatter.el, which is a great library. i have positive experiences using it: i recently published a json and jsonlines reformatter: jq-format.el, which, together with my related flycheck json-jq checker (included in flycheck), makes working with json and jsonlines files a breeze.

let's get back to python. what i want is: black integration for python editing in emacs, with buffer reformatting, partial formatting, an on save hook, and minimal configuration. in practice, i want an emacs package that:

  • is based on reformatter.el
  • uses black for normal operations
  • optionally uses black-macchiato for partial reformatting
  • is available on melpa
  • is actively maintained
  • has documentation (could be a README) ;)

i really want to see this happen. compared to the current blacken.el, the above will be simpler (by reusing third party libraries and tools), and it would also solve things like #11 properly.

now, i can see this going forward in a couple of ways:

  • port this package over to reformatter.el and add the missing features. this will likely break backwards compatibility, e.g. different command names, configuration, maybe even the package name.

  • a ‘competing’ package doing exactly what i described above, and publish it on melpa. actually i considered doing this already, then stopped myself… and now i am writing this message instead.

i would be interested in your thoughts. i understand you may not be interested in my ideas/proposal, or you are too busy, and that is totally fine… which leads me to the following:

you could just say ‘go ahead and take over!’ and let me reimplement another emacs-python-black package, since i am actually willing to do all the work to make this happen. once i'm done, you could consider deprecating this package (or just leave it)... and you could sit back, relax, and use my package instead. :)

for reference, i successfully did a similar thing with my relatively widely used emacs-direnv project, which obsoleted multiple half-working earlier attempts to solve the same problem, projectile-direnv and direnv-el, in a friendly, non-competitive way; see here and here.

hope to hear from you!

This was referenced May 3, 2019
@proofit404
Copy link
Contributor

HI, thanks for open this discussion.

I was early black adopter and blacken package was born as a quick R&D project.

It works for me, so I don't make a lot of attention to it.

To be honest I have no experience with reformatter. But I open to the idea to use it for blacken reborn.

I prefer not to break public API, package or options names.

I think we can collaborate on the future version of black. There is no reason to increase the entropy of the universe I guess :)

Let start with initial PR we can discus.

If everything goes fine, I'll grand you collaborator access to the repo.

Have a good day,
Best regards, Artem.

@wbolster
Copy link
Author

wbolster commented May 5, 2019

hey, thanks for your reply.

just wondering, why does blacken.el expose configuration at all apart from the executable name? having python config in emacs which can be different per project seems ‘wrong’ to me. using a configuration file for black seems the right approach? see https://black.readthedocs.io/en/stable/pyproject_toml.html

my mini black-macchiato.el (from my dotfiles) exclusively calls black-macchiato, never black. while this makes partial reformatting work perfectly, depending on yet another tool is not ideal. ideally it shouldn't be used except for partial reformatting. i'll see whether i can convince reformatter.el to do that. :)

@wbolster
Copy link
Author

wbolster commented May 5, 2019

oh btw, for me personally the ability to reformat partial files (even though that is not ideal) is what made me look into this in the first place.

on top of that i like reformatter since i use a helper to make it work nicely with evil-mode, and some other helpers that make configuration a breeze. this means using reformatters in emacs is very consistent for different major modes (and i can use the same key bindings easily, and so on).

@wbolster
Copy link
Author

wbolster commented May 5, 2019

ok, i went ahead and played around a little.

the result is a working reformatter.el based black/black-macchiato mash-up (using an ugly stack trace inspection hack update: not needed after all). i published it here:

https://github.com/wbolster/emacs-python-black

except for minimal configuration, it is a superset of blacken.el, with support for partial file reformatting.

i figured it would be easiest to have a separate package for now. in case you are wondering why it looks complete and polished, that's because i just copy/pasted my jq-format library and did a find/replace as a starting point, which was easier than using blacken.el as the starting point.

i'm interested in hearing your thoughts!

@dakra
Copy link
Member

dakra commented May 5, 2019

Looking at your black-macchiato.py file it seems the core functionality is maybe 20 lines of python code and shouldn't be to hard to implement in emacs-lisp. I always dislike having to install yet another external tool especially if it's only a very small and could be done in elisp.

Otherwise this seems useful. Thanks for working on this.

@wbolster
Copy link
Author

wbolster commented May 6, 2019

the black project does not endorse and will never include partial formatting, and i wrote black-macchiato as a stop-gap measure. see the readme for links to relevant tickets and discussions.

implementing my hack again in emacs lisp feels wrong on many levels:

  • the functionality should not be used at all preferably
  • the logic has nothing to do with emacs so making an emacs-specific implementation seems a waste, especially since
  • an implementation already exists as a reusable, minimal tool

on top of that, reformatter.el only deals with external tools, and there is no facility to do pre/postprocessing before/after handing the contents to an external tool, which is why i (optionally!) call out to either black or black-macchiato in my implementation.

@dakra
Copy link
Member

dakra commented May 6, 2019

the functionality should not be used at all preferably

But that has nothing to do if it is included in this emacs package or as a separate package.

the logic has nothing to do with emacs so making an emacs-specific implementation seems a waste,

I don't really get this point.

especially since an implementation already exists as a reusable, minimal tool

I think this and the last point are one, and what you're saying is that you don't want to reinvent what you already have in elisp. Which is fair enough, but you could put those few lines of code in a string and call them with pythonic, that way they would be included in this package and someone doesn't need to install anything extra.

on top of that, reformatter.el only deals with external tools, and there is no facility to do pre/postprocessing before/after handing the contents to an external tool

Hmm, I guess you would need a custom function then, call your elisp part which creates the temp file and then run black on it like you do in your black-macciato script.

Overall I don't mind too much if it's external or internal, just wanted to mention that I (and probably many others) simply wont use this functionality if I first have to do yet another 'pip install..' in my system.

@wbolster
Copy link
Author

fwiw, python-black.el has more features than this package with less code, and (optionally!) depends on black-macchiato to do partial formatting. this tools is not emacs-specific which i consider a good thing since partial formatting has nothing to do with emacs, and everything to do with python (seems @dakra has other opinions here, which is fine).

i use it for a while, but never published on melpa, but multiple people are asking for that, so here goes:

will close this issue since i consider my problem solved now (albeit by having a ‘competitor’ package).

@dakra
Copy link
Member

dakra commented Aug 13, 2019

fwiw, python-black.el has more features than this package with less code,

I agree that this package should simply use the reformatter
library. I think it simply wasn't around when @proofit404 started.

and (optionally!) depends on black-macchiato to do
partial formatting. this tools is not emacs-specific which i
consider a good thing since partial formatting has nothing to do
with emacs, and everything to do with python (seems @dakra has
other opinions here, which is fine).

I was simply saying that you specifically wrote a python script
and published it on pypi and now is an extra dependency for your
Emacs package which also could have been written in very few lines
of Emacs lisp.

@wbolster
Copy link
Author

and i, as the author of both, argue it is not that n trivial. while black-macchiato is not extremely complex, it is tricky enough (it evolved in the mean time to handle more corner cases) that having an emacs port of this inherently python-specific logic to emacs lisp is just unnecessary duplication of already completed efforts. on top of that, it is optional and the main mode of operation of python-black.el, unlike the private package i used before making it a standalone emacs package, does not depend on anything but black itself.

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

3 participants