bug#65277: Fwd: bug#65277: 29.1.50; emacsclient Dired: frame is closed/killed when opening another dir

[brickviking <brickviking@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 6053 bytes --]

Apologies, this didn't quite get to the right place.

---------- Forwarded message ---------
From: brickviking <brickviking@gmail.com>
Date: Fri, 18 Aug 2023 at 20:02
Subject: Re: bug#65277: 29.1.50; emacsclient Dired: frame is closed/killed
when opening another dir
To: Eli Zaretskii <eliz@gnu.org>


whoops, we've crossed paths here somewhat. My last reply was for the second
patch set you sent through, not this last one. I'm compiling again with the
last patch set you provided, and will have some results for you shortly.

Regards, brickviking


On Fri, 18 Aug 2023 at 19:42, Eli Zaretskii <eliz@gnu.org> wrote:

> > Cc: 65277@debbugs.gnu.org
> > Date: Thu, 17 Aug 2023 22:02:16 +0300
> > From: Eli Zaretskii <eliz@gnu.org>
> >
> > > Cc: 65277@debbugs.gnu.org
> > > Date: Thu, 17 Aug 2023 12:44:47 +0300
> > > From: Eli Zaretskii <eliz@gnu.org>
> > >
> > > Thanks for testing.  Could you please test the improved patch below?
> >
> > Sorry, that patch had a silly mistake.  Please use the one below
> > instead:
>
> I have now tested the last patch I sent on a GNU/Linux system with
> using "emacsclient -t", and found another problem.  The updated patch
> below should solve it, so p-lease try that when you have time.
>
> With the patch below, the result of "C-x C-v" in a client
> terminal/frame is that the frame stays around, and also emacsclient
> doesn't exit.  However, the buffer that visits the alternate file is
> not considered a "client" buffer, so "C-x #" displays an error message
> saying there are no client buffers left and does nothing else, and
> "C-x k" doesn't delete the client frame.  To delete the client frame
> and cause emacsclient to exit, you need to type either "C-x C-c" or
> "C-x 5 0".  This might be a bit surprising, but OTOH the semantics of
> the client buffer after "C-x C-v" is not well defined: should the
> buffer visiting the alternate file have the same client property and
> share the same kill-buffer-hook?  I could argue either way.
>
> Here's the patch to try.  If there are no objections or bugs reported
> against it, I will install this on master in a week or so.
>
> diff --git a/lisp/files.el b/lisp/files.el
> index 68c0a10..3466a53 100644
> --- a/lisp/files.el
> +++ b/lisp/files.el
> @@ -1998,6 +1998,8 @@ kill-buffer-hook
>  Note: Be careful with let-binding this hook considering it is
>  frequently used for cleanup.")
>
> +(defvar find-alternate-file-dont-kill-client nil
> +  "If non-nil, `server-buffer-done' should not delete the client.")
>  (defun find-alternate-file (filename &optional wildcards)
>    "Find file FILENAME, select its buffer, kill previous buffer.
>  If the current buffer now contains an empty file that you just visited
> @@ -2044,7 +2046,8 @@ find-alternate-file
>      ;; save a modified buffer visiting a file.  Rather, `kill-buffer'
>      ;; asks that itself.  Thus, there's no need to temporarily do
>      ;; `(set-buffer-modified-p nil)' before running this hook.
> -    (run-hooks 'kill-buffer-hook)
> +    (let ((find-alternate-file-dont-kill-client 'dont-kill-client))
> +      (run-hooks 'kill-buffer-hook))
>      ;; Okay, now we can end-of-life the old buffer.
>      (if (get-buffer " **lose**")
>         (kill-buffer " **lose**"))
> diff --git a/lisp/server.el b/lisp/server.el
> index ba7e02d..10f1559 100644
> --- a/lisp/server.el
> +++ b/lisp/server.el
> @@ -330,6 +330,9 @@ server-with-environment
>  (defun server-delete-client (proc &optional noframe)
>    "Delete PROC, including its buffers, terminals and frames.
>  If NOFRAME is non-nil, let the frames live.
> +If NOFRAME is the symbol \\='dont-kill-client, also don't
> +delete PROC or its terminals, just kill its buffers: this is
> +for when `find-alternate-file' calls this via `kill-buffer-hook'.
>  Updates `server-clients'."
>    (server-log (concat "server-delete-client" (if noframe " noframe"))
> proc)
>    ;; Force a new lookup of client (prevents infinite recursion).
> @@ -366,23 +369,28 @@ server-delete-client
>             (set-frame-parameter frame 'client nil)
>             (delete-frame frame))))
>
> -      (setq server-clients (delq proc server-clients))
> +      (or (eq noframe 'dont-kill-client)
> +          (setq server-clients (delq proc server-clients)))
>
>        ;; Delete the client's tty, except on Windows (both GUI and
>        ;; console), where there's only one terminal and does not make
>        ;; sense to delete it, or if we are explicitly told not.
>        (unless (or (eq system-type 'windows-nt)
> +                  ;; 'find-alternate-file' caused the last client
> +                  ;; buffer to be killed, but we will reuse the client
> +                  ;; for another buffer.
> +                  (eq noframe 'dont-kill-client)
>                    (process-get proc 'no-delete-terminal))
>         (let ((terminal (process-get proc 'terminal)))
>           ;; Only delete the terminal if it is non-nil.
>           (when (and terminal (eq (terminal-live-p terminal) t))
>             (delete-terminal terminal))))
>
> -      ;; Delete the client's process.
> -      (if (eq (process-status proc) 'open)
> -         (delete-process proc))
> -
> -      (server-log "Deleted" proc))))
> +      ;; Delete the client's process (or don't).
> +      (unless (eq noframe 'dont-kill-client)
> +        (if (eq (process-status proc) 'open)
> +           (delete-process proc))
> +        (server-log "Deleted" proc)))))
>
>  (defvar server-log-time-function #'current-time-string
>    "Function to generate timestamps for `server-buffer'.")
> @@ -1590,7 +1598,8 @@ server-buffer-done
>                 ;; frames, which might change the current buffer.  We
>                 ;; don't want that (bug#640).
>                 (save-current-buffer
> -                 (server-delete-client proc))
> +                 (server-delete-client proc
> +
> find-alternate-file-dont-kill-client))
>               (server-delete-client proc))))))
>      (when (and (bufferp buffer) (buffer-name buffer))
>        ;; We may or may not kill this buffer;
>

[-- Attachment #2: Type: text/html, Size: 7941 bytes --]