From 56161250de2f68ce177530b9796f9d61c2e8ec11 Mon Sep 17 00:00:00 2001
From: ridiculousfish <corydoras@ridiculousfish.com>
Date: Tue, 31 Jan 2017 14:49:56 -0800
Subject: [PATCH] Make a missing --sets-mode property for a key binding do
 nothing

Currently, if bind is run with --mode but not --sets-mode, the
binding gets an implicit --sets-mode equivalent to the mode. This
is usually unobservable but it may matter if the mode is changed
by some internal part of the binding (e.g. set fish_bind_mode...)
then that setting will be lost after the binding is complete.
---
 src/builtin.cpp | 18 +++++-------------
 src/input.cpp   |  9 ++++++---
 2 files changed, 11 insertions(+), 16 deletions(-)

diff --git a/src/builtin.cpp b/src/builtin.cpp
index b66759e97..e03665c60 100644
--- a/src/builtin.cpp
+++ b/src/builtin.cpp
@@ -297,7 +297,7 @@ static bool builtin_bind_list_one(const wcstring &seq, const wcstring &bind_mode
         streams.out.append(L" -M ");
         streams.out.append(emode);
     }
-    if (sets_mode != bind_mode) {
+    if (!sets_mode.empty() && sets_mode != bind_mode) {
         const wcstring esets_mode = escape_string(sets_mode, ESCAPE_ALL);
         streams.out.append(L" -m ");
         streams.out.append(esets_mode);
@@ -330,13 +330,12 @@ static bool builtin_bind_list_one(const wcstring &seq, const wcstring &bind_mode
 static void builtin_bind_list(const wchar_t *bind_mode, io_streams_t &streams) {
     const std::vector<input_mapping_name_t> lst = input_mapping_get_names();
 
-    for (std::vector<input_mapping_name_t>::const_iterator it = lst.begin(), end = lst.end();
-         it != end; ++it) {
-        if (bind_mode != NULL && bind_mode != it->mode) {
+    for (const input_mapping_name_t &binding : lst) {
+        if (bind_mode != NULL && bind_mode != binding.mode) {
             continue;
         }
 
-        builtin_bind_list_one(it->seq, it->mode, streams);
+        builtin_bind_list_one(binding.seq, binding.mode, streams);
     }
 }
 
@@ -456,8 +455,7 @@ static int builtin_bind(parser_t &parser, io_streams_t &streams, wchar_t **argv)
     int all = 0;
     const wchar_t *bind_mode = DEFAULT_BIND_MODE;
     bool bind_mode_given = false;
-    const wchar_t *sets_bind_mode = DEFAULT_BIND_MODE;
-    bool sets_bind_mode_given = false;
+    const wchar_t *sets_bind_mode = L"";
     int use_terminfo = 0;
 
     w.woptind = 0;
@@ -516,7 +514,6 @@ static int builtin_bind(parser_t &parser, io_streams_t &streams, wchar_t **argv)
             }
             case 'm': {
                 sets_bind_mode = w.woptarg;
-                sets_bind_mode_given = true;
                 break;
             }
             case '?': {
@@ -530,11 +527,6 @@ static int builtin_bind(parser_t &parser, io_streams_t &streams, wchar_t **argv)
         }
     }
 
-    // if mode is given, but not new mode, default to new mode to mode.
-    if (bind_mode_given && !sets_bind_mode_given) {
-        sets_bind_mode = bind_mode;
-    }
-
     switch (mode) {
         case BIND_ERASE: {
             if (builtin_bind_erase(&argv[w.woptind], all, bind_mode_given ? bind_mode : NULL,
diff --git a/src/input.cpp b/src/input.cpp
index 9e9989b84..75fb91a36 100644
--- a/src/input.cpp
+++ b/src/input.cpp
@@ -57,7 +57,7 @@ struct input_mapping_t {
     wcstring sets_mode;
 
     input_mapping_t(const wcstring &s, const std::vector<wcstring> &c,
-                    const wcstring &m = DEFAULT_BIND_MODE, const wcstring &sm = DEFAULT_BIND_MODE)
+                    const wcstring &m, const wcstring &sm)
         : seq(s), commands(c), mode(m), sets_mode(sm) {
         static unsigned int s_last_input_map_spec_order = 0;
         specification_order = ++s_last_input_map_spec_order;
@@ -220,6 +220,8 @@ wcstring input_get_bind_mode() {
 /// Set the current bind mode.
 void input_set_bind_mode(const wcstring &bm) {
     // Only set this if it differs to not execute variable handlers all the time.
+    // modes may not be empty - empty is a sentinel value meaning to not change the mode
+    assert(! bm.empty());
     if (input_get_bind_mode() != bm.c_str()) {
         env_set(FISH_BIND_MODE_VAR, bm.c_str(), ENV_GLOBAL);
     }
@@ -449,7 +451,7 @@ static void input_mapping_execute(const input_mapping_t &m, bool allow_commands)
 
     // !has_functions && !has_commands: only set bind mode
     if (!has_commands && !has_functions) {
-        input_set_bind_mode(m.sets_mode);
+        if (!m.sets_mode.empty()) input_set_bind_mode(m.sets_mode);
         return;
     }
 
@@ -488,7 +490,8 @@ static void input_mapping_execute(const input_mapping_t &m, bool allow_commands)
         input_common_next_ch(R_NULL);
     }
 
-    input_set_bind_mode(m.sets_mode);
+    // Empty bind mode indicates to not reset the mode (#2871)
+    if (!m.sets_mode.empty()) input_set_bind_mode(m.sets_mode);
 }
 
 /// Try reading the specified function mapping.