Add option to call `lsp-format-buffer` on save

[michaelpj]

It would be nice to have an option to set up buffer formatting on save. This is something that users can definitely do for themselves (e.g. https://github.com/emacs-lsp/lsp-mode/blob/8c478ce4f4ffb7cd3f9c152966d5480f113d7b7e/docs/manual-language-docs/lsp-rust-rls.md#rustfmt), but it's a common IDE option and so it would be nice to provide a simple option for it from a "batteries included" perspective.

[ericdallo]

isn't that available already via lsp-enable-indentation?

[michaelpj]

No? That seems to use lsp-format-region to indent regions. It doesn't do anything on save.

[ericdallo]

ah, my bad, yeah, you are correct.
doom-emacs has custom logic to save after formatting buffer we could check how they do, but I agree a variable for that makes sense

[yyoncho]

I think we should enforce a new rule. If ChatGPT can answer it, then we don't need an additional flag/setting.

[ericdallo]

that's crazy hahahahah

[michaelpj]

I think it's reasonable to say that people should just know enough elisp to do this themselves. Not all of our users are as good at it as ChatGPT, though ;)

[dschrempf]

I agree with @ericdallo that there should be an option for activating or deactivating "format on save" for all lsp-mode managed buffers. When searching for lsp format and enable, I get the following 10 (!) results:

lsp-html-format-enable
lsp-xml-format-enabled
lsp-yaml-format-enable
lsp-vetur-format-enable
lsp-javascript-format-enable
lsp-typescript-format-enable
lsp-enable-on-type-formatting
lsp-intelephense-format-enable
lsp-svelte-plugin-svelte-format-enable
lsp-rust-analyzer-rustfmt-rangeformatting-enable

That's a bit ridiculous. It is easy to configure on a "per-langauge-basis", but then, I think it would be better to be able to opt out than opt in.

[dschrempf]

Suggestion:

(defvar-local lsp-format-on-save t
  "Format `lsp-mode'-managed buffer before save.")
(defun lsp-format-on-save-not-apheleia ()
  "Format on save using LSP server, not `apheleia'."
  (if lsp-format-on-save
      (progn
        (add-hook 'before-save-hook #'lsp-format-buffer nil 'local)
        (setq-local apheleia-mode nil))))
(add-hook 'lsp-configure-hook #'lsp-format-on-save-not-apheleia))

EDIT: This code contains stuff with apheleia, which I also use, but which should be removed I guess.

[jimeh]

I've had lsp-format-buffer-on-save-mode and lsp-organize-imports-on-save-mode minor-modes in my config for years now without issue:

(define-minor-mode lsp-format-buffer-on-save-mode
  "Run lsp-format-buffer as a before-save-hook."
  :lighter " fmt"
  (if lsp-format-buffer-on-save-mode
      (add-hook 'before-save-hook 'lsp-format-buffer t t)
    (remove-hook 'before-save-hook 'lsp-format-buffer t)))

(define-minor-mode lsp-organize-imports-on-save-mode
  "Run lsp-organize-imports as a before-save-hook."
  :lighter " imp"
  (if lsp-organize-imports-on-save-mode
      (add-hook 'before-save-hook 'lsp-organize-imports t t)
    (remove-hook 'before-save-hook 'lsp-organize-imports t)))

Then I just call either/both methods in a mode setup hook along with (lsp-deferred) or (lsp). For exmple, here is my setup for go-mode.

[ryuheechul]

@jimeh's code works well for me!

...

(define-minor-mode lsp-format-buffer-on-save-mode
  "Run lsp-format-buffer as a before-save-hook."
  :lighter " fmt"
  (if lsp-format-buffer-on-save-mode
      (add-hook 'before-save-hook 'lsp-format-buffer t t)
    (remove-hook 'before-save-hook 'lsp-format-buffer t)))

;; ...

...

However I wasn't able to figure out how to hook this to work with every buffer just by looking at the link.

So I came up with another way to make it work

(add-hook 'lsp-after-open-hook (lambda () (lsp-format-buffer-on-save-mode t)))

(I'm not strong with elisp, feel free to suggest a better way to do this or optimize the snippet above)

---

edit: just included my config regarding this at ryuheechul/dotfiles@4c9f9f4

[ncaq]

I need this option.

(defcustom lsp-format-buffer-on-save nil
  "Format buffer on save by lsp."
  :type 'boolean
  :group 'lsp-mode
  :safe t
  :local t)

Because I want config by .dir-locals.el.

Is there any reason why the code corresponding to this option should not be implemented?

[ericdallo]

@ncaq no, PR is welcome for that!