bug#39687: 26.3; Add customize-variable option for not locking keyboards

bug#39687: 26.3; Add customize-variable option for not locking keyboards

[Logan Perkins]

In GNU Emacs 26.3 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.11)
of 2020-01-28 built on gentoo-server
Windowing system distributor 'The X.Org Foundation', version 11.0.12005000
System Description: Gentoo Base System release 2.6

This is a feature request more than a bug report. When using the
built-in server (either via `emacs --daemon` or `server-start`) +
emacsclient, use of the minibuffer from one client completely blocks
other clients (they can't even quit until the mini buffer finishes).
This is governed by calls to
`temporarily_switch_to_single_kboard(struct frame *f)` in `keyboard.c`.

If I understand correctly, there are two reasons for locking other
clients while the minibuffer is in use. First, the input for the
minibuffer is stored in a single global variable; while enabling
recursive minibuffers is possible, bottom line is there can only be *one*
active mini buffer at a time. Locking secondary inputs reduces the
potential for confusion with fighting over the minibuffer.

Additionally, sometimes there is something which requires a user
response (such as confirmation on killing a modified buffer), and
resolving that is simpler if the state isn't changing in the background.

On the other hand, even with a confirmation box open, a user can switch
away from the minibuffer and continue changing state (potentially even
opening a recursive minibuffer), so I don't think the second case is
sufficient cause to disallow multi-keyboard mode when the minibuffer is
in use.

As for the first issue, I don't think the present behavior is clearly
best, as it doesn't *ignore* secondary keyboard input, it *queues*
it, executing it in one block when the minibuffer ends. This can cause
unexpected issues for novice users. Also, the inability to even close
the client (short of SIGTERMing it) is not ideal.

I've gutted the
`temporarily_switch_to_single_kboard(struct frame *f)` in `keyboard.c`
on a test system, and successfully used it with multiple people sharing
a single server instance (on a joint project), and it works reasonably
well. I'd like to propose adding a customizeable variable in the
`minibuffer` group which disables locking the other keyboards. Ideally,
the other clients should get a "minibuffer in use" message in their
minibuffers so users can see when someone is using the minibuffer.

I am happy to work on this, and submit patches for it, but would
appreciate some advice before I start.

Is there some further reason to lock the keyboard that I haven't
considered?

I think it's better to use the customizeable variable to prevent the 
call to
temporarily_switch_to_single_kboard, rather than have that function not
do what it's name implies it does. Should I intercept all calls to it
based on one new variable? Or should I split general minibuffer use
from confirmation uses, and so on? (Looks like 3-4 different places
it's called in the source).

Should I make the behavior depend on some elisp function? I think that
might be the easiest way to support the "minibuffer in use" message and
the like, but I'm not sure what the downside would be.

Is it a waste of time for me to submit patches related to this feature?
If there's zero interest in adding this, or it would be less work for
someone else to write it than review my patches, I won't waste your time
sending them.

Regards,
Logan Perkins

bug#39687: 26.3; Add customize-variable option for not locking keyboards

[Eli Zaretskii]

> From: Logan Perkins <logan@lp-programming.com>
> Date: Wed, 19 Feb 2020 21:01:30 -0800
> 
> Is there some further reason to lock the keyboard that I haven't
> considered?

Can we back up a little, and discuss the use cases where the current
behavior presents a limitation?  Is quitting in the other clients the
only one, or are there more?

Also, are you implicitly saying that several persons work
simultaneously vis-à-vis the same Emacs server?  Because if not, I'm
not sure I understand how simultaneous need to input from different
clients could even happen.

> Should I make the behavior depend on some elisp function? I think that
> might be the easiest way to support the "minibuffer in use" message and
> the like, but I'm not sure what the downside would be.
> 
> Is it a waste of time for me to submit patches related to this feature?
> If there's zero interest in adding this, or it would be less work for
> someone else to write it than review my patches, I won't waste your time
> sending them.

Speaking for myself, I think the interest depends on the relevant use
cases where the current behavior implies restrictions.  Thus my
questions above.

In any case, we thank you for your interest in Emacs and look forward
to seeing your contributions, but I suggest to start your legal
paperwork rolling now, because changes you are talking about will
probably be non-trivial in length, so we will need a copyright
assignment from you in order to accept the changes.  If you agree, I
can send you the form to fill and the instructions to go with it.

bug#39687: 26.3; Add customize-variable option for not locking keyboards

[Eli Zaretskii]

[Please keep the bug address on the CC list, so this whole discussion
is recorded by the Emacs issue tracker.]

> From: Logan Perkins <logan@lp-programming.com>
> Date: Fri, 21 Feb 2020 10:37:39 -0800
> 
> On 2/21/20 12:23 AM, Eli Zaretskii wrote:
>  >> From: Logan Perkins <logan@lp-programming.com>
>  >> Date: Wed, 19 Feb 2020 21:01:30 -0800
>  >>
>  >> Is there some further reason to lock the keyboard that I haven't
>  >> considered?
>  >
>  > Can we back up a little, and discuss the use cases where the current
>  > behavior presents a limitation? Is quitting in the other clients the
>  > only one, or are there more?
> 
> Quitting in other clients is one, but fairly minor (C-z; kill %1 will
> get you out of it). Switching clients generally is another minor case.
> If you walk away with the minibuffer open by accident, and then try to
> use a remote client (via SSH or similar) later, it's locked (you can
> work around this by registering a SIGUSR handler to close the
> minibuffer, but that's not ideal).

These seem to be valid use cases, so I tend to agree we should have an
easier way of breaking out of the minibuffer input in another client.

>  > Also, are you implicitly saying that several persons work
>  > simultaneously vis-à-vis the same Emacs server? Because if not, I'm
>  > not sure I understand how simultaneous need to input from different
>  > clients could even happen.
> 
> That's exactly the use-case where it matters most. If you're familiar
> with Ludum Dare and similar code-sprints, it's pretty common to
> have multiple people working on the same files at the same time. Having
> a shared editor makes it faster and easier to draw attention to exactly
> where one person needs help. It's also great for teaching (when you
> aren't physically in front of the same computer), or for onboarding new
> team members. Screen (the terminal multiplexer) can be used to similar
> effect, but the ability to simultaneously edit the *same* file is
> specific to emacs.

I don't understand what you expect Emacs to do in these use cases.  If
we process inputs from several clients as they arrive, we could
produce results that are unexpected and even disastrous.  For example,
suppose we receive C-x from one client followed by C-u from another
followed by C-s from the first one -- if we process these in the order
they were received, the result will be none of what the two clients
intended.

Maybe you thought that our input code will process input in chunks of
complete sequences, and thus avoid the above-mentioned disasters, but
then (a) I think we will need a very thorough restructuring of the
current code in keyboard.c, as it currently decides on this
dynamically; and (b) you will still have the same problem if the user
of one client types C-x and then pauses for some reason.

So I'm afraid I don't see what kind of solution is sought for here,
please clarify.

>  > In any case, we thank you for your interest in Emacs and look forward
>  > to seeing your contributions, but I suggest to start your legal
>  > paperwork rolling now, because changes you are talking about will
>  > probably be non-trivial in length, so we will need a copyright
>  > assignment from you in order to accept the changes. If you agree, I
>  > can send you the form to fill and the instructions to go with it.
> 
> I have no problem assigning copyright for my work on FSF projects to the
> FSF. I live in Eastern Washington, and am self employed, so getting
> the paperwork done should be about as trivial as it can be.

Thanks, I will send the form off-list.

bug#39687: 26.3; Add customize-variable option for not locking keyboards

[Logan Perkins]

On 2/22/20 1:27 AM, Eli Zaretskii wrote:
 > [Please keep the bug address on the CC list, so this whole discussion
 > is recorded by the Emacs issue tracker.]

Oops! I hit reply instead of reply-all, not sure why Thunderbird
doesn't make that the default, but I'll be more mindful of that in the 
future.

 >
 >> From: Logan Perkins <logan@lp-programming.com>
 >> Date: Fri, 21 Feb 2020 10:37:39 -0800
 >>
 >> On 2/21/20 12:23 AM, Eli Zaretskii wrote:
 >> >> From: Logan Perkins <logan@lp-programming.com>
 >> >> Date: Wed, 19 Feb 2020 21:01:30 -0800
 >> >>
 >> >> Is there some further reason to lock the keyboard that I haven't
 >> >> considered?
 >> >
 >> > Can we back up a little, and discuss the use cases where the current
 >> > behavior presents a limitation? Is quitting in the other clients the
 >> > only one, or are there more?
 >>
 >> Quitting in other clients is one, but fairly minor (C-z; kill %1 will
 >> get you out of it). Switching clients generally is another minor case.
 >> If you walk away with the minibuffer open by accident, and then try to
 >> use a remote client (via SSH or similar) later, it's locked (you can
 >> work around this by registering a SIGUSR handler to close the
 >> minibuffer, but that's not ideal).
 >
 > These seem to be valid use cases, so I tend to agree we should have an
 > easier way of breaking out of the minibuffer input in another client.
 >
 >> > Also, are you implicitly saying that several persons work
 >> > simultaneously vis-�-vis the same Emacs server? Because if not, I'm
 >> > not sure I understand how simultaneous need to input from different
 >> > clients could even happen.
 >>
 >> That's exactly the use-case where it matters most. If you're familiar
 >> with Ludum Dare and similar code-sprints, it's pretty common to
 >> have multiple people working on the same files at the same time. Having
 >> a shared editor makes it faster and easier to draw attention to exactly
 >> where one person needs help. It's also great for teaching (when you
 >> aren't physically in front of the same computer), or for onboarding new
 >> team members. Screen (the terminal multiplexer) can be used to similar
 >> effect, but the ability to simultaneously edit the *same* file is
 >> specific to emacs.
 >
 > I don't understand what you expect Emacs to do in these use cases. If
 > we process inputs from several clients as they arrive, we could
 > produce results that are unexpected and even disastrous. For example,
 > suppose we receive C-x from one client followed by C-u from another
 > followed by C-s from the first one -- if we process these in the order
 > they were received, the result will be none of what the two clients
 > intended.
 >
 > Maybe you thought that our input code will process input in chunks of
 > complete sequences, and thus avoid the above-mentioned disasters, but
 > then (a) I think we will need a very thorough restructuring of the
 > current code in keyboard.c, as it currently decides on this
 > dynamically; and (b) you will still have the same problem if the user
 > of one client types C-x and then pauses for some reason.
 >
 > So I'm afraid I don't see what kind of solution is sought for here,
 > please clarify.


So I've just done some more testing, with emacs 26.3-r1 (the latest
stable version in Gentoo), and emacs 27.0.50_pre20191223, the latest
snapshot, both compiled with and without the gutted
switch_to_single_kboard... (I'll see about getting the latest
development version from the repo probably this evening).

Looks like keyboard handling got changed sometime between the two
versions. I ran the following sequence in all 4 copies.

1. ./emacs -nw #on seat0
2. M-x start-server
3. ./emacsclient -t #on seat1
4. Switch both to the scratch buffer
5. Put each seat's cursor on its own line
6. On seat0, type abcde
7. On seat1, type 12345
8. On seat0, type C-x
9. On seat1, type u
10. On seat1, type C-x
11. On seat0, type C-c

Note that this sequence doesn't try to put emacs into single keyboard
mode, so the gutted function has no impact on the results.

In emacs-26, step 9 inserts a literal u on seat1's line, step 11 closes
emacs. Before step 11, both seat and seat1 display C-x as "breadcrumbs"
in the minibuffer.

In emacs-27, step 9 undoes seat0's last action (removes abcde), step 11
closes seat1's emacsclient.

Obviously, the behaviour in emacs-27 precludes simultanuous input. I
also think it's poor behavior, even if we don't unlock the keyboard when
the minibuffer is in use, since someone walking away after hitting C-x
(or some other partial command) and then connecting later will get
unexpected (and probably unseen) results with their first keypress.

I'll see if I can figure out which changes in keyboard.c account for the
changed behavior, and what the reason for them was.

bug#39687: 26.3; Add customize-variable option for not locking keyboards

[Logan Perkins]

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

On 2/22/20 1:27 AM, Eli Zaretskii wrote:

> > Also, are you implicitly saying that several persons work
>>   > simultaneously vis-à-vis the same Emacs server? Because if not, I'm
>>   > not sure I understand how simultaneous need to input from different
>>   > clients could even happen.
>>
>> That's exactly the use-case where it matters most. If you're familiar
>> with Ludum Dare and similar code-sprints, it's pretty common to
>> have multiple people working on the same files at the same time. Having
>> a shared editor makes it faster and easier to draw attention to exactly
>> where one person needs help. It's also great for teaching (when you
>> aren't physically in front of the same computer), or for onboarding new
>> team members. Screen (the terminal multiplexer) can be used to similar
>> effect, but the ability to simultaneously edit the *same* file is
>> specific to emacs.
> I don't understand what you expect Emacs to do in these use cases.  If
> we process inputs from several clients as they arrive, we could
> produce results that are unexpected and even disastrous.  For example,
> suppose we receive C-x from one client followed by C-u from another
> followed by C-s from the first one -- if we process these in the order
> they were received, the result will be none of what the two clients
> intended.
>
> Maybe you thought that our input code will process input in chunks of
> complete sequences, and thus avoid the above-mentioned disasters, but
> then (a) I think we will need a very thorough restructuring of the
> current code in keyboard.c, as it currently decides on this
> dynamically; and (b) you will still have the same problem if the user
> of one client types C-x and then pauses for some reason.
>
> So I'm afraid I don't see what kind of solution is sought for here,
> please clarify.

Alright, I finally had time to dig in to what commit broke the split input.
The commit was e3cebbb839fc94f314659bf667c6790edebf4297, from 19 October 2019.
It was to fix Bug#37782, and improve the fix for Bug#5095.

Reverting that commit resolves the issue, but obviously reintroduces Bug#37782.
I *think* I have a patch that still fixes the current behavior, and does not
reintroduce those two bugs, I've included it below.  Basically, the fix for
Bug#5095 should only be applied if we are in the right context.  If we're not,
the if block above puts a Qswitch_frame at the head of the side queue and
triggers replay_entire_sequence, so we just skip the second check.  It'll get
run again and catch the interruption on the next pass, but in the right context.

diff --git a/src/keyboard.c b/src/keyboard.c
index f9b9399d50..90ed1d3e9a 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -9599,17 +9599,23 @@ read_key_sequence (Lisp_Object *keybuf, Lisp_Object prompt,
                       (interrupted_kboard,
                        Fcons (make_lispy_switch_frame (frame),
                               KVAR (interrupted_kboard, kbd_queue)));
+                   mock_input = 0;
+                 }
+               else
+                 {
+                   if (FIXNUMP (key) && XFIXNUM (key) != -2)
+                     {
+                       /* If interrupted while initializing terminal, we
+                          need to replay the interrupting key.  See
+                          Bug#5095 and Bug#37782.  */
+                       mock_input = 1;
+                       keybuf[0] = key;
+                     }
+                   else
+                     {
+                       mock_input = 0;
+                     }
                   }
-                if (FIXNUMP (key) && XFIXNUM (key) != -2)
-                  {
-                    /* If interrupted while initializing terminal, we
-                       need to replay the interrupting key.  See
-                       Bug#5095 and Bug#37782.  */
-                    mock_input = 1;
-                    keybuf[0] = key;
-                  }
-                else
-                  mock_input = 0;
                 goto replay_entire_sequence;
               }
           }




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

