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

[REQUEST] Only use format-all-the-code if the language server doesn't support formatting #1652

Closed
rassie opened this issue Aug 8, 2019 · 3 comments
Labels
is:feature Adds or requests new features, or extends existing ones module:editor/format Pertains to Doom's :editor format module module:tools/lsp Pertains to Doom's :tools lsp module status:resolved Issue was addressed internally

Comments

@rassie
Copy link

rassie commented Aug 8, 2019

Describe the feature

The basic idea has been introduced to format-all-the-code in lassik/emacs-format-all-the-code#47. The assumption is that a language server probably has a better idea how to format the code than any default set in format-all-the-code. The proposal, quoted from there, goes like this:

  1. If lsp-mode is not enabled for the current buffer, fallback to the current behaviour.
  2. Otherwise, check for textDocument/formatting capability of the current language server (or maybe servers, since that's apparently a supported workflow). If it's not available, fallback to current behaviour.
  3. Otherwise, use textDocument/formatting (probably via lsp-format-buffer) instead of the predefined tool.

It's not clear whether it's a better idea to make format-all-the-things LSP-aware or extend doom-emacs' auto-formatting wrapper. Either way I'd be happy to have the feature.

System information

- OS: gnu/linux (x86_64-pc-linux-gnu)
- Shell: /bin/bash
- Emacs: 26.2 (Apr 12, 2019)
- Doom: 2.0.9 (HEAD -> develop, _upgrade/develop f754d4ff 2019-07-23 18:23:17 +0200)
- Graphic display: nil (daemon: nil)
- System features: XPM JPEG TIFF GIF PNG RSVG IMAGEMAGICK SOUND GPM DBUS GSETTINGS GLIB NOTIFY LIBSELINUX GNUTLS LIBXML2 FREETYPE M17N_FLT LIBOTF XFT ZLIB TOOLKIT_SCROLL_BARS GTK3 X11 XDBE XIM MODULES THREADS XWIDGETS LIBSYSTEMD LCMS2
- Details:
  ```elisp
  env bootstrapper: nil
  elc count: 0
  uname -a:  Linux 5.0.0-25-generic #26~18.04.1-Ubuntu SMP Thu Aug 1 13:51:02 UTC 2019 x86_64
  modules:   (:completion company (ivy +fuzzy +childframe) :ui doom doom-dashboard hl-todo modeline nav-flash ophints (popup +all +defaults) treemacs unicode vc-gutter vi-tilde-fringe window-select workspaces :editor fold (format +onsave) multiple-cursors rotate-text snippets :emacs dired vc :tools ansible direnv docker editorconfig eval flycheck (lookup +docsets) lsp magit make pdf rgb terraform :lang data emacs-lisp (go +lsp) (haskell +intero) (java +lsp) (javascript +lsp) (kotlin +lsp) latex markdown (org +attach +babel +capture +export +habit +present +protocol) perl php plantuml (python +lsp) qt rest ruby (sh +fish) (web +lsp) :app write :config default)
  packages:  (full-ack visual-regexp nginx-mode company-qml persistent-scratch gitlab-ci-mode gitlab-ci-mode-flycheck rpm-spec-mode kotlin-mode flycheck-kotlin puppet-mode prettier-js add-node-modules-path solaire-mode comment-dwim-2 ag lsp-python-ms glsl-mode)
  exec-path: (./node_modules/.bin ~/.npm-packages/bin/ ~/.local/bin/ /bin ~/go/bin ~/.npm-packages/bin ~/.cargo/bin/ ~/.npm-packages/bin/ ~/.local/bin/ /bin ~/go/bin /bin ~/bin ~/.local/bin /usr/local/sbin /usr/local/bin /usr/sbin /usr/bin /sbin /bin /usr/games /usr/local/games /snap/bin ~/.dotnet/tools /usr/lib/maven3/bin ~/.fzf/bin /usr/lib/x86_64-linux-gnu/emacs/26.2/x86_64-linux-gnu)
@hlissner hlissner added is:feature Adds or requests new features, or extends existing ones module:editor/format Pertains to Doom's :editor format module module:tools/lsp Pertains to Doom's :tools lsp module labels Aug 8, 2019
@hlissner
Copy link
Member

Sorry for the late response; just chiming in to let you know I am aware of this issue. The :editor format module is painfully overdue a rewrite, both to make it LSP-aware, and possibly to replace format-all-the-code with reformatter.el to facilitate a more customizable formatting backend.

I don't yet have time to work on this, however, but I haven't forgotten about it.

Also, @Kaali in #2516 brought up this LSP-unawareness in go-mode. For the time being, the workaround for this is to disable our formatter by adding go-mode to +format-on-save-enabled-modes (which is a negated list, by default, due to its first element being not), then add lsp-format-buffer to that mode's local before-save-hook. e.g.

(add-to-list '+format-on-save-enabled-modes 'go-mode t)

(add-hook! 'go-mode-hook
  (add-hook 'before-save-hook #'lsp-format-buffer nil 'local)
  (add-hook 'before-save-hook #'lsp-organize-imports nil 'local))

@rassie
Copy link
Author

rassie commented Feb 21, 2020

@hlissner Thanks for the feedback! Before you replace format-all-the-code, you might be interested in recent developments, which introduces global and buffer-local configurable formatters. Otherwise: don't mind me, I'll be happy if and when it's done, but I'm also quite happy with Doom right now even without it :)

hlissner added a commit that referenced this issue May 9, 2020
@hlissner
Copy link
Member

As of 7472cff, our format commands support LSP (if the running server supports formatting).

This can be disabled globally with

(setq +format-with-lsp nil)

Or on a per-mode basis with

(setq-hook! 'python-mode-hook +format-with-lsp nil)

In any case I'll consider this resolved. Thanks for bringing it to my attention!

@hlissner hlissner added status:resolved Issue was addressed internally and removed status:backlog labels May 25, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 31, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
is:feature Adds or requests new features, or extends existing ones module:editor/format Pertains to Doom's :editor format module module:tools/lsp Pertains to Doom's :tools lsp module status:resolved Issue was addressed internally
Projects
None yet
Development

No branches or pull requests

2 participants