introduction of less intrusive eglot--user-error

[Inc0n]

This is to address the discussion made in #1053.

Comments

There are two places where eglot--user-error is prefered over eglot--error, which could be of annoyance when debug-on-error is set to non-nil value:

• Didn't match any of the commands
• No code action

A relevant unhandle nil case in eglot-code-actions (eglot--dcase) when invoked as mouse buffer menu

Additionally, the "didn't match any of the commands" comes from eglot--dcase which this specific one comes from eglot-code-actions is erroring when none of the code action is selected in the mouse buffer menu, which results in nil and is not part of the case.

So in that specific senario, it may be best to handle the nil case, however, the solution to that problem would be outside the scope of this commit. But if a solution has been found, I think it would be better to use eglot--error...

[joaotavora]

I agree with the need for user-error in that one "code actions" spot. But the other one is wrong in the eglot--dbind util. If you are bumping into errors there, something else is wrong.

By the way, having debug-on-error turned on means you are interested in debugging Elisp program errors with a backtrace. So you shouldn't be surprised when one shows up. I you aren't interested in these backtraces, don't turn on debug-on-error. I only do when I put one my Elisp developer hat, I don't have it on when I have the Emacs user hat on.

[joaotavora]

You should explain clearly the steps that lead to the eglot--dbind failure. I don't understand them. Explain them in Eglot user's terms.

[Inc0n]

Sorry for not explaining them clearly. I would like to invite you to discuss this further in its own separate issue thread.

I will try to formulate them here again, properly like you said.

[Inc0n]

Steps:

• At any point, where there are code actions available.
• Invoke the diagnosis with mouse-1.
• Then cancel it by clicking away the poped up menu (i.e. not selecting any of the code actions)
• This would result in the following error (with backtrace enabled, some deeper down frames are ommited):

