Docs Request: Usage with Eglot

[denisw]

Thank you for creating flymake-collection! I use it together with Eglot to fill the gaps left by the LSP server (e.g., running ESLint along typescript-language-server). However, doing so is a little tricky as Eglot replaces all registered Flymake backends when activated rather than replacing them.

It would be great if the README had a section on how to work around this. For reference, here is what I do, though I'm not sure it's the best approach:

(defun my/eglot-managed-mode-hook ()
  ;; Add the configured flymake-collection checkers on top of
  ;; eglot-flymake-backend.
  (let* ((entry (assoc major-mode flymake-collection-config))
         (checkers (cdr entry)))
    (dolist (c checkers)
      (push c flymake-diagnostic-functions))))

(add-hook 'eglot-managed-mode 'my/eglot/managed-mode-hook)

emacs-managed-mode-hook is described in the Eglot manual.

[mohkale]

@denisw

I don't think it makes sense to maintain workarounds for behavior set by another package in this one, although I don't mind adding documentation for it to the README if someone creates a PR for it.

[meain]

I think all you need to do is to ask eglot to stay away from completely taking over flymake and just add it as another item.

(add-to-list 'eglot-stay-out-of 'flymake)
(add-hook 'flymake-diagnostic-functions 'eglot-flymake-backend)

[peterhoeg]

The original issue is still valid though and while it stricly speaking may be off-topic here, I think at least figuring out the official way to do things would be helpful.

This is basically the different scenarios:

1. the language server is good so you don't want any other checks run through flymake. This is the default scenario with eglot that assumes that to be the case.
2. you want to use both elgot and other flymake checks. Here you need 'eglot-stay-out-of and then you can mess around with flymake-diagnostic-functions manually either as suggested here or through make-local-variable.
3. there is no LS for a given major mode and as a consequence it's all about flymake
4. but then we have another case - sometimes you want to use eglot plus zero or more flymake checks or some flymake checks

The example for item 4 is crystal where the language server is still in early stages, so using it alone is not enough, but depending on the type of file you're working with you either want eglot (because it's part of a proper crystal project) or you want a standalone flymake check using the crystal compiler because it's a standalone file.

And I must admit I'm having a bit of a hard time modelling exactly how to deal with the scenario outlined in item 4.

What makes it even worse is of course that you could argue this should be the responsibility of flymake or eglot to know when to enable certain checks and not really flymake-collection.

On a final note - thanks for flymake-collection, it's so easy to add new checks, thank you!!

[mohkale]

@peterhoeg

To be clear, what you're describing is a very specialized workflow configuration and I don't think that belongs in flymake-collection. I'm generally of the mindset that language-server > flymake-checker and actually prefer eglot overriding flymake and flymake-collection. In situations where this is not desired I think it's fine to let users come up with their own way to setup flymake-diagnostic-functions to achieve what they prefer. Trying to cater to every possible combination of language-server and flymake checker is challenging and I don't think will ever be robust enough for everyone. I'm willing to accept patches to improve compatibility here within reason but still think this is a very specific desire that doesn't belong in a generic package like this one.

[peterhoeg]

Right, so I don’t think that I’m the only one with this issue - just as well as some will have multiple flymake checkers running, some would want them in addition to lsp, but I fully understand where you are coming from. You can also rightfully argue that helpers to assist with this belong in flymake proper or eglot. Or maybe completely outside so that it becomes either the responsibility of the individual or whichever emacs distribution (in my case doom) people are using. Any plans to upstream any of this into flymake?

[mohkale]

Any plans to upstream any of this into flymake?

There was some discussion of it while considering adding this package to ELPA/Non-GNU-Elpa, the main blocker at this point is just cleaning up the code base and me going through the process for submission (it's on my TODO list, just not a high priority). See also #2.

For reference this package isn't the only one providing a flymake helper. It's actually very heavily adapted from flymake-quickdef.

[peterhoeg]

I had a look at quickdef first but since you have already done the work and created a number of definitions of which I am now using 3, I thought it would make more sense to stick to yours. Very nice to hear that the end-goal is possible upstreaming! Thanks again for your hard work. Emacs 29 with flymake is proving to be very smooth due to amongst other things flymake-collection!

[sethidden]

For the people that are looking for a code snippet to work around this, it's at the bottom of this comment.

It's true that whenever you enter a mode that's managed by Eglot, flymake-diagnostic-functions gets overriden to simply '(eglot-flymake-backend). The code that makes this happen can be seen here:

(eglot--setq-saving flymake-diagnostic-functions '(eglot-flymake-backend))

To prevent Eglot from doing this, you can use eglot-stay-out-of, but you need to use the name of the variable being set above, not just flymake (if you use just 'flymake' - looking at eglot code - this just prevents starting flymake whenever eglot starts):

(add-to-list 'eglot-stay-out-of 'flymake-diagnostic-functions))

Now, if we come back to flymake-collection and how the readme file tells you to start it - calling (flymake-collection-hook-setup) adds flymake-collection-hook-set-backends to after-change-major-mode-hook. After the above eglot-stay-out-of snippet the flymake backends should be set according to how flymake-collection wants it (instead of how eglot wants it), and diagnostics should be showing up in buffers.

The only problem now is that eglot-flymake-backend is not added to flymake backends, and diagnostics from language servers are not rendered. To fix this, you can manually re-add the eglot backend (this time gracefully with add-to-list rather than angrily with setq like eglot does it) the eglot backend:

(add-hook eglot-managed-mode-hook (lambda () (add-to-list 'flymake-diagnostic-functions 'eglot-flymake-backend)))

---

The snippet promised at the top is below. I'm not super knowledgeable about the :init and :config keywords so the placement isn't very smart

 ;; I'm using elpaca
  (use-package flymake-collection
    :ensure t
    :init
    (flymake-collection-hook-setup) ;; idk I'm not using (after-init-hook) like the README tells me to, because then `flymake-collection-hook-setup` doesn't fire
    :hook
    (eglot-managed-mode . (lambda () (add-to-list 'flymake-diagnostic-functions 'eglot-flymake-backend)))
    :config
    (add-to-list 'eglot-stay-out-of 'flymake-diagnostic-functions))