:editor format refactor

I’m on it as we speak :stuck_out_tongue: Probably won’t be far off what I said, some kind of funcall on emacs-lisp-indentation-function applied to the whole buffer.

https://github.com/lassik/emacs-format-all-the-code/blob/828280eaf3b46112e17746a7d03235570a633425/format-all.el#L811-L821

apparently fomrat-all does exactly this so it should basically be equivalent

1 Like

although for some reason just evaling (indent-region (point-min) (point-max)) doesn’t seem to do anything

@@ -927,6 +927,7 @@ being run, for diagnostic purposes."
     (gofmt . ("gofmt"))
     (google-java-format . ("google-java-format" "-"))
     (isort . ("isort" "-"))
+    (el-indent-buffer . apheleia-indent-emacs-lisp-buffer)
     (ktlint . ("ktlint" "--stdin" "-F"))
     (latexindent . ("latexindent" "--logfile=/dev/null"))
     (mix-format . ("mix" "format" "-"))
@@ -998,6 +999,12 @@ rather than using this system."
              (const :tag "Name of temporary file used for output" output)))
            (function :tag "Formatter function"))))
 
+(cl-defun apheleia-indent-emacs-lisp-buffer (&key buffer formatter callback remote stdin &allow-other-keys)
+  (with-current-buffer buffer
+    (indent-region (point-min) (point-max))
+    (save-buffer)))
+
 (defun apheleia--run-formatters
     (formatters buffer remote callback &optional stdin)
   "Run one or more code formatters on the current buffer.
@@ -1053,6 +1060,7 @@ function: %s" command)))
     (css-mode . prettier)
     (dart-mode . dart-format)
     (elixir-mode . mix-format)
+    (emacs-lisp-mode . el-indent-buffer)
     (fish-mode . fish-indent)
     (go-mode . gofmt)
     (haskell-mode . brittany)

Seems to work, though this triggers two writes :confused: Any thoughts?

For me it only write once, but for some reason committing the changes and reloading emacs didn’t get the changes to propagate, i had to manually re-evaluate the buffer.

Can you upload your local elisp format stuff to your fork so i’m sure we’re on the same package?

Also, consider sending a patch file next time :wink:

It definitely saves twice.

reloading emacs didn’t get the changes to propagate, i had to manually re-evaluate the buffer

Yeah I get this too, probably autoload behaviour.

Can you upload your local elisp format stuff to your fork so i’m sure we’re on the same package?

Yeah I will

oh actually i rebased on the latest doom commit, that probably won’t effect anything, but can you do that since you should anyway?

Pushed to feat/add-elisp

It still only writes once. Did you rebase the PR on the latest master and still get double writes?

Latest doom master? I rebase it regularly yeah

@MonadGauge I have it working now without using save-buffer so I only get the one intended write and it doesn’t stall emacs.

See feat: add emacs-lisp formatting by elken · Pull Request #102 · radian-software/apheleia · GitHub for the status

In apheleia-indent-emacs-lisp-buffer, I think that we should add

(funcall callback)

Otherwise, the hidden scratch buffer won’t be killed.

I know, that’s what I’ve done :slight_smile: Haven’t pushed changes yet because the test suite doesn’t account for formatters as functions yet

1 Like

Currently if there’s an error formatting, you only get a small notice about the error buffer in the echo area (that sometimes gets overwritten). imo the previous behavior of popping up the error buffer was better, although it should probably be in a popup (which the previous one wasn’t for some reason)

One to query upstream (though we could implement it easily enough if raxod doesn’t agree)

Okay, this has been dead way longer than I would have liked…

Various things have come up, work, life etc, which for the most part have been resolved now.

In any case; I have pushed an emacs-lisp formatter to my work that if anyone is interested should attempt to use and provide examples of it failing.

I have also asked Henrik to review and potentially have it dogfood doom, so this final PR might end up being huge…

In terms of the other modules, I would ask that if anyone uses one of the languages above on a daily basis and can provide the “best” linter along with sane default arguments (if needed) that would save me having to make guesses and potentially choose a bad formatter :smiley:

Another minor-ish update, I’ve amended the post to clean it up a bit and added a table of contents to make navigating it a bit easier.

It’s also in sync with master as of this commit and refactored to handle the new modulep! macro.

Progress wise the first pass is about half way through having basic module integration, though I have found some missing modules in terms of tooling.

If you’re a user of any module that is crossed out, please do let me know of a working linter/formatter that:

  • Accepts input from stdin
  • Sends output to stdout
  • Can be installed/used as a script/executable or at worst through arguments to a tool

First intended pass is done!

Every module listed above has been actioned in some way now, either included and tested in an isolated case, already included upstream, or crossed out due to lacking tooling support/requiring a domain expert.

Next pass will be spending a bit more time on each module (as well as checking if I’ve missed any modules that make sense to add a formatter for) and then writing install documentation for each module (as well as workarounds for any known issues/weirdness)

As it stands, this is ready for testing by whomever wants to; I’m not an expert on every language so I would have no idea about best practices in most of the modules.

And as above, if you’re a user of any module that is crossed out, please do let me know of a working linter/formatter that:

  • Accepts input from stdin
  • Sends output to stdout
  • Can be installed/used as a script/executable or at worst through arguments to a tool

TL;DR large bulk of the work is done, please test if you can

Upstream does support nix, but it’s the nixfmt formatter which isn’t used very much. The community has not yet agreed on a canonical formatter but AFAIK, nixpkgs-fmt is used far more frequently, so maybe we should add that in here for now?

Thanks for bringing that to my attention!

This is almost certainly a good candidate for an upstream change, so I’ll test it tonight and create an upstream PR (and also make the change in doom so it’s not blocked)