apheleia icon indicating copy to clipboard operation
apheleia copied to clipboard

Add emacs-lisp formatting

Open elken opened this issue 2 years ago • 10 comments

Created as a draft because it doesn't quite work; I've tried a couple of approaches but they either don't update correctly or require a second write.

I'm sure I'm implementing it incorrectly, so this is more an RFC than an actual changeset :)

elken avatar Jun 03 '22 18:06 elken

There is an example to re-indent elisp in https://github.com/radian-software/apheleia/pull/63

dakra avatar Jun 03 '22 20:06 dakra

There is an example to re-indent elisp in #63

That's where I started, doesn't quite work :)

elken avatar Jun 03 '22 20:06 elken

What are the issues you're facing with your current implementation? Maybe I can help advise.

raxod502 avatar Jun 04 '22 00:06 raxod502

What are the issues you're facing with your current implementation? Maybe I can help advise.

You're right that I don't want to use save-buffer, but if I don't then the buffer is left changed. If I try and use the method in #63 the callback throws a wrong number of arguments error

EDIT: Turns out a good night's sleep is important, I've fixed it now :) Trying to test it though gives a wrong type argument so there will probably be a change to apheleia-ft too

elken avatar Jun 04 '22 04:06 elken

Pushed changes, this now works in emacs but the test fails for the slightest diff

diff --git a/tmp/apheleia-ft-file-EDn7yr.expected b/tmp/apheleia-ft-file-Uvnxkx.actual
index 7aeecb4..c0d102c 100644
--- a/tmp/apheleia-ft-file-EDn7yr.expected
+++ b/tmp/apheleia-ft-file-Uvnxkx.actual
@@ -1,4 +1,4 @@
 (if (and (< 3 5)
-         (= 1 1))
+        (= 1 1))
     (message "true")
   (message "false"))

Even if I manually adjust out.el, there's an issue with tabs vs spaces. Could be related to doom emacs' custom +emacs-lisp-indentation-function, maybe it would be worth hoisting it out of doom here so we can use it?

elken avatar Jun 04 '22 06:06 elken

there's an issue with tabs vs spaces

Yeah, that's just because indent-tabs-mode foolishly defaults to non-nil. We can disable it in file-local variables, no need to pull in anything from Doom. (That function is just for fixing up plist indentation, which is a definite improvement but just not the issue at hand here.) I've pushed a commit fixing the issue.

raxod502 avatar Jun 12 '22 21:06 raxod502

Related to the comment, this makes the indentation very wacky

image

Thoughts?

EDIT: This was resolved by setting the mode, looks like it works now! Couple more tweaks and this should be good to review

elken avatar Jun 14 '22 20:06 elken

I'll compare my local branch with the commits you added in because it seems like some things have come in by mistake

elken avatar Jun 21 '22 03:06 elken

OK scratch that, I was tired when I did that.

Apart from the merge conflict I'm about to resume, are you happy with this?

elken avatar Jun 23 '22 21:06 elken

@raxod502 what's left to do here? the lint failure doesn't align with how the function lints elisp.

Think we should adjust the lint step to utilize what we define here so it dogfoods itself. If desired, I can add that to this PR

elken avatar Jul 17 '22 20:07 elken

I feel bad for letting this sit so long, let me see if I can fix up any remaining CI failures and merge this in.

raxod502 avatar Sep 03 '22 18:09 raxod502