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

feat(document_change): extract nvim_input from document change #1347

Closed

Conversation

theol0403
Copy link
Member

Ref: #1266

This removes the need for the custom dot repeat hack and composite escape. For composite escape to work it currently needs https://github.com/max397574/better-escape.nvim.

I'm not sure if this is a viable option, just due to edge cases and bugs. However, worth investigating.

Todo:

  • handle moving cursor (like with auto parens)
  • ignore document changes while in insert mode?
  • consider what happens with pasting a line of code containing a composite escape
  • try to remove need for better-escape (use vscode cursor instead, and ignore large movements)
  • simplify code
  • isolate situations where nvim_input is used

@xiyaowong
Copy link
Collaborator

xiyaowong commented May 18, 2024

Will close this PR. It's a good idea, but it won't work:

  1. Performance issues. Any modifications from vscode such as paste, formatting, revert, etc., are split into inputs, undoubtedly causing significant performance issues.
  2. Edge cases:
    • What happens if a user pastes text containing a map key sequence in insert mode, how to handle it? (Very difficult)
    • What if the user has other input during insertion?
    • Dotrepeat no longer works correctly, undo stacks in nvim and vscode may be corrupted.
    • Multiple cursor editing in vscode.
    • Modifications from external sources, multiple documents being modified simultaneously, modified buffers not being the current buffer.
    • etc.

Furthermore, the issues that this PR attempts to address are currently quite minor.

@xiyaowong xiyaowong closed this May 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants