Hi all, I believe this is a vi-mode POSIX issue, this time:
Let's take the following line: 'ball' 'animal' 'angle' With the cursor at the front, in vim's compatibility mode, I can issue the following commands (with spaces added only for clarity here): fb dt' fa . ; . ... and I will be left with three sets of quotes: '' '' '' This sequence illustrates two features not present in readline's vi-mode: 1) Since the `fa' command will overwrite _rl_vi_last_search_char, the following `.' command doesn't remember it should delete up to a single-quote. The POSIX section for "[count] ." states: "Repeated commands with associated motion commands shall repeat the motion command as well". The dot should repeat dt' regardless of any intervening t/T/f/F commands. 2) Since the `.' command overwrites _rl_cs_dir to FTO, the next `;' command has forgotten that it is repeating a FFIND command (fa). The POSIX section for "[count] ." states: "If the motion component of the repeated command is f, F, t, or T, the repeated command shall not set the remembered search character for the ; and , commands." I note that, strictly speaking, it only talks about not setting the search character, and doesn't specify the interaction between the t/T/f/F command with subsequent `;'. But, vim-compatibility-mode keeps `.' and `;' fully independent, and I think that is the least surprising interpretation from a user's perspective. nvi disagrees with vim here, though, so maybe it is an extension after all. (I'm using http://pubs.opengroup.org/onlinepubs/9699919799/utilities/vi.html as my reference) I've altered my local copy so that rl_vi_char_search() tracks when it is running as the motion part of a d/c/y command, and stores the search character to a new variable (_rl_vi_last_search_motion_char), so that it can be used as part of redo commands. That takes care of issue (1). Second, I've simplified the handling of _rl_cs_dir (no more need for the _orig_ variant), and made certain not to overwrite it in the case of a redo command. This takes care of issue (2). I tested this with multibyte on and off. I'm honestly not sure how to test the "callback mode" code path, though I did attempt to make the correct changes for that as well. Patch against plain git devel is below, if you are interested. I set RL_STATE_VIMOTION even when we aren't using callback-mode, so that the search code can always tell if it is servicing c/d/y commands. It seems innocuous, but it also felt slightly wrong since vi-mode seems to only use those states to keep track of where it is in the face of select()-style callbacks. But my main goal was to demonstrate the type of changes I think would help without disturbing the existing code much. diff --git a/vi_mode.c b/vi_mode.c index d6fa38e..3b50855 100644 --- a/vi_mode.c +++ b/vi_mode.c @@ -108,11 +108,15 @@ static int vi_insert_buffer_size; static int _rl_vi_last_repeat = 1; static int _rl_vi_last_arg_sign = 1; static int _rl_vi_last_motion; +static int _rl_cs_dir; #if defined (HANDLE_MULTIBYTE) static char _rl_vi_last_search_mbchar[MB_LEN_MAX]; static int _rl_vi_last_search_mblen; +static char _rl_vi_last_search_motion_mbchar[MB_LEN_MAX]; +static int _rl_vi_last_search_motion_mblen; #else static int _rl_vi_last_search_char; +static int _rl_vi_last_search_motion_char; #endif static char _rl_vi_last_replacement[MB_LEN_MAX+1]; /* reserve for trailing NULL */ @@ -1375,6 +1379,7 @@ int rl_vi_delete_to (int count, int key) { int c, r; + RL_SETSTATE (RL_STATE_VIMOTION); _rl_vimvcxt = _rl_mvcxt_alloc (VIM_DELETE, key); _rl_vimvcxt->start = rl_point; @@ -1401,7 +1406,6 @@ rl_vi_delete_to (int count, int key) #if defined (READLINE_CALLBACKS) else if (RL_ISSTATE (RL_STATE_CALLBACK)) { - RL_SETSTATE (RL_STATE_VIMOTION); return (0); } #endif @@ -1464,6 +1468,8 @@ rl_vi_change_to (int count, int key) { int c, r; + RL_SETSTATE (RL_STATE_VIMOTION); + _rl_vimvcxt = _rl_mvcxt_alloc (VIM_CHANGE, key); _rl_vimvcxt->start = rl_point; @@ -1489,7 +1495,6 @@ rl_vi_change_to (int count, int key) #if defined (READLINE_CALLBACKS) else if (RL_ISSTATE (RL_STATE_CALLBACK)) { - RL_SETSTATE (RL_STATE_VIMOTION); return (0); } #endif @@ -1531,6 +1536,8 @@ rl_vi_yank_to (int count, int key) { int c, r; + RL_SETSTATE (RL_STATE_VIMOTION); + _rl_vimvcxt = _rl_mvcxt_alloc (VIM_YANK, key); _rl_vimvcxt->start = rl_point; @@ -1556,7 +1563,6 @@ rl_vi_yank_to (int count, int key) #if defined (READLINE_CALLBACKS) else if (RL_ISSTATE (RL_STATE_CALLBACK)) { - RL_SETSTATE (RL_STATE_VIMOTION); return (0); } #endif @@ -1731,29 +1737,46 @@ rl_vi_first_print (int count, int key) return (rl_vi_back_to_indent (1, key)); } -static int _rl_cs_dir, _rl_cs_orig_dir; - #if defined (READLINE_CALLBACKS) static int _rl_vi_callback_char_search (_rl_callback_generic_arg *data) { int c; + _rl_cs_dir = data->i1; + #if defined (HANDLE_MULTIBYTE) - c = _rl_vi_last_search_mblen = _rl_read_mbchar (_rl_vi_last_search_mbchar, MB_LEN_MAX); + c = _rl_read_mbchar (_rl_vi_last_search_mbchar, MB_LEN_MAX); + if (c <= 0) + { + RL_UNSETSTATE (RL_STATE_CHARSEARCH); + return -1; + } + _rl_vi_last_search_mblen = c; + /* When reading a c, d, or y command, remember the char + separately for redo purposes, since another [tTfF] search + could happen before the user does the '.' redo. */ + if (RL_ISSTATE(RL_STATE_VIMOTION)) + { + memcpy(_rl_vi_last_search_motion_mbchar, + _rl_vi_last_search_mbchar, + MB_LEN_MAX); + _rl_vi_last_search_motion_mblen = c; + } #else RL_SETSTATE(RL_STATE_MOREINPUT); c = rl_read_key (); RL_UNSETSTATE(RL_STATE_MOREINPUT); -#endif - - if (c <= 0) + if (c < 0) { RL_UNSETSTATE (RL_STATE_CHARSEARCH); return -1; } - -#if !defined (HANDLE_MULTIBYTE) _rl_vi_last_search_char = c; + /* When reading a c, d, or y command, remember the char + separately for redo purposes, since another [tTfF] search + could happen before the user does the '.' redo. */ + if (RL_ISSTATE(RL_STATE_VIMOTION)) + _rl_vi_last_search_motion_char = c; #endif _rl_callback_func = 0; @@ -1764,7 +1787,7 @@ _rl_vi_callback_char_search (_rl_callback_generic_arg *data) return (_rl_char_search_internal (data->count, _rl_cs_dir, _rl_vi_last_search_mbchar, _rl_vi_last_search_mblen)); #else return (_rl_char_search_internal (data->count, _rl_cs_dir, _rl_vi_last_search_char)); -#endif +#endif } #endif @@ -1772,56 +1795,73 @@ int rl_vi_char_search (int count, int key) { int c; + int cs_dir; +#if defined (HANDLE_MULTIBYTE) + char *target; + int *tlen; +#else + int *target; +#endif + + /* Initially point the target to the last search. In the case + of _rl_vi_redoing it will be changed later to the last + motion command, EXCEPT when `key' is ; or , */ #if defined (HANDLE_MULTIBYTE) - static char *target; - static int tlen; + target = _rl_vi_last_search_mbchar; + tlen = &_rl_vi_last_search_mblen; #else - static char target; + target = &_rl_vi_last_search_char; #endif if (key == ';' || key == ',') { - if (_rl_cs_orig_dir == 0) + if (_rl_cs_dir == 0) return 1; #if defined (HANDLE_MULTIBYTE) - if (_rl_vi_last_search_mblen == 0) + if (*tlen == 0) return 1; #else - if (_rl_vi_last_search_char == 0) + if (*target == 0) return 1; #endif - _rl_cs_dir = (key == ';') ? _rl_cs_orig_dir : -_rl_cs_orig_dir; + cs_dir = (key == ';') ? _rl_cs_dir : -_rl_cs_dir; } else { switch (key) { case 't': - _rl_cs_orig_dir = _rl_cs_dir = FTO; + cs_dir = FTO; break; case 'T': - _rl_cs_orig_dir = _rl_cs_dir = BTO; + cs_dir = BTO; break; case 'f': - _rl_cs_orig_dir = _rl_cs_dir = FFIND; + cs_dir = FFIND; break; case 'F': - _rl_cs_orig_dir = _rl_cs_dir = BFIND; + cs_dir = BFIND; break; } if (_rl_vi_redoing) { - /* set target and tlen below */ + /* When redoing with '.' use the last motion char */ +#if defined (HANDLE_MULTIBYTE) + target = _rl_vi_last_search_motion_mbchar; + tlen = &_rl_vi_last_search_motion_mblen; +#else + target = &_rl_vi_last_search_motion_char; +#endif } #if defined (READLINE_CALLBACKS) else if (RL_ISSTATE (RL_STATE_CALLBACK)) { _rl_callback_data = _rl_callback_data_alloc (count); - _rl_callback_data->i1 = _rl_cs_dir; + _rl_callback_data->i1 = cs_dir; _rl_callback_data->i2 = key; _rl_callback_func = _rl_vi_callback_char_search; RL_SETSTATE (RL_STATE_CHARSEARCH); @@ -1830,11 +1870,22 @@ rl_vi_char_search (int count, int key) #endif else { + _rl_cs_dir = cs_dir; #if defined (HANDLE_MULTIBYTE) c = _rl_read_mbchar (_rl_vi_last_search_mbchar, MB_LEN_MAX); if (c <= 0) return -1; _rl_vi_last_search_mblen = c; + /* When reading a c, d, or y command, remember the char + separately for redo purposes, since another [tTfF] search + could happen before the user does the '.' redo. */ + if (RL_ISSTATE(RL_STATE_VIMOTION)) + { + memcpy(_rl_vi_last_search_motion_mbchar, + _rl_vi_last_search_mbchar, + MB_LEN_MAX); + _rl_vi_last_search_motion_mblen = c; + } #else RL_SETSTATE(RL_STATE_MOREINPUT); c = rl_read_key (); @@ -1842,21 +1893,19 @@ rl_vi_char_search (int count, int key) if (c < 0) return -1; _rl_vi_last_search_char = c; + /* When reading a c, d, or y command, remember the char + separately for redo purposes, since another [tTfF] search + could happen before the user does the '.' redo. */ + if (RL_ISSTATE(RL_STATE_VIMOTION)) + _rl_vi_last_search_motion_char = c; #endif } } #if defined (HANDLE_MULTIBYTE) - target = _rl_vi_last_search_mbchar; - tlen = _rl_vi_last_search_mblen; -#else - target = _rl_vi_last_search_char; -#endif - -#if defined (HANDLE_MULTIBYTE) - return (_rl_char_search_internal (count, _rl_cs_dir, target, tlen)); + return (_rl_char_search_internal (count, cs_dir, target, *tlen)); #else - return (_rl_char_search_internal (count, _rl_cs_dir, target)); + return (_rl_char_search_internal (count, cs_dir, *target)); #endif } _______________________________________________ Bug-readline mailing list Bug-readline@gnu.org https://lists.gnu.org/mailman/listinfo/bug-readline