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

vim: Refactor and fix multiline operations #25055

Open
wants to merge 32 commits into
base: main
Choose a base branch
from

Conversation

5brian
Copy link
Contributor

@5brian 5brian commented Feb 18, 2025

Changes:

Matches neovim:

  • Cursor at the start during yank operations on objects (yip, yab etc).

From plugins:

  • Text object operations in visual block and visual line modes maintains original visual mode after completion.
  • Refactors this: Trim all leading and trailing whitespace from inner multiline bracket selection.
    • This leaves a nicely indented line when doing ci{ vi{d etc
    • Checks for empty selection
    • Removed moving cursor to the start in visual bracket operations

This cleans up the previous implementation by providing a simpler check in surrounding_markers, instead of calling a new function in expand_object. No functionality was changed there except for handling the empty selection and removing some cursor adjustments that should not have been there after further testing.

Release Notes:

  • vim: Text object operations in visual block and visual line modes maintains original visual mode after completion.

@cla-bot cla-bot bot added the cla-signed The user has signed the Contributor License Agreement label Feb 18, 2025
@ConradIrwin
Copy link
Member

Thank you! Blazingly fast as usual :D.

Could you please incorporate the original example from the issue as a test

@5brian

This comment was marked as resolved.

@5brian 5brian marked this pull request as ready for review February 18, 2025 03:49
@5brian

This comment was marked as resolved.

@ConradIrwin
Copy link
Member

I'm going to fix the crash for now: #25138, but sounds like we should make bigger changes here?

Happy to wait until you're done with exams

@5brian
Copy link
Contributor Author

5brian commented Feb 19, 2025

Sounds good, thanks! will continue working on these

When yanking inside brackets like yib, cursor should move to the front.
When in visual block, using text objects should stay in visual block after the operation.

@5brian 5brian marked this pull request as draft February 19, 2025 07:18
@5brian

This comment was marked as resolved.

@ConradIrwin
Copy link
Member

@5brian when I do vi( in nvim --clean it acts like your vib screenshot - i.e. in this case it does select all the whitespace. Do you have other plugins that are changing this behavior?

@5brian
Copy link
Contributor Author

5brian commented Feb 20, 2025

Ah, just realized i had mini.nvim affecting the selections. Will redo this

@5brian
Copy link
Contributor Author

5brian commented Feb 20, 2025

@ConradIrwin should be good for a review now, updated the description with the changes

Just confused about a test case i added for having visual line persist after using objects.
Performing shift-v a { from inside a brace should result in the whole function including the declaration being selected, in visual line mode. I added the test with the set and assert exactly like that, when testing locally it works, but the ci test is failing -- i'm probably missing something but i cant think of anything

assert local build ci
image image image

@ConradIrwin
Copy link
Member

The line mode selections are not reflected in the positions of the « »; even though the selection looks like it covers the whole line. If you hit v in that situation you can see where the real selection is, which should match what the test is assuming

@5brian
Copy link
Contributor Author

5brian commented Feb 22, 2025

Thank you

@5brian 5brian marked this pull request as ready for review February 22, 2025 03:02
@ConradIrwin
Copy link
Member

@5brian can we keep using the NeovimBackedtestcontext for either of these tests given the above? Ideally we are exactly matching neovim when it has the features we have

@5brian
Copy link
Contributor Author

5brian commented Feb 22, 2025

@ConradIrwin from my testing against nvim --clean

    cx.simulate_keystrokes("a ]");
    cx.assert_state("hello (in «[parens]ˇ» o)", Mode::VisualBlock);

Switches to visual mode instead of staying in visual block

    cx.simulate_keystrokes("shift-v a {");
    cx.assert_state(
        indoc! {
            "func empty(a string) bool «{
               if a == \"\" {
                  return true
               }
               return false
            }ˇ»"
        },

Switches to visual mode instead of staying in visual line

I have them both staying in their original mode after the object operations, let me know if this is something i should change

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed The user has signed the Contributor License Agreement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants