Fix `commandline` behavior in bind functions

When a key is bound to a fish function, if that function invokes
`commandline`, it gets a stale copy of the commandline. This is because
any keys passed to `self-insert` (the default) don't actually get added
to the commandline until a special character is processed, such as the
R_NULL that gets returned after running a binding for a fish command.

To fix this, don't allow fish commands to be run for bindings if we're
processing more than one key. When a key wants to invoke a fish command,
instead we push the invocation sequence back onto the input, followed by
an R_NULL, and return. This causes the input loop to break out and
update the commandline. When it starts up again, it will re-process the
keys and invoke the fish command.

This is primarily an issue with pasting text that includes bound keys in
it. Typed text is slow enough that fish will update the commandline
between each character.

---

I don't know of any way to write a test for this, but the issue can be
reproduced as follows:

    > bind _ 'commandline -i _'

This binds _ to a command that inserts _. Typing the following works:

    > echo wat_is_it

But if you copy that line and paste it instead of typing it, the end
result looks like

    > _echo wat_isit

With this fix in place, the pasted output correctly matches the typed
output.

3 files changed, 31 insertions, 9 deletions

diff --git a/input.cpp b/input.cpp
index 128fe0fb..0d7a91be 100644
--- a/input.cpp
+++ b/input.cpp
@@ -549,8 +549,10 @@ void input_function_push_args(int code)
 
 /**
    Perform the action of the specified binding
+   allow_commands controls whether fish commands should be executed, or should
+   be deferred until later.
 */
-static void input_mapping_execute(const input_mapping_t &m)
+static void input_mapping_execute(const input_mapping_t &m, bool allow_commands)
 {
     /* By default input functions always succeed */
     input_function_status = true;
@@ -575,7 +577,7 @@ static void input_mapping_execute(const input_mapping_t &m)
         {
             input_unreadch(code);
         }
-        else
+        else if (allow_commands)
         {
             /*
              This key sequence is bound to a command, which
@@ -588,6 +590,16 @@ static void input_mapping_execute(const input_mapping_t &m)
 
             input_unreadch(R_NULL);
         }
+        else
+        {
+            /* We don't want to run commands yet. Put the characters back and return R_NULL */
+            for (wcstring::const_reverse_iterator it = m.seq.rbegin(), end = m.seq.rend(); it != end; ++it)
+            {
+                input_unreadch(*it);
+            }
+            input_unreadch(R_NULL);
+            return; /* skip the input_set_bind_mode */
+        }
     }
 
     input_set_bind_mode(m.sets_mode.c_str());
@@ -644,7 +656,7 @@ void input_unreadch(wint_t ch)
     input_common_unreadch(ch);
 }
 
-static void input_mapping_execute_matching_or_generic()
+static void input_mapping_execute_matching_or_generic(bool allow_commands)
 {
     const input_mapping_t *generic = NULL;
 
@@ -669,14 +681,14 @@ static void input_mapping_execute_matching_or_generic()
         }
         else if (input_mapping_is_match(m))
         {
-            input_mapping_execute(m);
+            input_mapping_execute(m, allow_commands);
             return;
         }
     }
 
     if (generic)
     {
-        input_mapping_execute(*generic);
+        input_mapping_execute(*generic, allow_commands);
     }
     else
     {
@@ -687,7 +699,7 @@ static void input_mapping_execute_matching_or_generic()
     }
 }
 
-wint_t input_readch()
+wint_t input_readch(bool allow_commands)
 {
     CHECK_BLOCK(R_NULL);
 
@@ -738,7 +750,10 @@ wint_t input_readch()
         else
         {
             input_unreadch(c);
-            input_mapping_execute_matching_or_generic();
+            input_mapping_execute_matching_or_generic(allow_commands);
+            // regarding allow_commands, we're in a loop, but if a fish command
+            // is executed, R_NULL is unread, so the next pass through the loop
+            // we'll break out and return it.
         }
     }
 }
diff --git a/input.h b/input.h
index 1ab43c03..18d0d2b6 100644
--- a/input.h
+++ b/input.h
@@ -103,8 +103,12 @@ void input_destroy();
    readch attempts to parse it. If no more input follows after the
    escape key, it is assumed to be an actual escape key press, and is
    returned as such.
+
+   The argument determines whether fish commands are allowed to be run
+   as bindings. If false, when a character is encountered that would
+   invoke a fish command, it is unread and R_NULL is returned.
 */
-wint_t input_readch();
+wint_t input_readch(bool allow_commands = true);
 
 /**
    Push a character or a readline function onto the stack of unread
diff --git a/reader.cpp b/reader.cpp
index bb3b57fc..c45e62b2 100644
--- a/reader.cpp
+++ b/reader.cpp
@@ -3117,7 +3117,10 @@ const wchar_t *reader_readline(void)
                             c = 0;
                             break;
                         }
-                        c = input_readch();
+                        // only allow commands on the first key; otherwise, we might
+                        // have data we need to insert on the commandline that the
+                        // commmand might need to be able to see.
+                        c = input_readch(i == 1);
                         if ((!wchar_private(c)) && (c>31) && (c != 127))
                         {
                             arr[i]=c;