bug#39687: 26.3; Add customize-variable option for not locking keyboards

[Lars Ingebrigtsen]

Logan Perkins <logan@lp-programming.com> writes:

> I *think* I have a patch that still fixes the current behavior, and
> does not reintroduce those two bugs, I've included it below.
> Basically, the fix for Bug#5095 should only be applied if we are in
> the right context.  If we're not, the if block above puts a
> Qswitch_frame at the head of the side queue and triggers
> replay_entire_sequence, so we just skip the second check.  It'll get
> run again and catch the interruption on the next pass, but in the
> right context.

Eli, did you have a chance to take a look at the proposed patch here?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#39687: 26.3; Add customize-variable option for not locking keyboards

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: Eli Zaretskii <eliz@gnu.org>,  39687@debbugs.gnu.org
> Date: Thu, 01 Oct 2020 20:44:30 +0200
> 
> Logan Perkins <logan@lp-programming.com> writes:
> 
> > I *think* I have a patch that still fixes the current behavior, and
> > does not reintroduce those two bugs, I've included it below.
> > Basically, the fix for Bug#5095 should only be applied if we are in
> > the right context.  If we're not, the if block above puts a
> > Qswitch_frame at the head of the side queue and triggers
> > replay_entire_sequence, so we just skip the second check.  It'll get
> > run again and catch the interruption on the next pass, but in the
> > right context.
> 
> Eli, did you have a chance to take a look at the proposed patch here?

