search

lassik / emacs-format-all-the-code

Auto-format source code in many languages with one command
https://melpa.org/#/format-all
MIT License
607 stars 106 forks source link

node-cljfmt is deprecated, can we switch to cljfmt? #260

Open sunng87 opened 19 hours ago

sunng87 commented 19 hours ago

The node-cljfmt project is deprecated https://github.com/snoe/node-cljfmt

The original cljfmt project is still well maintained https://github.com/weavejester/cljfmt. Can we make a switch? Note that the default command is different between two projects so it may cause confusion when users are using original cljfmt.

If this sounds like a good idea, I can make a PR for this switch.

lassik commented 9 hours ago

Thank you for bringing this for our attention. The situation seems complicated.

The node-cljfmt readme says that it is deprecated, but not that it has stopped working. I assume it still works.

All three of cljfmt, node-cljfmt, and cljfmt-graalvm seem to install the shell command cljfmt. This means format-all should offer one formatter, cljfmt, that can use any of the three. This is what we currently do:

(define-format-all-formatter cljfmt
  (:executable "cljfmt")
  (:install "npm install --global node-cljfmt")
  (:languages "Clojure")
  (:features)
  (:format (format-all--buffer-easy executable)))

However, since node-cljfmt is deprecated, suggesting that people install it via npm install --global node-cljfmt is not a good idea. I don't know what to suggest. The cljfmt readme gives the install command /bin/bash -c "$(curl -fsSL ...). Piping curl to bash is widely considered bad practice. It's probably not time to recommend cljfmt-graalvm yet. GraalVM is not nearly as popular as classic JVM. Perhaps it's best not to suggest any install command. We can link to the readmes of all three projects.

The cljfmt readme says it can also be installed via clj -Ttools install. In that case it is run as clj -Tcljfmt. If people use this with format-all, then we should add another formatter (define-format-all-formatter clj-cljfmt ...).

I'll make a PR about this and let you review it.

lassik commented 8 hours ago

The original cljfmt suggests lein cljfmt as a third option. We'd have to add (define-format-all-formatter lein-cljfmt ...).

lassik commented 8 hours ago

Looks like there is more than one cljfmt-graalvm project. I'm not sure whether they support formatting stdin->stdout. I'll leave out cljfmt-graalvm until this is cleared up.