Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Possibly incorrect transitions between modes #12115

Closed
pierreganty opened this issue Mar 7, 2023 · 20 comments
Closed

Possibly incorrect transitions between modes #12115

pierreganty opened this issue Mar 7, 2023 · 20 comments
Labels

Comments

@pierreganty
Copy link
Contributor

pierreganty commented Mar 7, 2023

Steps to reproduce

Sequence 1

  1. i
  2. <C-o> (insert)
  3. <C-v> (insert) Visual Block

Sequence 2

  1. i
  2. <C-o> (insert)
  3. <C-v> (insert) Visual Block
  4. <C-g> (insert) Select Block
  5. <C-o> (insert) Visual Block
  6. c Insert
  7. <C-o> (insert)
  8. <C-v> (insert) Select Block

Unless I am making a mistake I would expect that sequence 2 also ends up in (insert) Visual Block like Sequence 1 since we visit Insert mode again at step 6.

Expected behaviour

Sequence 1 and 2 should end up in the same mode.

Version of Vim

9.0.1388

Environment

macOS version 13.2 - arm64
Terminal.app

Logs and stack traces

No response

@pierreganty pierreganty added the bug label Mar 7, 2023
@brammool
Copy link
Contributor

brammool commented Mar 7, 2023 via email

@pierreganty
Copy link
Contributor Author

pierreganty commented Mar 7, 2023

This issue is the result of trying to automatically compute the state machine underlying Vim (modes as states, keystrokes as transitions as in this example). My two cents are that sequence 2 should end up in the same state sequence 1 does. For otherwise, the state machine that I compute has more states. Moreover I doubt a user would know/expect the behavior of sequence 2.
On the other hand, I completely understand that the effort involved in changing the behavior of sequence 2 is probably not worth it. The decision to close the issue with a change or not belongs to you.

@pierreganty
Copy link
Contributor Author

pierreganty commented Mar 8, 2023

More concretely, the state machine that is automatically inferred below has two Insert states (the large s1 and s108) as a consequence of the above reported issue. My two cents are that state s1 and s108 should be a unique state.

nvim-Lstar-100walks-15len xdot

In the long run such automatically inferred state machine could serve as documentation, as a way to generate tests, or as a formal specification to check against between changes to the code that should not alter the state machine.

@vimpostor
Copy link
Contributor

Sorry for being off topic, but I have to ask out of curiosity: How did you generate this monster of mode state machine? It's quite something :D

@pierreganty
Copy link
Contributor Author

I used some finite state machine learning algorithm. Basically the algorithm inputs tens of thousand of sequences of key strokes into vim and systematically computes a state machine from the modes returned by vim after each such sequence. If you are lucky in your choice of keystrokes and the configuration of vim then you get a state machine as in the picture. More often though, the state machine can grow undoubtedly and you realize this is because some transition between modes in vim depends on the content of the buffer, the jump list, etc. So you have to set up some options correctly and set up some key mapping (e.g. all inputs in insert mode go to <Nop>, map <C-o> to to prevent using the jumplist).

@brammool
Copy link
Contributor

Without looking at the details, the nested Insert mode is a different state, since it behaves different when leaving Insert mode.
Anyway, I still think there is not an actual issue in Vim. As with my earlier response, I don't think we should change anything.

@pierreganty
Copy link
Contributor Author

@brammool as I wrote above "The decision to close the issue with a change or not belongs to you." I am in no position to decide what's the course of action. I am fine with no changes, hence the reported behavior is deemed correct.

@brammool
Copy link
Contributor

OK, let's close it then.

@brammool brammool closed this as not planned Won't fix, can't repro, duplicate, stale Mar 25, 2023
@Madeleine12
Copy link

Madeleine12 commented Jul 4, 2023

Hi there,

I revisited the issue with Pierre and we have found a "simpler" sequence that does not fit the explanations formulated previously.

  1. <C-v> Visual Block
  2. <C-g> Select Block
  3. <C-o> Visual Block
  4. c Insert
  5. <C-o> (insert)
  6. <C-v> (insert) Select Block

Previously (March 7) it was argued that
"CTRL-O in Select mode goes to Visual mode for one command. When getting to
step 6. that one command has been done, thus Select mode is used again."
However this time the Select Block is nested with the insert mode for one command
"(insert)" and the nesting seems quite strange. Indeed, it is worth pointing
that switch-to-visual-for-the-duration-of-1-command (step 3) finishes before
the switch-to-insert-for-the-duration-of-1-command (step 6).

Using an XML-like notation the nesting looks like <select> <visual><insert>
</visual>. It seems unexpected we don't have a closing tags for insert mode
(i.e. </insert>) before </visual>.

@pierreganty
Copy link
Contributor Author

@brammool we have an update on this issue, but it is not clear whether you have seen it or not.

@pierreganty
Copy link
Contributor Author

@chrisbra given that you helped with issue #12684 you might be interested into that one which is a bit more intricate.

@chrisbra
Copy link
Member

chrisbra commented Jul 21, 2023 via email

@chrisbra
Copy link
Member

chrisbra commented Jul 23, 2023

So I checked it. I am not sure what problem you are facing here.

So going through the above steps, this is what happens:

  1. <C-v> Visual Block -- We are in visual mode
  2. <C-g> Select Block -- Switching to Select Mode
  3. <C-o> Visual Block -- Switching to Visual Block Mode for 1 command (remembe we need to go to Select Mode)
  4. c Insert -- Go to Insert Mode
  5. <C-o> (insert) -- Go to normal Mode
  6. <C-v> (insert) Select Block -- Start Block Mode, but since we remembered to go back to Select mode, this is what happens.