I didn't feel that I understand the issue well enough, and this area
is notorious for having all kinds of dark corners.

bug#39687: 26.3; Add customize-variable option for not locking keyboards

[Lars Ingebrigtsen]

Logan Perkins <logan@lp-programming.com> writes:

> I *think* I have a patch that still fixes the current behavior, and
> does not reintroduce those two bugs, I've included it below.
> Basically, the fix for Bug#5095 should only be applied if we are in
> the right context.  If we're not, the if block above puts a
> Qswitch_frame at the head of the side queue and triggers
> replay_entire_sequence, so we just skip the second check.  It'll get
> run again and catch the interruption on the next pass, but in the
> right context.

I tried the various test cases, and the behaviour with your patch
clearly seems better to me, so I've applied it to Emacs 28.

But as Eli says, this is a ticklish area, so if this (re-)introduces
other bugs, we'll have to revert.

This change was small enough to apply without assigning copyright to the
FSF, but for future patches you want to submit, it might make sense to
get the paperwork started now, so that subsequent patches can be applied
speedily. Would you be willing to sign such paperwork?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#39687: 26.3; Add customize-variable option for not locking keyboards

[Logan Perkins]

On Wed, 21 Jul 2021 08:57:44 -0700,
Lars Ingebrigtsen wrote:
> 
> Logan Perkins <logan@lp-programming.com> writes:
> 
> > I *think* I have a patch that still fixes the current behavior, and
> > does not reintroduce those two bugs, I've included it below.
> > Basically, the fix for Bug#5095 should only be applied if we are in
> > the right context.  If we're not, the if block above puts a
> > Qswitch_frame at the head of the side queue and triggers
> > replay_entire_sequence, so we just skip the second check.  It'll get
> > run again and catch the interruption on the next pass, but in the
> > right context.
> 
> I tried the various test cases, and the behaviour with your patch
> clearly seems better to me, so I've applied it to Emacs 28.

