bug#43716: 28.0.50; Error with emacsclient <file>:

[Tassilo Horn]

Lars Ingebrigtsen <larsi@gnus.org> writes:

Hi Lars,

>> I found it. :-)
>>
>> lib-src/emacsclient.c
>> 2095               fprintf (stderr, "*ERROR*: %s", str);
>
> Ah!  So the Emacs server reports it back to the client as an error.
> No wonder there's no backtrace here.  :-/
>
> What about the following patch -- if that's the correct condition-case
> that's catching all this, then it should give you a backtrace in the
> server Emacs after reloading server.el.

That doesn't work but pointed into the right direction.  I've edebugged
`server-process-filter' now, and the Wrong number of arguments message
appears when the (server-execute ...)  form is evaluated.

--8<---------------cut here---------------start------------->8---
          (process-put
           proc 'continuation
           (lambda ()
             (with-current-buffer (get-buffer-create server-buffer)
               ;; Use the same cwd as the emacsclient, if possible, so
               ;; relative file names work correctly, even in `eval'.
               (let ((default-directory
                       (if (and dir (file-directory-p dir))
                           dir default-directory)))
                 (server-execute proc files nowait commands
                                 dontkill frame tty-name)))))
--8<---------------cut here---------------end--------------->8---

So I edebugged server-execute, and the error happens when calling
`server-switch-buffer' in:

--8<---------------cut here---------------start------------->8---
          (cond
           ((or isearch-mode (minibufferp))
            nil)
           ((and frame (null buffers))
            (run-hooks 'server-after-make-frame-hook)
            (message "%s" (substitute-command-keys
                           "When done with this frame, type \\[delete-frame]")))
           ((not (null buffers))
            (run-hooks 'server-after-make-frame-hook)
            (server-switch-buffer
             (car buffers) nil (cdr (car files))
             ;; When triggered from "emacsclient -c", we popped up a
             ;; new frame.  Ensure that we switch to the requested
             ;; buffer in that frame, and not in some other frame
             ;; where it may be displayed.
             (plist-get (process-plist proc) 'frame))
--8<---------------cut here---------------end--------------->8---

And that is *advised* by with-editor.el (used by Magit):

--8<---------------cut here---------------start------------->8---
server-switch-buffer is a Lisp closure in ‘server.el’.

(server-switch-buffer &optional NEXT-BUFFER KILLED-ONE FILEPOS THIS-FRAME-ONLY)

This function has :around advice:
‘server-switch-buffer--with-editor-server-window-alist’.
--8<---------------cut here---------------end--------------->8---

>From with-editor.el:

--8<---------------cut here---------------start------------->8---
(defun server-switch-buffer--with-editor-server-window-alist
    (fn &optional next-buffer killed-one filepos)
  "Honor `with-editor-server-window-alist' (which see)."
  (let ((server-window (with-current-buffer
                           (or next-buffer (current-buffer))
                         (when with-editor-mode
                           (setq with-editor-previous-winconf
                                 (current-window-configuration)))
                         (with-editor-server-window))))
    (funcall fn next-buffer killed-one filepos)))

(advice-add 'server-switch-buffer :around
            'server-switch-buffer--with-editor-server-window-alist)
--8<---------------cut here---------------end--------------->8---

I've added Jonas to the Cc.  What breaks the advice is the new
`this-frame-only' parameter of `server-switch-buffer' Lars introduced on
master with 7f2c35d0287.

I guess with-editor.el needs to discriminate by emacs version or use a
&rest argument in the advice and checking the individual 3 (emacs < 28)
or 4 (emacs 28) arguments separately.

Bye,
Tassilo