GNU bug report logs - #48493
28.0.50; quit-window doesn't work

Previous Next

Full log

View this message in rfc822 format

From: pillule <pillule <at> riseup.net>
To: martin rudalics <rudalics <at> gmx.at>
Cc: pillule <pillule <at> riseup.net>, Sujith Manoharan <sujith.wall <at> gmail.com>, 48493 <at> debbugs.gnu.org
Subject: bug#48493: 28.0.50; quit-window doesn't work
Date: Wed, 09 Jun 2021 14:34:54 +0200

[Message part 1 (text/plain, inline)]

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: pillule <pillule <at> riseup.net>
>> Date: Tue, 08 Jun 2021 01:23:11 +0200
>> Cc: pillule <pillule <at> riseup.net>, Sujith Manoharan 
>> <sujith.wall <at> gmail.com>,
>>  48493 <at> debbugs.gnu.org
>>
>> > We have to document it in the Elisp manual.
>>
>> Here another draft with the info manual changes:
>
> Thanks.  Once again, I leave it to Martin and others to comment 
> on
> most of the essence of the patch, and provide here only few 
> minor
> nits:
>
>> * doc/lispref/windows.texi
>>   (Buffers and Windows): mention the exception of side windows 
>>   and
>> add a reference.
>>   (Buffer Display Action Alists): explicit that
>> `display-buffer-in-side-window' is dedicating to side by 
>> default.
>>   (Dedicated Windows): add the case (4) and explicit its 
>>   meaning,
>> add a reference.
>>   (Displaying Buffers in Side Windows): move the
>> switch-to-(prev|next)-buffer paragraph into a new item to 
>> emphasize
>> the special meaning of dedication for side windows.
>
> Again, this log message is not formatted according to our rules. 
> It
> should look like this:
>
> * doc/lispref/windows.texi (Buffers and Windows): Mention the
> exception of side windows and add a reference.
> (Buffer Display Action Alists): Say explicitly that
> 'display-buffer-in-side-window' is dedicating to side by 
> default.
> (Dedicated Windows): Add case (4) and explain its meaning, add
> a reference.
> (Displaying Buffers in Side Windows): Move the paragraph about
> 'switch-to-(prev|next)-buffer' into a new item to emphasize the
> special meaning of dedication for side windows.
>
> Note that I also fixed the wording a bit, and that our 
> conventions for
> quoting in log entries is 'like this'.
>
>>  The replacement buffer in each window is chosen via
>> -@code{switch-to-prev-buffer} (@pxref{Window History}).  Any 
>> dedicated
>> -window displaying @var{buffer-or-name} is deleted if possible
>> -(@pxref{Dedicated Windows}).  If such a window is the only 
>> window on its
>> -frame and there are other frames on the same terminal, the 
>> frame is
>> -deleted as well.  If the dedicated window is the only window 
>> on the only
>> -frame on its terminal, the buffer is replaced anyway.
>> +@code{switch-to-prev-buffer} (@pxref{Window History}).  With 
>> the
>> +exception of side windows, any dedicated window displaying
>    ^^^^^^^^^^^^^^^^^^^^^^^^^
> Here' it is quite important that the reader understands what are 
> "side
> windows", otherwise he/she will not understand the exception.
> However, "side window" was not yet described in the manual by 
> this
> point, and its description is not in this section.  In these 
> cases,
> always include a cross-reference to where the term is described, 
> like
> this:
>
>   With the exception of side windows (@pxref{Side Windows}), any 
>   ...
>
>>     In particular, @code{delete-windows-on} (@pxref{Deleting 
>>     Windows})
>> -handles case (2) by deleting the associated frame and case (3) 
>> by
>> -showing another buffer in that frame's only window.  The 
>> function
>> +handles case (2) by deleting the associated frame and case (3) 
>> and (4)
>                                                          ^^^^
> "cases", plural.
>

Here note that I suppressed the comment

@c FIXME: Does replace-buffer-in-windows _delete_ a window in case 
(1)?

Because yes, it is the case.

>> +@item dedicated
>> +The dedicated flag is not reserved to this function but have a
>                                                       ^    ^^^^
> A comma is missing before "but".  Also, please use "has" instead 
> of
> "have".
>
>> +slightly different meaning for side windows.  They receive it 
>> upon
>> +creation with the value @code{side}; it serves to prevent
>> +@code{display-buffer} to uses these windows with others action
>                          ^^^^^^^
> "to use"
>
>> +functions, and it persists across invocations of 
>> @code{quit-window},
>> +@code{kill-buffer}, @code{previous-buffer} and 
>> @code{next-buffer}
>> +(@pxref{note Window History}).  In particular, these commands 
>> will
>            ^^^^^^^^^^^^^^^^^^^
> No need for "note" here.

Thanks.  Got them.

martin rudalics <rudalics <at> gmx.at> writes:

> Thanks for the patches.  Please follow ELi's suggestions first, 
> I'll
> then comment on the manual changes.
>
> -	(current (eq (window-buffer window) (current-buffer))))
> +	(current (eq (window-buffer window) (current-buffer)))
> +        (dedicated (window-dedicated-p window)))
>      (set-window-buffer window buffer)
> +    ;; restore the dedicated side flag
> +    (when (eq dedicated 'side)
> +      (set-window-dedicated-p window 'side))
>
> Here you could use the 'dedicated-side' solution you used below 
> in
> `replace-buffer-in-windows'.

ok

> +    (or ;; first try to delete dedicated windows that are not 
> side windows
> +     (and dedicated (not (eq dedicated 'side))
> +          (window--delete window 'dedicated (eq bury-or-kill 
> 'kill)))
> +     (cond
> +       ((and (not prev-buffer)
> +             (eq (nth 1 quit-restore) 'tab)
> +             (eq (nth 3 quit-restore) buffer))
> +        (tab-bar-close-tab)
> +        ;; If the previously selected window is still alive, 
> select it.
> +        (when (window-live-p (nth 2 quit-restore))
> +          (select-window (nth 2 quit-restore))))
>
> What's wrong with putting the first disjunct into the 
> conditional as in
> the below?  In general, always try to avoid larger indentation 
> changes -
> they can make forensics cumbersome while bisecting.
>
>      (cond
>       ;; First try to delete dedicated windows that are not side 
>       windows
>       ((and dedicated (not (eq dedicated 'side))
>             (window--delete window 'dedicated (eq bury-or-kill 
>             'kill))))
>       ((and (not prev-buffer)
>              (eq (nth 1 quit-restore) 'tab)
>              (eq (nth 3 quit-restore) buffer))

The difference is a window dedicated with flag t may not be 
deletable, and in this case, we want it
to pass through the others conditionals branch of 
quit-restore-window, so it can try to use the
'quit-restore parameter, close the tab or to fallback in t, etc.
Explaining it makes me thing I could use 'window-deletable-p' in 
its conditional and ...
I guess, problem solved

In this revision I also restored the use of (select-window (nth 2 
quit-restore)) on the branch t.

> BTW, how's your paperwork process proceeding?

I am currently exchanging with the clerk to complete it.

[0003-Improve-handling-of-side-dedicated-flag.patch (text/x-diff, attachment)]

[Message part 3 (text/plain, inline)]

--

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.