[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Bug-readline] vi-mode: fix relationship of `.' to f/F/t/T
From: |
Richard Todd |
Subject: |
[Bug-readline] vi-mode: fix relationship of `.' to f/F/t/T |
Date: |
Sun, 30 Sep 2018 00:09:57 +0000 |
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
}
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- [Bug-readline] vi-mode: fix relationship of `.' to f/F/t/T,
Richard Todd <=