I assume you only applied the patch I supplied, which improves the fix
for Bug#5095, but does not address Bug#39687.  Hopefully, with that
sorted, we can discuss the issues around `temporarily_switch_to_single_kboard`.

> 
> But as Eli says, this is a ticklish area, so if this (re-)introduces
> other bugs, we'll have to revert.

Fair enough.  I am fairly confident that patch won't cause problems
(but it might allow latent problems to surface).  This is, indeed, a
tricky area: it took the better part of 2 hours last year to "wrap my
head around" the issue well enough to write the patch.  If any other
issues arise, I'm happy to address those.

> 
> This change was small enough to apply without assigning copyright to the
> FSF, but for future patches you want to submit, it might make sense to
> get the paperwork started now, so that subsequent patches can be applied
> speedily. Would you be willing to sign such paperwork?

I submitted my assignment paperwork 18 May 2020, signed via GPG.
Mr. Topham responded (on 4 Jun 2020) that he was having difficulty
verifying the signature (possibly due to it being an EDDSA key?), and
that he would try again.  I have heard nothing since, but did not
follow up as this thread was stalled, and last summer was a
bit... intense.  I still have the assignment document and can submit
it again if that would help.

Regards,
Logan

bug#39687: 26.3; Add customize-variable option for not locking keyboards