Debugger entered--Lisp error: (error "[eglot] nil didn't match any of (((Command) comman...")
  signal(error ("[eglot] nil didn't match any of (((Command) comman..."))
  error("[eglot] %s" "nil didn't match any of (((Command) command argume...")
  eglot--error("%S didn't match any of %S" nil (((Command) command arguments) ((CodeAction) edit command)))
  (cond ((condition-case nil (progn (eglot--check-object 'Command obj-once t (memq 'disallow-non-standard-keys eglot-strict-mode) t)) (error nil)) (let* ((--cl-rest-- obj-once) (command (car (cdr (plist-member --cl-rest-- ...)))) (arguments (car (cdr (plist-member --cl-rest-- ...))))) (eglot-execute-command server (intern command) arguments))) ((condition-case nil (progn (eglot--check-object 'CodeAction obj-once t (memq 'disallow-non-standard-keys eglot-strict-mode) t)) (error nil)) (let* ((--cl-rest-- obj-once) (edit (car (cdr (plist-member --cl-rest-- ...)))) (command (car (cdr (plist-member --cl-rest-- ...))))) (progn (if edit (progn (eglot--apply-workspace-edit edit))) (if command (progn (let (...) (let* ... ...))))))) (t (eglot--error "%S didn't match any of %S" obj-once '(((Command) command arguments) ((CodeAction) edit command)))))
  (let ((obj-once action)) (cond ((condition-case nil (progn (eglot--check-object 'Command obj-once t (memq 'disallow-non-standard-keys eglot-strict-mode) t)) (error nil)) (let* ((--cl-rest-- obj-once) (command (car (cdr ...))) (arguments (car (cdr ...)))) (eglot-execute-command server (intern command) arguments))) ((condition-case nil (progn (eglot--check-object 'CodeAction obj-once t (memq 'disallow-non-standard-keys eglot-strict-mode) t)) (error nil)) (let* ((--cl-rest-- obj-once) (edit (car (cdr ...))) (command (car (cdr ...)))) (progn (if edit (progn (eglot--apply-workspace-edit edit))) (if command (progn (let ... ...)))))) (t (eglot--error "%S didn't match any of %S" obj-once '(((Command) command arguments) ((CodeAction) edit command))))))
  (let* ((server (eglot--current-server-or-lose)) (actions (jsonrpc-request server :textDocument/codeAction (list :textDocument (eglot--TextDocumentIdentifier) :range (list :start (eglot--pos-to-lsp-position beg) :end (eglot--pos-to-lsp-position end)) :context (cons ':diagnostics (cons (apply ... ...) (if action-kind ...)))) :deferred t)) (menu-items (or (let* ((--cl-vec-- actions) (--cl-idx-- -1) (action nil) (--cl-var-- nil)) (while (and (setq --cl-idx-- ...) (< --cl-idx-- ...)) (setq action (aref --cl-vec-- --cl-idx--)) (if (or ... ...) (progn ...))) (nreverse --cl-var--)) (apply #'eglot--user-error (if action-kind (list "No \"%s\" code actions here" action-kind) '("No code actions here"))))) (preferred-action (cl-find-if #'(lambda (menu-item) (plist-get (cdr menu-item) :isPreferred)) menu-items)) (default-action (car (or preferred-action (car menu-items)))) (action (if (and action-kind (null (car (cdr menu-items)))) (cdr (car menu-items)) (if (listp last-nonmenu-event) (x-popup-menu last-nonmenu-event (list "Eglot code actions:" (cons "dummy" menu-items))) (cdr (assoc (completing-read ... menu-items nil t nil nil default-action) menu-items)))))) (let ((obj-once action)) (cond ((condition-case nil (progn (eglot--check-object 'Command obj-once t (memq ... eglot-strict-mode) t)) (error nil)) (let* ((--cl-rest-- obj-once) (command (car ...)) (arguments (car ...))) (eglot-execute-command server (intern command) arguments))) ((condition-case nil (progn (eglot--check-object 'CodeAction obj-once t (memq ... eglot-strict-mode) t)) (error nil)) (let* ((--cl-rest-- obj-once) (edit (car ...)) (command (car ...))) (progn (if edit (progn ...)) (if command (progn ...))))) (t (eglot--error "%S didn't match any of %S" obj-once '((... command arguments) (... edit command)))))))
  eglot-code-actions(530 534 nil)
  funcall-interactively(eglot-code-actions 530 534 nil)
  call-interactively(eglot-code-actions)

I would consider this as a bug, since invoking the code actions via eglot-code-action lands users into a completing-read prompt, which can be exited out using C-g. Therefore, it would not land into a nil case like using mouse-1 would do.

[Inc0n]

After putting on the developer hat on, the culprit should be x-popup-menu in let bind action from the function eglot-code-actions. Using t instead of last-nonmenu-event could allows for a quit to fix this, this behaviour is explained as per the x-popup-menu doc:

If the user gets rid of the menu without making a valid choice, for
instance by clicking the mouse away from a valid choice or by typing
keyboard input, then this normally results in a quit and
‘x-popup-menu’ does not return. But if POSITION is a mouse button
event (indicating that the user invoked the menu with the mouse) then
no quit occurs and ‘x-popup-menu’ returns nil.

[Inc0n]

Sorry, for clustering this PR, I was uncertain if the nil case error may be reproducible, which I am more confident it is. However, I did already reported here, so hopefully this would turn out alright.

[Inc0n]

Doc string removed.

[joaotavora]

I would consider this as a bug, since invoking the code actions via eglot-code-action lands users into a completing-read prompt, which can be exited out using C-g. Therefore, it would not land into a nil case like using mouse-1 would do.

I agree and I have reproduced this. I see you have created a new PR. Let's continue this bug there.

[aecay]

Hi there. Just checking on this PR, specifically the "No code actions here" part. I understand that this part of the change is uncontroversial (unlike the "%s didn't match" change, where there's some issue that I haven't fully understood).

I generally run with debug-on-error set to t, and the "No code actions" errors are a bit disruptive. I'm happy to split the relevant parts of this into a new PR if that will be helpful 🙂