Vim icon indicating copy to clipboard operation
Vim copied to clipboard

Discussion: `<Tab>` key

Open berknam opened this issue 5 years ago • 5 comments

Describe the bug Currently we can't capture the <Tab> key while in insert mode because that key needs to be handled by vscode so that the proper command is executed and there can be many commands depending on the situation like insertSnippet, jumpToNextSnippetPlaceholder, insertBestCompletion, insertNextSuggestion, editor.emmet.action.expandabbreviation among other commands that might be registered from other extensions.

This creates a long list of issues like the following (there is probably others I can't remember right now):

  1. Can't record macros as only keystrokes because we can't capture <Tab>
  2. Can't record . dot register as keystrokes because we can't capture <Tab>
  3. Can't record DotCommand as keystrokes because we can't capture <Tab>
  4. Can't store macros to disk because right know they are stored as complex RecordedState objects to include all the actions that look for and store document changes like the DocumentContentChangeAction
  5. Can't edit macros by simply copying a series of keys to some register like doing "ayiw on text ATest<Esc>j so that you can call that with @a and result in append 'Test' to the end of current line and move down.
  6. Right now with the change to selections when you enter a snippet with multicursors it enters MulticursorVisualMode and then you need to press c to change the selected text which puts you in MulticursorInsertMode, but if you then press <Tab> to go to the next placeholder, since we don't capture that key, we will change mode to MulticursorVisualMode again but we still have an unfinished DocumentContentChangeAction on the RecordedState that also includes an unfinished ChangeOperator action, so if you then press c to try to delete the selected text and enter MulticursorInsertMode it will actually do nothing, you'll have to press it a second time in order to work.
  7. Right now if we record a macro and replay a macro, or we try to insert last inserted text with <C-a> in a situation where we used <C-r> in insert mode to insert the text from some register it will always save the inserted text and it will always be the same even if we change the contents of that register. This is normal Vim behavior usually we would record the keystrokes and replay them which would result in pressing <C-r> again and then the register name, which would insert the updated register contents. This happens because we can't record only the keystrokes due to not being able to capture and resend the <Tab> key.

Additional context I've opened this issue microsoft/vscode#104832 in vscode to see if it is possible to resend the <Tab> key to be handled normally again after we capture it and also send it again when needed, for example when replaying a macro.

@J-Fields If you can give your input on this and on the opened vscode issue I would really appreciate. Like I mentioned before, I don't really know the reasons behind using the RecordedState for macros instead of just keystrokes, but from looking into it I believe that this <Tab> key situation is the major issue. (maybe that situation with the remapper undoing a remapped inserted key might have created some issues but that should no longer be a problem now)

berknam avatar Aug 17 '20 16:08 berknam

Don't have time at the moment to really dig into this - I'll come back after I get the chance to think more deeply about it, but some initial thoughts:

  1. For perfect emulation, we need https://github.com/microsoft/vscode/issues/104832, but it very likely won't be addressed for a long time, if it ever is
  2. I don't think we need https://github.com/microsoft/vscode/issues/104832 to store lists of keystrokes instead of RecordedStates (if there's something I'm not thinking of, let me know), which would solve most of these issues. It seems to me we can serialize the DocumentContentChangeAction, which would do the right thing except when tabs are expanded to spaces and the action is repeated in a place where the tab should've been expanded to a different number of spaces (which isn't a huge deal). We might even be able to handle most of those cases correctly if we cleverly detect multiple spaces appearing and interpret that as a tab being entered, though that might have unintended consequences.
  3. Regardless, we need to solve the issue of a tab not creating a new DocumentContentChangeAction, only being rolled into an existing one (#2337)

J-Fields avatar Aug 17 '20 16:08 J-Fields

3. Regardless, we need to solve the issue of a tab not creating a new `DocumentContentChangeAction`, only being rolled into an existing one (#2337)

This might be doable when <Tab> is inserting spaces, but it will be harder to do in situations like mentioned on my point 6.:

  1. Right now with the change to selections when you enter a snippet with multicursors it enters MulticursorVisualMode and then you need to press c to change the selected text which puts you in MulticursorInsertMode, but if you then press <Tab> to go to the next placeholder, since we don't capture that key, we will change mode to MulticursorVisualMode again but we still have an unfinished DocumentContentChangeAction on the RecordedState that also includes an unfinished ChangeOperator action, so if you then press c to try to delete the selected text and enter MulticursorInsertMode it will actually do nothing, you'll have to press it a second time in order to work.

Which I'm presuming that it will result in a lot of complaining once the current master is released. To try to fix this I'm thinking of moving the current code from the handleSelectionChangeEvent to a new action (for example SyncCursorsAction) and then sending a special key like <SyncCursors> to the handleKeyEvent from the handleSelectionChangeEvent. That way, this new action would finish any pending DocumentContentChangeAction. The problem is how would this work with macros...

berknam avatar Aug 17 '20 18:08 berknam

Assigning myself to this to remember to come back to it. It's an interesting problem that's definitely gonna require more thought

J-Fields avatar Aug 20 '20 19:08 J-Fields

Another problem to add to this TAB key situation. Now with the remapper overhaul added in version 1.17 when you are writing a piece of code and the autocompletion shows up when the last key pressed was a potential remap and then you press TAB to autocomplete we don't know that the TAB key was pressed, so from the point view of the remapper no key was pressed and when the timeout finishes it inserts that potential remap key.

Example: With the insert mode remap ii -> <Esc>, and with the following code (| is the cursor):

const a = ['a'];
a.|

If you then press 'joi' the last 'i' is a potential remap so it isn't inserted right away, it waits for timeout or another key, but the autocomplete shows join and if you press TAB to autocomplete the TAB key won't be captured by us so we don't know a key was pressed and short after the timeout finishes and we insert the 'i' that was waiting resulting in the following:

const a = ['a'];
a.joini|

I'm not really sure how to fix this yet, haven't had the time to look into it. I'm trying to work out a fix for the snippet situations, one of the options I'm doing is moving all the handleSelectionChange code to an action with the key <SyncCursors> and then I'm trying to see on that action if the selection change was due to a snippet being inserted. This could also fix this situation here if we let the selection change change our internal cursors even when in insert mode, this way that handleSelectionChange would be called which would then call handleKeyEvent('<SyncCursors>'). Then since we got a key (<SyncCursors>) after 'i' the 'i' would be inserted straight away which might work fine or we might end up with ".joiin" instead of ".joini".

berknam avatar Sep 28 '20 16:09 berknam

https://stackoverflow.com/a/45575913/644945

expelledboy avatar Dec 16 '23 09:12 expelledboy