[Eli Zaretskii]

> Date: Wed, 21 Jul 2021 10:52:34 -0700
> From: Logan Perkins <logan@lp-programming.com>
> Cc: Eli Zaretskii <eliz@gnu.org>,
> 	39687@debbugs.gnu.org
> 
> > This change was small enough to apply without assigning copyright to the
> > FSF, but for future patches you want to submit, it might make sense to
> > get the paperwork started now, so that subsequent patches can be applied
> > speedily. Would you be willing to sign such paperwork?
> 
> I submitted my assignment paperwork 18 May 2020, signed via GPG.
> Mr. Topham responded (on 4 Jun 2020) that he was having difficulty
> verifying the signature (possibly due to it being an EDDSA key?), and
> that he would try again.  I have heard nothing since, but did not
> follow up as this thread was stalled, and last summer was a
> bit... intense.  I still have the assignment document and can submit
> it again if that would help.

Please just ping the FSF copyright clerk with the above information.

bug#39687: 26.3; Add customize-variable option for not locking keyboards

[Lars Ingebrigtsen]

Logan Perkins <logan@lp-programming.com> writes:

> I assume you only applied the patch I supplied, which improves the fix
> for Bug#5095, but does not address Bug#39687.

Oh, sorry; I forgot what the bug report was really about.  Now
reopened.  :-)

> Hopefully, with that sorted, we can discuss the issues around
> `temporarily_switch_to_single_kboard`.

Yes.  This is basically the same issue as in bug#9729, so I'll merge the
two.  And I think it'd be great to have this fixed.

>  I still have the assignment document and can submit
> it again if that would help.

That'd be good; or ask copyright-clerk@fsf.org what the status is.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#39687: 26.3; Add customize-variable option for not locking keyboards

[Logan Perkins]

On Wed, 21 Jul 2021 15:08:45 -0700,
Lars Ingebrigtsen wrote:
> 
> Logan Perkins <logan@lp-programming.com> writes:
> > Hopefully, with that sorted, we can discuss the issues around
> > `temporarily_switch_to_single_kboard`.
> 
> Yes.  This is basically the same issue as in bug#9729, so I'll merge the
> two.  And I think it'd be great to have this fixed.

Yes, it is the same as bug#9729, or at least the same underlying
cause/problem.  I missed that bug when I went looking to see if this
was already supported, as that only mentions it being a problem in
context of opening files.  The underlying problem applies anytime the
minibuffer is in use.  Still, makes sense to merge them.  

I just read that bug report end-to-end.  I *think* for part of its
discussion they were running into the fix from bug#5095.  While it is
true that emacs (being single threaded) only reads from one virtual
keyboard at a time, it is capable of switching between virtual
keyboards and interleaving their input.  Nor is it the case that only
one minibuffer input is possible at a time.  This is fundamentally how
`enable-recursive-minibuffers` works, and each client has its own
minibuffer even when that is `nil`.

Anyway, the fix for this needs to be gated behind an option (at least
initially), as it does cause some odd interactions with the
minibuffer.  (For example, when two clients both open a minibuffer,
when the first client finishes with the minibuffer, the prompt
vanishes on the second client, and the focus switches away, but the
minibuffer remains open).  I think these problems can be worked
through, but until they are, we need people to opt-in to them.  It's
also worth noting that, without recursive minibuffers, the vanishing
minibuffer issue doesn't occur, but one client gets a minibuffer in
use message instead.

> 
> >  I still have the assignment document and can submit
> > it again if that would help.
> 
> That'd be good; or ask copyright-clerk@fsf.org what the status is.
> 

I sent a message to that email an hour (ish) ago.  

bug#39687: 26.3; Add customize-variable option for not locking keyboards

[Lars Ingebrigtsen]

Logan Perkins <logan@lp-programming.com> writes:

> Anyway, the fix for this needs to be gated behind an option (at least
> initially), as it does cause some odd interactions with the
> minibuffer. 

Yup.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no