I am not sure the current behaviour is wrong. I guess it can be argued either way, not sure it should be changed.

@pierreganty
Copy link
Contributor Author

Thank you, @chrisbra. I would have expected the "for-1-command" mode changes to be "stacked" (i.e. in LIFO order) hence that the insert mode for-1-command (step 5) would be over before the visual mode for-1-command (step 3).

This is a strictly personal view because I think the state machine underlying the modes changes (see my previous posts) should have as few states as possible because the more they have, the heavier the mental load for the user. Arguing from the other side, I think no vim user has the entire state machine "loaded" in their brain (and would therefore be knowledgeable with the sequences reported above). Instead, based on personal experience, I think the user knows the most commonly used parts of the state machine. If they end up in an unfamiliar state (like with the above sequence) it is easy for them to get back to a "comfort mode" (e.g. a couple of <Esc>).

From a maintainer standpoint, I would argue that the fewer states the state machine has, the better. However, I also understand that the current code has "nothing wrong" as you said, and so I won't argue something has to be fixed. You guys (=vim maintainers) have the last word, and a "won't fix" is a reasonable way out.

@chrisbra
Copy link
Member

I don't know. It is a bit strange. Perhaps one should argue, that after entering insert command, resetting Select mode should not happen. That would be a minimal change:

diff --git a/src/normal.c b/src/normal.c
index 4004d4204..43de5ea47 100644
--- a/src/normal.c
+++ b/src/normal.c
@@ -1047,9 +1047,6 @@ normal_end:
            (void)edit(restart_edit, FALSE, 1L);
     }

-    if (restart_VIsual_select == 2)
-       restart_VIsual_select = 1;
-
     // Save count before an operator for next time.
     opcount = ca.opcount;
 }

I am not sure anybody would notice, Select Mode seems a seldomly used mode even more in combination with Ctrl-O. Not sure if this would break any test.

I also checked when the above two lines initially appeared and it only appeared with Vim 7.0 (which was a quite large patch obviously).

@pierreganty
Copy link
Contributor Author

pierreganty commented Jul 24, 2023

With your patch we have a test failing:

" CTRL-O in select mode switches to visual mode for one command
call setline(1, 'abcdef')
exe "normal! gggh\<C-O>3lm"
call assert_equal('mef', getline(1))

Without the patch the last m in the test changes the visually selected abcd to m.
With the patch abcd stays visually selected.

@pierreganty
Copy link
Contributor Author

pierreganty commented Jul 26, 2023

For the record, I am pasting here the latest finite state machine we automatically generated.
Focusing on the "large" nodes only (thus ignoring the small ones) it has:

  • 1 Normal node (s0)
  • 2 Insert nodes (s2 and s92)
  • 2 Visual nodes (s1 and s45)
  • 2 Select nodes (s9 and s62)

The fact it has two nodes for Insert, two nodes for Visual and two nodes for Select is the consequence of the behaviors discussed in this thread. (See #12115 (comment) to get the gist).
For instance, the sequence of keys <C-o> , <C-v> distinguishes s2 and s92: s2 ends up in a sub-mode of Visual, s92 ends up in a sub-mode of Select.

nvim_KV_300_10

@pierreganty
Copy link
Contributor Author

pierreganty commented Jul 28, 2023

The following one liner seems to do the trick, tests have passed and the new state machine (see below) is smaller.

--- a/src/edit.c
+++ b/src/edit.c
@@ -3854,6 +3854,7 @@ ins_insert(int replaceState)
     static void
 ins_ctrl_o(void)
 {
+    restart_VIsual_select = 0;
     if (State & VREPLACE_FLAG)
        restart_edit = 'V';
     else if (State & REPLACE_FLAG)

Would that be an acceptable fix?

nvim_KV_300_10

@chrisbra
Copy link
Member

can you please send this in as a PR?

@pierreganty
Copy link
Contributor Author

Thank you. I will submit a PR late August, early September at the latest.

pierreganty added a commit to pierreganty/vim that referenced this issue Sep 2, 2023
chrisbra added a commit that referenced this issue Sep 2, 2023
Problem:  i_CTRL-O does not reset Select Mode
Solution: Reset select mode on CTRL-O in insert mode

closes: #13001
closes: #12115

Signed-off-by: Christian Brabandt <cb@256bit.org>
Co-authored-by: Christian Brabandt <cb@256bit.org>
zeertzjq added a commit to zeertzjq/neovim that referenced this issue Sep 3, 2023
Problem:  i_CTRL-O does not reset Select Mode
Solution: Reset select mode on CTRL-O in insert mode

closes: vim/vim#13001
closes: vim/vim#12115

vim/vim@d69aecf

Co-authored-by: pierreganty <pierreganty@gmail.com>
Co-authored-by: Christian Brabandt <cb@256bit.org>
zeertzjq added a commit to neovim/neovim that referenced this issue Sep 3, 2023
Problem:  i_CTRL-O does not reset Select Mode
Solution: Reset select mode on CTRL-O in insert mode

closes: vim/vim#13001
closes: vim/vim#12115

vim/vim@d69aecf

Co-authored-by: pierreganty <pierreganty@gmail.com>
Co-authored-by: Christian Brabandt <cb@256bit.org>
chrisbra added a commit to chrisbra/vim that referenced this issue Sep 22, 2023
Problem:  i_CTRL-O does not reset Select Mode
Solution: Reset select mode on CTRL-O in insert mode

closes: vim#13001
closes: vim#12115

Signed-off-by: Christian Brabandt <cb@256bit.org>
Co-authored-by: Christian Brabandt <cb@256bit.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants