Reset fish_bind_mode when changing fish_key_bindings

Also avoid resetting bindings if fish_key_bindings is "modified" without
actually changing.

Fixes #1638.

9 files changed, 107 insertions, 20 deletions

diff --git a/share/functions/__fish_config_interactive.fish b/share/functions/__fish_config_interactive.fish
index 1d3a1bea..173c7456 100644
--- a/share/functions/__fish_config_interactive.fish
+++ b/share/functions/__fish_config_interactive.fish
@@ -205,6 +205,14 @@ function __fish_config_interactive -d "Initializations that should be performed
 
 	# Reload key bindings when binding variable change
 	function __fish_reload_key_bindings -d "Reload key bindings when binding variable change" --on-variable fish_key_bindings
+		# do nothing if the key bindings didn't actually change
+		# This could be because the variable was set to the existing value
+		# or because it was a local variable
+		if test "$fish_key_bindings" = "$__fish_active_key_bindings"
+			return
+		end
+		set -g __fish_active_key_bindings "$fish_key_bindings"
+		set -g fish_bind_mode default
 		# Do something nasty to avoid two forks
 		if test "$fish_key_bindings" = fish_default_key_bindings
 			fish_default_key_bindings
diff --git a/tests/bind.expect b/tests/bind.expect
new file mode 100644
index 00000000..91585c96
--- /dev/null
+++ b/tests/bind.expect
@@ -0,0 +1,43 @@
+# vim: set filetype=expect:
+
+spawn $fish
+
+expect_prompt
+
+# test switching key bindings
+# this should leave the mode in the appropriate state
+
+send_line "set -g fish_key_bindings fish_vi_key_bindings"
+expect_prompt
+send_line -h "echo fail\033ddiecho success"
+expect_prompt -re {\r\nsuccess\r\n} {
+    puts "success"
+} -nounmatched -re {\r\nfail} {
+    puts stderr "fail"
+} unmatched {
+    puts stderr "Couldn't find expected output 'success'"
+}
+# try again without the human typing
+send_line "echo fail\033ddiecho success"
+expect_prompt -re {\r\nsuccess\r\n} {
+    puts "success"
+} -nounmatched -re {\r\nfail} {
+    puts stderr "fail"
+} unmatched {
+    puts stderr "Couldn't find expected output 'success'"
+}
+
+# still in insert mode, switch back to regular key bindings
+send_line "set -g fish_key_bindings fish_default_key_bindings"
+expect_prompt
+send_line "echo success"
+expect_prompt -re {\r\nsuccess\r\n} {
+    puts "success"
+} unmatched {
+    puts stderr "Couldn't find expected output 'success'"
+} timeout {
+    set msg ""
+    append msg "Timeout after setting fish_key_bindings to fish_default_key_bindings\n" \
+               "\$fish_bind_mode is most likely still set to 'insert'"
+    abort $msg
+}
diff --git a/tests/bind.expect.err b/tests/bind.expect.err
new file mode 100644
index 00000000..e69de29b
--- /dev/null
+++ b/tests/bind.expect.err
diff --git a/tests/bind.expect.out b/tests/bind.expect.out
new file mode 100644
index 00000000..a670195e
--- /dev/null
+++ b/tests/bind.expect.out
@@ -0,0 +1,3 @@
+success
+success
+success
diff --git a/tests/bind.expect.status b/tests/bind.expect.status
new file mode 100644
index 00000000..573541ac
--- /dev/null
+++ b/tests/bind.expect.status
@@ -0,0 +1 @@
+0
diff --git a/tests/interactive.config b/tests/interactive.config
index 911bfd65..835bdfd3 100644
--- a/tests/interactive.config
+++ b/tests/interactive.config
@@ -1,13 +1,16 @@
 # vim: set filetype=fish sw=4 ts=4 et:
 
-set -g prompt_counter 1
+# source the non-interactive config
+source $XDG_CONFIG_HOME/../tmp.config/fish/config.fish
+
+set -g prompt_counter 0
 set -g prompt_counter_incr 0
 function fish_prompt
-    echo "prompt $prompt_counter>"
     if test $prompt_counter_incr -eq 1
         set -g prompt_counter (expr $prompt_counter + 1)
         set -g prompt_counter_incr 0
     end
+    echo "prompt $prompt_counter>"
 end
 function fish_prompt_event --on-event fish_prompt
     set -g prompt_counter_incr 1
diff --git a/tests/interactive.expect.rc b/tests/interactive.expect.rc
index a1a855de..02161107 100644
--- a/tests/interactive.expect.rc
+++ b/tests/interactive.expect.rc
@@ -42,17 +42,24 @@ proc log_debug string {
 set prompt_counter 1
 # expect_prompt takes an argument list like `expect` does.
 # It supports a special pattern "unmatched" that is run if no
-# other provided patterns match.
+# other provided patterns match, and a special flag "-nounmatched"
+# that marks the following pattern as not being tracked for
+# "unmatched" handling.
+# If multiple patterns are provided, they may all match. Each pattern
+# is matched at most once. The matching only ends once the prompt is
+# found.
 proc expect_prompt {args} {
     global prompt_counter
     upvar expect_out expect_out
-    set prompt_pat [list -re "(?:\\r\\n?|^)prompt $prompt_counter>(?:$|\r)"]
+    set prompt_pat [list -re "(?:\\r\\n?|^)prompt $prompt_counter>(?:$|\\r)"]
     if {[llength $args] == 1 && [string match "\n*" $args]} {
         set args [join $args]
     }
     set prompt_action ""
     set expargs {}
-    upvar expect_out up_expect_out
+    set nounmatched no
+    set matchidx 0
+    set matched(any) no
     set state "firstarg"
     foreach arg $args {
         switch $state {
@@ -63,14 +70,17 @@ proc expect_prompt {args} {
             "action" {
                 lappend expargs [subst -nocommands {
                     log_debug "matched extra pattern to expect_prompt: [quote \$expect_out(0,string)]"
-                    if {\$matched} {
-                        exp_continue
+                    if {!\$matched($matchidx)} {
+                        set matched($matchidx) yes
+                        if {!$nounmatched} { set matched(any) yes }
+                        uplevel 1 {$arg}
                     }
-                    set matched yes
-                    uplevel 1 {$arg}
                     exp_continue
                 }]
+                set matched($matchidx) no
+                incr matchidx
                 set state "firstarg"
+                set nounmatched no
             }
             "firstarg" -
             "arg" {
@@ -78,8 +88,8 @@ proc expect_prompt {args} {
                     set state "unmatched"
                     continue
                 }
-                lappend expargs $arg
-                switch $arg {
+                set keep yes
+                switch -glob -- $arg {
                     -gl -
                     -re -
                     -ex {
@@ -89,6 +99,20 @@ proc expect_prompt {args} {
                     -timeout {
                         set state "flagarg"
                     }
+                    -nounmatched {
+                        set keep no
+                        set nounmatched yes
+                        set state "arg"
+                    }
+                    -* {
+                        error "BUG: unknown expect flag in expect_prompt"
+                    }
+                    default {
+                        set state "pat"
+                    }
+                }
+                if {$keep} {
+                    lappend expargs $arg
                 }
             }
             "flagarg" {
@@ -96,9 +120,11 @@ proc expect_prompt {args} {
                 set state "arg"
             }
             "unmatched" {
+                set state "firstarg"
                 if {$prompt_action ne ""} continue
                 set prompt_action [subst -nocommands {
-                    if {!\$matched} {
+                    if {!\$matched(any)} {
+                        log_debug "triggered unmatched action in expect_prompt"
                         uplevel 1 {$arg}
                     }
                 }]
@@ -114,7 +140,6 @@ proc expect_prompt {args} {
         log_info "expecting prompt $prompt_counter"
     }
     set expargs [concat $prompt_pat [list $prompt_action] $expargs]
-    set matched no
     expect {*}$expargs
     incr prompt_counter
 }
@@ -128,6 +153,7 @@ proc trace_expect {cmd args} {
     switch [lindex $args end] {
         enter {
             log_debug "entering expect"
+            log_debug "command: $cmd"
             uplevel {set expect_out(buffer) {}}
         }
         leave {
@@ -160,7 +186,7 @@ proc trace_spawn {cmd args} {
         }
         leave {
             log_debug "[quote $cmd]: code [lindex $args 0], result [lindex $args 1]"
-            expect_before {
+            expect_after {
                 timeout {
                     expect "*" {
                         log_debug "timeout; buffer=[quote $expect_out(buffer)]"
@@ -216,7 +242,6 @@ proc print_var_contents name {
 
     # match on the results
     set pat {\r\n@GUARD:$guard@\r\n(.*)\r\n@/GUARD:$guard@\r\n}
-    set matched false
     expect_prompt -re [subst -nocommands -nobackslashes $pat] {
         log_info "get_var_contents: result: [quote $expect_out(1,string)]"
         puts $expect_out(1,string)
diff --git a/tests/interactive.fish b/tests/interactive.fish
index 987ad80b..369b8546 100644
--- a/tests/interactive.fish
+++ b/tests/interactive.fish
@@ -15,20 +15,21 @@ for i in *.expect
     begin
         set -lx XDG_CONFIG_HOME $PWD/tmp.interactive.config
         set -lx TERM dumb
-        expect -n -c 'source interactive.expect.rc' -f $i >tmp.out ^tmp.err
+        expect -n -c 'source interactive.expect.rc' -f $i >$i.tmp.out ^$i.tmp.err
     end
     set -l tmp_status $status
     set res ok
-    if not diff tmp.out $i.out >/dev/null
+    mv -f interactive.tmp.log $i.tmp.log
+    if not diff $i.tmp.out $i.out >/dev/null
         set res fail
         echo "Output differs for file $i. Diff follows:"
-        diff -u tmp.out $i.out
+        diff -u $i.tmp.out $i.out
     end
 
-    if not diff tmp.err $i.err >/dev/null
+    if not diff $i.tmp.err $i.err >/dev/null
         set res fail
         echo "Error output differs for file $i. Diff follows:"
-        diff -u tmp.err $i.err
+        diff -u $i.tmp.err $i.err
     end
 
     if test $tmp_status != (cat $i.status)
@@ -38,6 +39,8 @@ for i in *.expect
 
     if test $res = ok
         echo "File $i tested ok"
+        # clean up tmp files
+        rm -f $i.tmp.{err,out,log}
     else
         echo "File $i failed tests"
     end
diff --git a/tests/interactive.out b/tests/interactive.out
index 53577b14..87a9dd13 100644
--- a/tests/interactive.out
+++ b/tests/interactive.out
@@ -1 +1,2 @@
+File bind.expect tested ok
 File read.expect tested ok