From 814cb51eb5dafac779324cf8a9ce4c7894b3af9a Mon Sep 17 00:00:00 2001 From: Kurtis Rader Date: Tue, 22 Dec 2015 17:58:38 -0800 Subject: [PATCH 1/8] improve handling of the escape character Increase the delay between seeing an escape character and giving up on whether additional characters that match a key binding are seen. I'm setting the value to 500 ms to match the readline library. We don't need such a large window for sequences transmitted by a terminal (even over ssh where network delays can be a problem). However, we can't expect humans to reliably press the escape key followed by another key with an inter-char delay of less than ~250 ms based on my testing and research. A value of 500 ms provides a nice experience even for people using "fish_vi_mode" bindings as a half second to switch from insert to normal mode is still fast enough that most people won't even notice. Resolves #1356 --- src/input_common.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/input_common.cpp b/src/input_common.cpp index 24fde6e2d..cda4ca11e 100644 --- a/src/input_common.cpp +++ b/src/input_common.cpp @@ -32,8 +32,10 @@ Implementation file for the low level input library Time in milliseconds to wait for another byte to be available for reading after \\x1b is read before assuming that escape key was pressed, and not an escape sequence. + + This is the value used by the readline library. */ -#define WAIT_ON_ESCAPE 10 +#define WAIT_ON_ESCAPE 500 /** Characters that have been read and returned by the sequence matching code */ static std::deque lookahead_list; From 6969cfab3dc20de9f757b94b8832b504ed1fdd40 Mon Sep 17 00:00:00 2001 From: Kurtis Rader Date: Wed, 23 Dec 2015 15:24:45 -0800 Subject: [PATCH 2/8] fix unit tests related to the escape timeout --- tests/bind.expect | 57 ++++++++++++++++++++++++------------------ tests/bind.expect.out | 8 +++--- tests/interactive.fish | 14 +++++++++-- tests/test.fish | 1 - 4 files changed, 48 insertions(+), 32 deletions(-) mode change 100644 => 100755 tests/interactive.fish diff --git a/tests/bind.expect b/tests/bind.expect index 252709557..6c80b8f11 100644 --- a/tests/bind.expect +++ b/tests/bind.expect @@ -1,48 +1,55 @@ # vim: set filetype=expect: - spawn $fish - expect_prompt -# test switching key bindings -# this should leave the mode in the appropriate state +# Test switching key bindings to vi mode. +# This should leave the mode in the appropriate state (i.e., insert mode). -send_line "set -g fish_key_bindings fish_vi_key_bindings" +send "set -g fish_key_bindings fish_vi_key_bindings\r" expect_prompt -send_line -h "echo fail\033ddiecho success" +send -h "echo fail\033" +# Delay needed to allow fish to transition to vi "normal" mode. +sleep 0.510 +send -h "ddiecho success\r" expect_prompt -re {\r\nsuccess\r\n} { - puts "success" + puts "vi replace line 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 -h "echo fail\033ddiecho success" -expect_prompt -re {\r\nsuccess\r\n} { - puts "success" -} -nounmatched -re {\r\nfail} { - puts stderr "fail" + puts stderr "vi replace line fail" } unmatched { puts stderr "Couldn't find expected output 'success'" } -# Test lowercase-r replace -send_line -h "\033ddiecho TEXT\033hhrAi" +# Verify that a human can transpose words using \et (which is an emacs default +# binding but should be valid while in vi insert mode). +send "echo abc def\033" +# Fish should still be in vi "insert" mode after this delay. +sleep 0.400 +send "t\r" +expect_prompt -re {\r\ndef abc\r\n} { + puts "vi transpose words success" +} unmatched { + puts stderr "vi transpose words fail" +} + +# Test replacing a single character. +send -h "echo TEXT\033" +# Delay needed to allow fish to transition to vi "normal" mode. +sleep 0.510 +send -h "hhrAi\r" expect_prompt -re {\r\nTAXT\r\n} { - puts "replace success" + puts "vi mode replace success" } -nounmatched -re {\r\nfail} { - puts stderr "replace fail" + puts stderr "vi mode replace fail" } unmatched { puts stderr "Couldn't find expected output 'TAXT'" } -# still in insert mode, switch back to regular key bindings -send_line -h "set -g fish_key_bindings fish_default_key_bindings" +# Switch back to regular (emacs mode) key bindings. +send -h "set -g fish_key_bindings fish_default_key_bindings\r" expect_prompt -send_line "echo success" +send "echo success\r" expect_prompt -re {\r\nsuccess\r\n} { - puts "success" + puts "emacs success" } unmatched { puts stderr "Couldn't find expected output 'success'" } timeout { diff --git a/tests/bind.expect.out b/tests/bind.expect.out index 7f883abdc..f82eec235 100644 --- a/tests/bind.expect.out +++ b/tests/bind.expect.out @@ -1,4 +1,4 @@ -success -success -replace success -success +vi replace line success +vi transpose words success +vi mode replace success +emacs success diff --git a/tests/interactive.fish b/tests/interactive.fish old mode 100644 new mode 100755 index ad53e2624..9f5e979ce --- a/tests/interactive.fish +++ b/tests/interactive.fish @@ -2,7 +2,17 @@ # # Interactive tests using `expect` -source test_util.fish (status -f); or exit +# Change to directory containing this script +cd (dirname (status -f)) + +# Test files specified on commandline, or all *.expect files +if set -q argv[1] + set files_to_test $argv.expect +else + set files_to_test *.expect +end + +source test_util.fish (status -f) $argv; or exit say -o cyan "Testing interactive functionality" if not type -q expect @@ -65,7 +75,7 @@ function test_file end set -l failed -for i in *.expect +for i in $files_to_test if not test_file $i set failed $failed $i end diff --git a/tests/test.fish b/tests/test.fish index 994e7a764..588a8f354 100755 --- a/tests/test.fish +++ b/tests/test.fish @@ -6,7 +6,6 @@ cd (dirname (status -f)) # Test files specified on commandline, or all *.in files -set -q argv[1] if set -q argv[1] set files_to_test $argv.in else From 4b9fece9f48615f2e8068054c60763e844415a7c Mon Sep 17 00:00:00 2001 From: Kurtis Rader Date: Thu, 14 Jan 2016 21:46:53 -0800 Subject: [PATCH 3/8] allow configuring the escape delay timeout Introduce a "fish_escape_delay_ms" variable to allow the user to configure the delay used when seeing a bare escape before assuming no other characters will be received that might match a bound character sequence. This is primarily useful for vi mode so that a bare escape character (i.e., keystroke) can switch to vi "insert" to "normal" mode in a timely fashion. --- src/env.cpp | 6 ++- src/input_common.cpp | 59 +++++++++++++-------- src/input_common.h | 3 ++ tests/bind.expect | 119 ++++++++++++++++++++++++++++++++---------- tests/bind.expect.out | 12 +++-- 5 files changed, 145 insertions(+), 54 deletions(-) diff --git a/src/env.cpp b/src/env.cpp index 3d7023db3..527ed253b 100644 --- a/src/env.cpp +++ b/src/env.cpp @@ -303,7 +303,7 @@ static void handle_locale() } -/** React to modifying hte given variable */ +/** React to modifying the given variable */ static void react_to_variable_change(const wcstring &key) { if (var_is_locale(key)) @@ -319,6 +319,10 @@ static void react_to_variable_change(const wcstring &key) { reader_react_to_color_change(); } + else if (key == L"fish_escape_delay_ms") + { + update_wait_on_escape_ms(); + } } /** diff --git a/src/input_common.cpp b/src/input_common.cpp index cda4ca11e..291a37a7b 100644 --- a/src/input_common.cpp +++ b/src/input_common.cpp @@ -33,9 +33,11 @@ Implementation file for the low level input library reading after \\x1b is read before assuming that escape key was pressed, and not an escape sequence. - This is the value used by the readline library. + This is the value used by the readline library. It can be overridden by + setting the fish_escape_delay_ms variable. */ -#define WAIT_ON_ESCAPE 500 +#define WAIT_ON_ESCAPE_DEFAULT 500 +static int wait_on_escape_ms = WAIT_ON_ESCAPE_DEFAULT; /** Characters that have been read and returned by the sequence matching code */ static std::deque lookahead_list; @@ -79,6 +81,7 @@ static int (*interrupt_handler)(); void input_common_init(int (*ih)()) { interrupt_handler = ih; + update_wait_on_escape_ms(); } void input_common_destroy() @@ -214,36 +217,48 @@ static wint_t readb() return arr[0]; } +// Update the wait_on_escape_ms value in response to the fish_escape_delay_ms +// user variable being set. +void update_wait_on_escape_ms() +{ + env_var_t escape_time_ms = env_get_string(L"fish_escape_delay_ms"); + if (escape_time_ms.missing_or_empty()) + { + wait_on_escape_ms = WAIT_ON_ESCAPE_DEFAULT; + return; + } + + wchar_t *endptr; + long tmp = wcstol(escape_time_ms.c_str(), &endptr, 10); + + if (*endptr != '\0' || tmp < 10 || tmp >= 5000) + { + fwprintf(stderr, L"ignoring fish_escape_delay_ms: value '%ls' " + "is not an integer or is < 10 or >= 5000 ms\n", + escape_time_ms.c_str()); + } + else + { + wait_on_escape_ms = (int)tmp; + } +} + + wchar_t input_common_readch(int timed) { if (! has_lookahead()) { if (timed) { - int count; fd_set fds; - struct timeval tm= - { - 0, - 1000 * WAIT_ON_ESCAPE - } - ; - FD_ZERO(&fds); FD_SET(0, &fds); - count = select(1, &fds, 0, 0, &tm); - - switch (count) + struct timeval tm = {wait_on_escape_ms / 1000, + 1000 * (wait_on_escape_ms % 1000)}; + int count = select(1, &fds, 0, 0, &tm); + if (count <= 0) { - case 0: - return WEOF; - - case -1: - return WEOF; - break; - default: - break; - + return WEOF; } } diff --git a/src/input_common.h b/src/input_common.h index 5ec34b398..72ccc073e 100644 --- a/src/input_common.h +++ b/src/input_common.h @@ -35,6 +35,9 @@ void input_common_init(int (*ih)()); */ void input_common_destroy(); +// Adjust the escape timeout. +void update_wait_on_escape_ms(); + /** Function used by input_readch to read bytes from stdin until enough bytes have been read to convert them to a wchar_t. Conversion is diff --git a/tests/bind.expect b/tests/bind.expect index 6c80b8f11..9bde0689e 100644 --- a/tests/bind.expect +++ b/tests/bind.expect @@ -2,59 +2,124 @@ spawn $fish expect_prompt -# Test switching key bindings to vi mode. -# This should leave the mode in the appropriate state (i.e., insert mode). +# Test switching key bindings to vi mode. This should leave the mode in the +# appropriate state (i.e., insert mode). These initial tests assume the +# default escape timeout of 500ms is in effect. send "set -g fish_key_bindings fish_vi_key_bindings\r" expect_prompt -send -h "echo fail\033" +send "echo fail: default escape timeout" +send "\033" # Delay needed to allow fish to transition to vi "normal" mode. sleep 0.510 -send -h "ddiecho success\r" -expect_prompt -re {\r\nsuccess\r\n} { - puts "vi replace line success" +send "ddi" +send "echo success: default escape timeout\r" +expect_prompt -re {\r\nsuccess: default escape timeout\r\n} { + puts "vi replace line: default escape timeout" } -nounmatched -re {\r\nfail} { - puts stderr "vi replace line fail" + puts stderr "vi replace line fail: default escape timeout" } unmatched { - puts stderr "Couldn't find expected output 'success'" + puts stderr "couldn't find expected output: replace line, default escape timeout" } # Verify that a human can transpose words using \et (which is an emacs default # binding but should be valid while in vi insert mode). -send "echo abc def\033" -# Fish should still be in vi "insert" mode after this delay. +send "echo abc def" +send "\033" +# Fish should still be in vi insert mode after this delay to simulate a slow +# typist. sleep 0.400 send "t\r" expect_prompt -re {\r\ndef abc\r\n} { - puts "vi transpose words success" + puts "vi transpose words: default escape timeout" } unmatched { - puts stderr "vi transpose words fail" + puts stderr "vi transpose words fail: default escape timeout" } # Test replacing a single character. -send -h "echo TEXT\033" +send "echo TEXT" +send "\033" # Delay needed to allow fish to transition to vi "normal" mode. sleep 0.510 -send -h "hhrAi\r" +send "hhrAi\r" expect_prompt -re {\r\nTAXT\r\n} { - puts "vi mode replace success" + puts "vi mode replace: default escape timeout" } -nounmatched -re {\r\nfail} { - puts stderr "vi mode replace fail" + puts stderr "vi mode replace fail: default escape timeout" } unmatched { - puts stderr "Couldn't find expected output 'TAXT'" + puts stderr "couldn't find expected output 'TAXT': default escape timeout" +} + +# Verify that changing the escape timeout has an effect. The vi key bindings +# should still be in effect. +send "set -g fish_escape_delay_ms 100\r" +expect_prompt +send "echo fail: shortened escape timeout" +send "\033" +sleep 0.110 +send "ddi" +send "echo success: shortened escape timeout\r" +expect_prompt -re {\r\nsuccess: shortened escape timeout\r\n} { + puts "vi replace line: shortened escape timeout" +} -nounmatched -re {\r\nfail} { + puts stderr "vi replace line fail: shortened escape timeout" +} unmatched { + puts stderr "couldn't find expected output: replace_line, shortened escape timeout" +} + +# Verify that we don't switch to vi normal mode if we don't wait long enough +# after sending escape. The vi key bindings should still be in effect. +send "echo fail: no normal mode" +send "\033" +sleep 0.050 +send "ddi" +send "inserted\r" +expect_prompt -re {\r\nfail: no normal modediinserted\r\n} { + puts "vi normal mode: shortened escape timeout" +} -nounmatched -re {\r\nfail} { + puts stderr "vi normal mode fail: shortened escape timeout" +} unmatched { + puts stderr "couldn't find expected output: no normal mode" } # Switch back to regular (emacs mode) key bindings. -send -h "set -g fish_key_bindings fish_default_key_bindings\r" +send "set -g fish_key_bindings fish_default_key_bindings\r" expect_prompt -send "echo success\r" -expect_prompt -re {\r\nsuccess\r\n} { - puts "emacs success" + +# Verify the emacs transpose word (\et) behavior using various delays, +# including none, after the escape character. + +# Start by testing with no delay. This should transpose the words. +send "echo abc def" +send "\033t\r" +expect_prompt -re {\r\ndef abc\r\n} { + puts "emacs transpose words: no escape delay" } 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 + puts stderr "emacs transpose words fail: no escape delay" +} + +# Now test with a delay > 0 and < the escape timeout. This should transpose +# the words. +send "set -g fish_escape_delay_ms 100\r" +expect_prompt +send "echo ghi jkl" +send "\033" +sleep 0.050 +send "t\r" +expect_prompt -re {\r\njkl ghi\r\n} { + puts "emacs transpose words: short escape delay" +} unmatched { + puts stderr "emacs transpose words fail: short escape delay" +} + +# Now test with a delay > the escape timeout. The transposition should not +# occur and the "t" should become part of the text that is echoed. +send "echo mno pqr" +send "\033" +sleep 0.110 +send "t\r" +expect_prompt -re {\r\nmno pqrt\r\n} { + puts "emacs transpose words: long escape delay" +} unmatched { + puts stderr "emacs transpose words fail: long escape delay" } diff --git a/tests/bind.expect.out b/tests/bind.expect.out index f82eec235..f4682d2c4 100644 --- a/tests/bind.expect.out +++ b/tests/bind.expect.out @@ -1,4 +1,8 @@ -vi replace line success -vi transpose words success -vi mode replace success -emacs success +vi replace line: default escape timeout +vi transpose words: default escape timeout +vi mode replace: default escape timeout +vi replace line: shortened escape timeout +vi normal mode: shortened escape timeout +emacs transpose words: no escape delay +emacs transpose words: short escape delay +emacs transpose words: long escape delay From 11785c2bf4f443985d809cece26b7e8a92b15c92 Mon Sep 17 00:00:00 2001 From: Kurtis Rader Date: Sun, 17 Jan 2016 19:14:54 -0800 Subject: [PATCH 4/8] make bind unit tests more robust on travis-ci My previous commit failed in the travis-ci environment despite passing on my local computer. This appears to be due to expect timing out looking for the expected input. See if increasing the expect timeout slightly fixes the problem. --- tests/bind.expect | 2 -- tests/interactive.expect.rc | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/bind.expect b/tests/bind.expect index 9bde0689e..155571638 100644 --- a/tests/bind.expect +++ b/tests/bind.expect @@ -76,8 +76,6 @@ send "ddi" send "inserted\r" expect_prompt -re {\r\nfail: no normal modediinserted\r\n} { puts "vi normal mode: shortened escape timeout" -} -nounmatched -re {\r\nfail} { - puts stderr "vi normal mode fail: shortened escape timeout" } unmatched { puts stderr "couldn't find expected output: no normal mode" } diff --git a/tests/interactive.expect.rc b/tests/interactive.expect.rc index c148b5622..1949d3281 100644 --- a/tests/interactive.expect.rc +++ b/tests/interactive.expect.rc @@ -5,7 +5,7 @@ log_file -noappend interactive.tmp.log set fish ../fish -set timeout 4 +set timeout 5 set send_human {.05 .1 5 .02 .2} From 0dac245b580451f8129fee1974bf017ed51230c1 Mon Sep 17 00:00:00 2001 From: Kurtis Rader Date: Mon, 18 Jan 2016 18:58:45 -0800 Subject: [PATCH 5/8] document the escape timeout --- doc_src/bind.txt | 11 +++++++++++ doc_src/index.hdr.in | 15 +++++++++------ 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/doc_src/bind.txt b/doc_src/bind.txt index 8a099eb38..0ac2c32b0 100644 --- a/doc_src/bind.txt +++ b/doc_src/bind.txt @@ -131,3 +131,14 @@ set -g fish_key_bindings fish_vi_key_bindings bind -M insert \cc kill-whole-line force-repaint \endfish Turns on Vi key bindings and rebinds @key{Control,C} to clear the input line. + + +\subsection special-case-escape Special Case: The escape Character + +The escape key (or character) poses a special challenge for fish. Consider Vi mode. On the one hand you want to be able to press the escape key to switch from insert to normal mode. On the other hand you want fish to recognize multi-char sequences that begin with an escape character (which is what the function and arrow-keys send) without having the escape character switch to normal mode. So fish needs to wait a little bit to see if the escape introduces a multi-char sequence but not so long that you're left wondering if fish will ever switch to Vi normal mode after you press escape. + +Even in emacs mode (the default key bindings) you don't want fish to wait forever for more characters when it sees an escape character to determine if the sequence matches a key binding. On the other hand you want fish to wait a little bit of time so that you can use the escape key as a "meta" key. + +The solution is for fish to wait, by default, 500 ms (0.5 seconds) after it sees an escape character for another character that might represent a valid binding. This is the Gnu readline library default escape timeout. It can be configured by setting the `fish_escape_delay_ms` variable to a value between 10 and 5000 ms. It is recommended that this be a universal variable that you set once from an interactive session. + +Note: fish versions up thru 2.2.0 used a default of 10 ms and provided no way to configure it. That effectively made it impossible to use escape as a meta key while in emacs mode. diff --git a/doc_src/index.hdr.in b/doc_src/index.hdr.in index e20c8cf05..b9e3f8fc0 100644 --- a/doc_src/index.hdr.in +++ b/doc_src/index.hdr.in @@ -753,17 +753,20 @@ All arrays are one-dimensional and cannot contain other arrays, although it is p \subsection variables-special Special variables -The user can change the settings of `fish` by changing the values of -certain environment variables. - -- `BROWSER`, the user's preferred web browser. If this variable is set, fish will use the specified browser instead of the system default browser to display the fish documentation. - -- `CDPATH`, an array of directories in which to search for the new directory for the `cd` builtin. By default, the fish configuration defines `CDPATH` to be a universal variable with the values `.` and `~`. +The user can change the settings of `fish` by changing the values of certain variables. - A large number of variable starting with the prefixes `fish_color` and `fish_pager_color.` See Variables for changing highlighting colors for more information. - `fish_greeting`, the greeting message printed on startup. +- `fish_escape_delay_ms` to override the default timeout of 500 ms after + seeing an escape character before giving up on matching a key binding. See + the documentation for the bind builtin. + +- `BROWSER`, the user's preferred web browser. If this variable is set, fish will use the specified browser instead of the system default browser to display the fish documentation. + +- `CDPATH`, an array of directories in which to search for the new directory for the `cd` builtin. By default, the fish configuration defines `CDPATH` to be a universal variable with the values `.` and `~`. + - `LANG`, `LC_ALL`, `LC_COLLATE`, `LC_CTYPE`, `LC_MESSAGES`, `LC_MONETARY`, `LC_NUMERIC` and `LC_TIME` set the language option for the shell and subprograms. See the section Locale variables for more information. - `fish_user_paths`, an array of directories that are prepended to `PATH`. This can be a universal variable. From 4e465ee04cf574fcf355ae5ca953b4baa9d93e0a Mon Sep 17 00:00:00 2001 From: Kurtis Rader Date: Sat, 23 Jan 2016 18:24:54 -0800 Subject: [PATCH 6/8] make travis-ci happy again --- tests/bind.expect | 24 +++++++++++++++++------- tests/bind.expect.out | 1 + 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/tests/bind.expect b/tests/bind.expect index 155571638..f8caa1661 100644 --- a/tests/bind.expect +++ b/tests/bind.expect @@ -8,18 +8,27 @@ expect_prompt send "set -g fish_key_bindings fish_vi_key_bindings\r" expect_prompt + +# This test is only present to make the Travis-CI framework succeed +# consistently. It's not clear why the following tests succeed without this +# test when executed on a local machine but not in the Travis-CI framework. +send "echo success: default escape timeout\r" +expect_prompt -re {\r\nsuccess: default escape timeout\r\n} { + puts "prime vi mode: default escape timeout" +} unmatched { + puts stderr "prime vi mode fail: default escape timeout" +} + send "echo fail: default escape timeout" send "\033" # Delay needed to allow fish to transition to vi "normal" mode. -sleep 0.510 +sleep 0.550 send "ddi" send "echo success: default escape timeout\r" expect_prompt -re {\r\nsuccess: default escape timeout\r\n} { puts "vi replace line: default escape timeout" -} -nounmatched -re {\r\nfail} { - puts stderr "vi replace line fail: default escape timeout" } unmatched { - puts stderr "couldn't find expected output: replace line, default escape timeout" + puts stderr "vi replace line fail: default escape timeout" } # Verify that a human can transpose words using \et (which is an emacs default @@ -40,7 +49,7 @@ expect_prompt -re {\r\ndef abc\r\n} { send "echo TEXT" send "\033" # Delay needed to allow fish to transition to vi "normal" mode. -sleep 0.510 +sleep 0.550 send "hhrAi\r" expect_prompt -re {\r\nTAXT\r\n} { puts "vi mode replace: default escape timeout" @@ -56,7 +65,7 @@ send "set -g fish_escape_delay_ms 100\r" expect_prompt send "echo fail: shortened escape timeout" send "\033" -sleep 0.110 +sleep 0.150 send "ddi" send "echo success: shortened escape timeout\r" expect_prompt -re {\r\nsuccess: shortened escape timeout\r\n} { @@ -100,6 +109,7 @@ expect_prompt -re {\r\ndef abc\r\n} { # the words. send "set -g fish_escape_delay_ms 100\r" expect_prompt + send "echo ghi jkl" send "\033" sleep 0.050 @@ -114,7 +124,7 @@ expect_prompt -re {\r\njkl ghi\r\n} { # occur and the "t" should become part of the text that is echoed. send "echo mno pqr" send "\033" -sleep 0.110 +sleep 0.150 send "t\r" expect_prompt -re {\r\nmno pqrt\r\n} { puts "emacs transpose words: long escape delay" diff --git a/tests/bind.expect.out b/tests/bind.expect.out index f4682d2c4..2bb2dfe0a 100644 --- a/tests/bind.expect.out +++ b/tests/bind.expect.out @@ -1,3 +1,4 @@ +prime vi mode: default escape timeout vi replace line: default escape timeout vi transpose words: default escape timeout vi mode replace: default escape timeout From e88bfbc440a05d647a555bea78195a463d012a89 Mon Sep 17 00:00:00 2001 From: Kurtis Rader Date: Mon, 25 Jan 2016 19:32:42 -0800 Subject: [PATCH 7/8] incorporate suggestion by @oranja --- doc_src/bind.txt | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/doc_src/bind.txt b/doc_src/bind.txt index 0ac2c32b0..77fb607a1 100644 --- a/doc_src/bind.txt +++ b/doc_src/bind.txt @@ -135,10 +135,8 @@ Turns on Vi key bindings and rebinds @key{Control,C} to clear the input line. \subsection special-case-escape Special Case: The escape Character -The escape key (or character) poses a special challenge for fish. Consider Vi mode. On the one hand you want to be able to press the escape key to switch from insert to normal mode. On the other hand you want fish to recognize multi-char sequences that begin with an escape character (which is what the function and arrow-keys send) without having the escape character switch to normal mode. So fish needs to wait a little bit to see if the escape introduces a multi-char sequence but not so long that you're left wondering if fish will ever switch to Vi normal mode after you press escape. +Since the escape key (or character) plays two distinct roles it poses a special challenge for fish. In its first role it stands by itself. In Vi mode, for example, escape is used to switch from insert to normal mode. In its second role escape is used as a "meta" key where it is only the beginning of some longer character sequence. Coming back to the Vi mode example, sometimes fish is expected to realize that the escape key should be regarded as a meta key, meaning that the escape character is part of a multi-char sequence. Function keys (e.g., F1, F2, etc...) and arrow keys are common cases of multi-char sequences that begin with the escape character. Custom bindings can also be defined that begin with an escape character. Obviously, fish is not supposed to exit insert mode when the escape is part of a longer character sequence. -Even in emacs mode (the default key bindings) you don't want fish to wait forever for more characters when it sees an escape character to determine if the sequence matches a key binding. On the other hand you want fish to wait a little bit of time so that you can use the escape key as a "meta" key. +To be able distinguish between these two roles fish has to wait after it sees an escape character. In this waiting period any additional key presses make the escape key behave as a meta key. Otherwise, it remains simply an escape key press. The waiting period is set to 500 milliseconds (0.5 seconds) by default. This is the Gnu readline library default escape timeout. It can be configured by setting the `fish_escape_delay_ms` variable to a value between 10 and 5000 ms. It is recommended that this be a universal variable that you set once from an interactive session. -The solution is for fish to wait, by default, 500 ms (0.5 seconds) after it sees an escape character for another character that might represent a valid binding. This is the Gnu readline library default escape timeout. It can be configured by setting the `fish_escape_delay_ms` variable to a value between 10 and 5000 ms. It is recommended that this be a universal variable that you set once from an interactive session. - -Note: fish versions up thru 2.2.0 used a default of 10 ms and provided no way to configure it. That effectively made it impossible to use escape as a meta key while in emacs mode. +Note: fish versions up thru 2.2.0 used a default of 10 ms and provided no way to configure it. That effectively made it impossible to use escape as a meta key. From 2646d51a0bb2551e1d53d164cea2a6a15be44b6e Mon Sep 17 00:00:00 2001 From: Kurtis Rader Date: Sun, 31 Jan 2016 17:37:51 -0800 Subject: [PATCH 8/8] change default escape timeout This changes the default escape timeout for the default keybindings (emacs mode) to 300ms and the default for vi keybindings to 10ms. I couldn't resist fixing a few nits in the fish_vi_key_bindings.fish file since I was touching it to set the escape timeout. --- doc_src/bind.txt | 4 +- doc_src/index.hdr.in | 4 +- share/functions/fish_vi_key_bindings.fish | 34 ++-- src/input_common.cpp | 13 +- tests/bind.expect | 234 ++++++++++++---------- tests/bind.expect.out | 21 +- 6 files changed, 168 insertions(+), 142 deletions(-) diff --git a/doc_src/bind.txt b/doc_src/bind.txt index 77fb607a1..3485f49bc 100644 --- a/doc_src/bind.txt +++ b/doc_src/bind.txt @@ -135,8 +135,8 @@ Turns on Vi key bindings and rebinds @key{Control,C} to clear the input line. \subsection special-case-escape Special Case: The escape Character -Since the escape key (or character) plays two distinct roles it poses a special challenge for fish. In its first role it stands by itself. In Vi mode, for example, escape is used to switch from insert to normal mode. In its second role escape is used as a "meta" key where it is only the beginning of some longer character sequence. Coming back to the Vi mode example, sometimes fish is expected to realize that the escape key should be regarded as a meta key, meaning that the escape character is part of a multi-char sequence. Function keys (e.g., F1, F2, etc...) and arrow keys are common cases of multi-char sequences that begin with the escape character. Custom bindings can also be defined that begin with an escape character. Obviously, fish is not supposed to exit insert mode when the escape is part of a longer character sequence. +Since the escape key (or character) plays two distinct roles it poses a special challenge for fish. In its first role it stands by itself. In Vi mode, for example, escape is used to switch from insert to normal (aka command) mode. In its second role escape is used as a "meta" key where it is only the beginning of some longer character sequence. Coming back to the Vi mode example, sometimes fish is expected to realize that the escape key should be regarded as a meta key, meaning that the escape character is part of a multi-char sequence. Function keys (e.g., F1, F2, etc...) and arrow keys are common cases of multi-char sequences that begin with the escape character. Custom bindings can also be defined that begin with an escape character. Obviously, fish is not supposed to exit insert mode when the escape is part of a longer character sequence. -To be able distinguish between these two roles fish has to wait after it sees an escape character. In this waiting period any additional key presses make the escape key behave as a meta key. Otherwise, it remains simply an escape key press. The waiting period is set to 500 milliseconds (0.5 seconds) by default. This is the Gnu readline library default escape timeout. It can be configured by setting the `fish_escape_delay_ms` variable to a value between 10 and 5000 ms. It is recommended that this be a universal variable that you set once from an interactive session. +To be able distinguish between these two roles fish has to wait after it sees an escape character. In this waiting period any additional key presses make the escape key behave as a meta key. Otherwise, it remains an isolated escape key press. The waiting period is set to 300 milliseconds (0.3 seconds) in the default key bindings and 10 milliseconds in the vi key bindings. It can be configured by setting the `fish_escape_delay_ms` variable to a value between 10 and 5000 ms. It is recommended that this be a universal variable that you set once from an interactive session. Note: fish versions up thru 2.2.0 used a default of 10 ms and provided no way to configure it. That effectively made it impossible to use escape as a meta key. diff --git a/doc_src/index.hdr.in b/doc_src/index.hdr.in index b9e3f8fc0..5eb92423a 100644 --- a/doc_src/index.hdr.in +++ b/doc_src/index.hdr.in @@ -759,9 +759,7 @@ The user can change the settings of `fish` by changing the values of certain var - `fish_greeting`, the greeting message printed on startup. -- `fish_escape_delay_ms` to override the default timeout of 500 ms after - seeing an escape character before giving up on matching a key binding. See - the documentation for the bind builtin. +- `fish_escape_delay_ms` overrides the default timeout of 300ms (default key bindings) or 10ms (vi key bindings) after seeing an escape character before giving up on matching a key binding. See the documentation for the bind builtin command. This delay facilitates using escape as a meta key. - `BROWSER`, the user's preferred web browser. If this variable is set, fish will use the specified browser instead of the system default browser to display the fish documentation. diff --git a/share/functions/fish_vi_key_bindings.fish b/share/functions/fish_vi_key_bindings.fish index 98f2ca129..a1496b80d 100644 --- a/share/functions/fish_vi_key_bindings.fish +++ b/share/functions/fish_vi_key_bindings.fish @@ -1,29 +1,29 @@ function fish_vi_key_bindings --description 'vi-like key bindings for fish' - bind --erase --all + # The default escape timeout is 300ms. But for users of Vi bindings that can + # be slightly annoying when trying to switch to Vi "normal" mode. Too, + # vi-mode users are unlikely to use escape-as-meta. So set a much shorter + # timeout in this case. + set -q fish_escape_delay_ms; or set -g fish_escape_delay_ms 10 + set -l init_mode insert if set -q argv[1] set init_mode $argv[1] end - # Inherit default key bindings - # Do this first so vi-bindings win over default + # Inherit default key bindings. + # Do this first so vi-bindings win over default. + bind --erase --all fish_default_key_bindings -M insert fish_default_key_bindings -M default - # Add a way to get out of insert mode + # Add a way to switch from insert to normal (command) mode. bind -M insert -m default \cc force-repaint bind -M insert -m default \e backward-char force-repaint - ## - ## command mode - ## - + # + # normal (command) mode + # bind :q exit - - # - # normal (default) mode - # - bind \cd exit bind \cc 'commandline ""' bind h backward-char @@ -128,9 +128,9 @@ function fish_vi_key_bindings --description 'vi-like key bindings for fish' bind -m insert cge backward-kill-word force-repaint bind -m insert cgE backward-kill-bigword force-repaint - bind '~' capitalize-word - bind gu downcase-word - bind gU upcase-word + bind '~' capitalize-word + bind gu downcase-word + bind gU upcase-word bind J end-of-line delete-char bind K 'man (commandline -t) ^/dev/null; or echo -n \a' @@ -172,7 +172,6 @@ function fish_vi_key_bindings --description 'vi-like key bindings for fish' # # Lowercase r, enters replace-one mode # - bind -m replace-one r force-repaint bind -M replace-one -m default '' delete-char self-insert backward-char force-repaint bind -M replace-one -m default \e cancel force-repaint @@ -180,7 +179,6 @@ function fish_vi_key_bindings --description 'vi-like key bindings for fish' # # visual mode # - bind -M visual \e\[C forward-char bind -M visual \e\[D backward-char bind -M visual -k right forward-char diff --git a/src/input_common.cpp b/src/input_common.cpp index 291a37a7b..ebe95fced 100644 --- a/src/input_common.cpp +++ b/src/input_common.cpp @@ -28,15 +28,10 @@ Implementation file for the low level input library #include "env.h" #include "iothread.h" -/** - Time in milliseconds to wait for another byte to be available for - reading after \\x1b is read before assuming that escape key was - pressed, and not an escape sequence. - - This is the value used by the readline library. It can be overridden by - setting the fish_escape_delay_ms variable. -*/ -#define WAIT_ON_ESCAPE_DEFAULT 500 +// Time in milliseconds to wait for another byte to be available for reading +// after \x1b is read before assuming that escape key was pressed, and not an +// escape sequence. +#define WAIT_ON_ESCAPE_DEFAULT 300 static int wait_on_escape_ms = WAIT_ON_ESCAPE_DEFAULT; /** Characters that have been read and returned by the sequence matching code */ diff --git a/tests/bind.expect b/tests/bind.expect index f8caa1661..6b5278ee4 100644 --- a/tests/bind.expect +++ b/tests/bind.expect @@ -2,96 +2,8 @@ spawn $fish expect_prompt -# Test switching key bindings to vi mode. This should leave the mode in the -# appropriate state (i.e., insert mode). These initial tests assume the -# default escape timeout of 500ms is in effect. - -send "set -g fish_key_bindings fish_vi_key_bindings\r" -expect_prompt - -# This test is only present to make the Travis-CI framework succeed -# consistently. It's not clear why the following tests succeed without this -# test when executed on a local machine but not in the Travis-CI framework. -send "echo success: default escape timeout\r" -expect_prompt -re {\r\nsuccess: default escape timeout\r\n} { - puts "prime vi mode: default escape timeout" -} unmatched { - puts stderr "prime vi mode fail: default escape timeout" -} - -send "echo fail: default escape timeout" -send "\033" -# Delay needed to allow fish to transition to vi "normal" mode. -sleep 0.550 -send "ddi" -send "echo success: default escape timeout\r" -expect_prompt -re {\r\nsuccess: default escape timeout\r\n} { - puts "vi replace line: default escape timeout" -} unmatched { - puts stderr "vi replace line fail: default escape timeout" -} - -# Verify that a human can transpose words using \et (which is an emacs default -# binding but should be valid while in vi insert mode). -send "echo abc def" -send "\033" -# Fish should still be in vi insert mode after this delay to simulate a slow -# typist. -sleep 0.400 -send "t\r" -expect_prompt -re {\r\ndef abc\r\n} { - puts "vi transpose words: default escape timeout" -} unmatched { - puts stderr "vi transpose words fail: default escape timeout" -} - -# Test replacing a single character. -send "echo TEXT" -send "\033" -# Delay needed to allow fish to transition to vi "normal" mode. -sleep 0.550 -send "hhrAi\r" -expect_prompt -re {\r\nTAXT\r\n} { - puts "vi mode replace: default escape timeout" -} -nounmatched -re {\r\nfail} { - puts stderr "vi mode replace fail: default escape timeout" -} unmatched { - puts stderr "couldn't find expected output 'TAXT': default escape timeout" -} - -# Verify that changing the escape timeout has an effect. The vi key bindings -# should still be in effect. -send "set -g fish_escape_delay_ms 100\r" -expect_prompt -send "echo fail: shortened escape timeout" -send "\033" -sleep 0.150 -send "ddi" -send "echo success: shortened escape timeout\r" -expect_prompt -re {\r\nsuccess: shortened escape timeout\r\n} { - puts "vi replace line: shortened escape timeout" -} -nounmatched -re {\r\nfail} { - puts stderr "vi replace line fail: shortened escape timeout" -} unmatched { - puts stderr "couldn't find expected output: replace_line, shortened escape timeout" -} - -# Verify that we don't switch to vi normal mode if we don't wait long enough -# after sending escape. The vi key bindings should still be in effect. -send "echo fail: no normal mode" -send "\033" -sleep 0.050 -send "ddi" -send "inserted\r" -expect_prompt -re {\r\nfail: no normal modediinserted\r\n} { - puts "vi normal mode: shortened escape timeout" -} unmatched { - puts stderr "couldn't find expected output: no normal mode" -} - -# Switch back to regular (emacs mode) key bindings. -send "set -g fish_key_bindings fish_default_key_bindings\r" -expect_prompt +# Fish should start in default-mode (i.e., emacs) bindings. The default escape +# timeout is 300ms. # Verify the emacs transpose word (\et) behavior using various delays, # including none, after the escape character. @@ -100,34 +12,154 @@ expect_prompt send "echo abc def" send "\033t\r" expect_prompt -re {\r\ndef abc\r\n} { - puts "emacs transpose words: no escape delay" + puts "emacs transpose words, default timeout: no delay" } unmatched { - puts stderr "emacs transpose words fail: no escape delay" + puts stderr "emacs transpose words fail, default timeout: no delay" } # Now test with a delay > 0 and < the escape timeout. This should transpose # the words. -send "set -g fish_escape_delay_ms 100\r" -expect_prompt - send "echo ghi jkl" send "\033" -sleep 0.050 +sleep 0.200 send "t\r" expect_prompt -re {\r\njkl ghi\r\n} { - puts "emacs transpose words: short escape delay" + puts "emacs transpose words, default timeout: short delay" } unmatched { - puts stderr "emacs transpose words fail: short escape delay" + puts stderr "emacs transpose words fail, default timeout: short delay" } # Now test with a delay > the escape timeout. The transposition should not # occur and the "t" should become part of the text that is echoed. send "echo mno pqr" send "\033" -sleep 0.150 +sleep 0.400 send "t\r" expect_prompt -re {\r\nmno pqrt\r\n} { - puts "emacs transpose words: long escape delay" + puts "emacs transpose words, default timeout: long delay" } unmatched { - puts stderr "emacs transpose words fail: long escape delay" + puts stderr "emacs transpose words fail, default timeout: long delay" +} + +# Test vi key bindings. +# This should leave vi mode in the insert state. +send "set -g fish_key_bindings fish_vi_key_bindings\r" +expect_prompt + +# These vi tests assume the fish_vi_key_bindings default escape timeout of +# 10ms is in effect; not the 300ms timeout for the default-mode. +# +# This test is only present to make the Travis-CI framework succeed +# consistently. It's not clear why the subsequent tests succeed without this +# test when executed on a local machine but not in the Travis-CI framework. +send "echo success: default escape timeout\r" +expect_prompt -re {\r\nsuccess: default escape timeout\r\n} { + puts "prime vi mode, default timeout" +} unmatched { + puts stderr "prime vi mode, default timeout" +} + +send "echo fail: default escape timeout" +send "\033" +# Delay needed to allow fish to transition to vi "normal" mode. +sleep 0.020 +send "ddi" +send "echo success: default escape timeout\r" +expect_prompt -re {\r\nsuccess: default escape timeout\r\n} { + puts "vi replace line, default timeout: long delay" +} unmatched { + puts stderr "vi replace line, default timeout: long delay" +} + +# Verify that a human can transpose words using \et (which is an emacs default +# binding but should be valid while in vi insert or normal mode). +send "echo abc def" +send "\033" +sleep 0.005 +send "t\r" +expect_prompt -re {\r\ndef abc\r\n} { + puts "vi transpose words, default timeout: short delay" +} unmatched { + puts stderr "vi transpose words, default timeout: short delay" +} + +# Test replacing a single character. +send "echo TEXT" +send "\033" +# Delay needed to allow fish to transition to vi "normal" mode. +sleep 0.020 +send "hhrAi\r" +expect_prompt -re {\r\nTAXT\r\n} { + puts "vi mode replace char, default timeout: long delay" +} unmatched { + puts stderr "vi mode replace char, default timeout: long delay" +} + +# Verify that changing the escape timeout has an effect. +send "set -g fish_escape_delay_ms 100\r" +expect_prompt +send "echo fail: lengthened escape timeout" +send "\033" +sleep 0.150 +send "ddi" +send "echo success: lengthened escape timeout\r" +expect_prompt -re {\r\nsuccess: lengthened escape timeout\r\n} { + puts "vi replace line, 100ms timeout: long delay" +} unmatched { + puts stderr "vi replace line, 100ms timeout: long delay" +} + +# Verify that we don't switch to vi normal mode if we don't wait long enough +# after sending escape. +send "echo fail: no normal mode" +send "\033" +sleep 0.050 +send "ddi" +send "inserted\r" +expect_prompt -re {\r\nfail: no normal modediinserted\r\n} { + puts "vi replace line, 100ms timeout: short delay" +} unmatched { + puts stderr "vi replace line, 100ms timeout: short delay" +} + +# Switch back to regular (emacs mode) key bindings. +# The custom escape timeout of 100ms set earlier should still be in effect. +send "set -g fish_key_bindings fish_default_key_bindings\r" +expect_prompt + +# Verify the emacs transpose word (\et) behavior using various delays, +# including none, after the escape character. + +# Start by testing with no delay. This should transpose the words. +send "echo abc def" +send "\033" +send "t\r" +expect_prompt -re {\r\ndef abc\r\n} { + puts "emacs transpose words, 100ms timeout: no delay" +} unmatched { + puts stderr "emacs transpose words fail, 100ms timeout: no delay" +} + + +# Same test as above but with a slight delay less than the escape timeout. +send "echo ghi jkl" +send "\033" +sleep 0.080 +send "t\r" +expect_prompt -re {\r\njkl ghi\r\n} { + puts "emacs transpose words, 100ms timeout: short delay" +} unmatched { + puts stderr "emacs transpose words fail, 100ms timeout: short delay" +} + +# Now test with a delay > the escape timeout. The transposition should not +# occur and the "t" should become part of the text that is echoed. +send "echo mno pqr" +send "\033" +sleep 0.120 +send "t\r" +expect_prompt -re {\r\nmno pqrt\r\n} { + puts "emacs transpose words, 100ms timeout: long delay" +} unmatched { + puts stderr "emacs transpose words fail, 100ms timeout: long delay" } diff --git a/tests/bind.expect.out b/tests/bind.expect.out index 2bb2dfe0a..07902427f 100644 --- a/tests/bind.expect.out +++ b/tests/bind.expect.out @@ -1,9 +1,12 @@ -prime vi mode: default escape timeout -vi replace line: default escape timeout -vi transpose words: default escape timeout -vi mode replace: default escape timeout -vi replace line: shortened escape timeout -vi normal mode: shortened escape timeout -emacs transpose words: no escape delay -emacs transpose words: short escape delay -emacs transpose words: long escape delay +emacs transpose words, default timeout: no delay +emacs transpose words, default timeout: short delay +emacs transpose words, default timeout: long delay +prime vi mode, default timeout +vi replace line, default timeout: long delay +vi transpose words, default timeout: short delay +vi mode replace char, default timeout: long delay +vi replace line, 100ms timeout: long delay +vi replace line, 100ms timeout: short delay +emacs transpose words, 100ms timeout: no delay +emacs transpose words, 100ms timeout: short delay +emacs transpose words, 100ms timeout: long delay