vim: Add support for `ap` and `ip` paragraph text objects
This PR adds support for ap/ip text objects in Vim mode and allows users to perform paragraph-based operations.
Cases where compatibility with Neovim's behavior is checked, cases where there are known differences in behavior with Neovim (cases where the landing position is other than the beginning of the line), and cases where the Neovim behavior in the test suite seems strange are separated in the test code so that they can be identified.
Release Notes:
- Added support for
apandipparagraph text objects in Vim mode (#7359).
@noritada thank you for taking a pass at this!
I have a number of suggestions for improvement. If you'd like to work together on getting this over the line, feel free to book time here: https://calendly.com/conradirwin/pairing; otherwise I'll upload comments to Github and we can try to do this async.
Thank you very much! I've booked at Calendly.
@ConradIrwin I have updated the main part. Could you review it?
"Paragraphs" in vim include not only non-blank paragraphs but also "blank paragraphs," which is a little different from what most people think of as paragraphs. I felt that it would be difficult to understand the code without shared recognition of this specification, so I have documented the specification in the doc comment.
Since changes for normal/delete.rs is just for edge cases, I think I should confirm the main part first.
@noritada Thanks for this, I like the new approach!
A few notes:
- In practice it seems like
vim_modestyle paragraphs need completely different code from the existing paragraph code in the editor. I think it would be better to make our own copies of those functionsvim_paragraph_endand put them in the vim crate instead of adding the boolean. (I had hoped they would be more similar, but I was wrong). - I think the
target_visual_modeshould return Mode::VisualLine for these (When used in Visual mode it is made linewise.from the docs). That would explain the test failures for visual mode.
@noritada Please let me know if you're still planning to work on this.
@ConradIrwin I think I have completed all of the necessary changes to the code that you pointed out.
However, after investigating the issue of not being able to properly test v i p and v a p, I found that it was due to the way we test visual line mode, so I am attempting to fix it. Since this fix involves a large change on its own, I have split the changes to PR #8333.
Could you please take a look at this PR to see why tests for the visual line mode is not passing?
@noritada I think even with the fixes to the tests, there are still bugs to be addressed here:
## vip should work on the last line of a file with no trailing newline:
# initial state:
ˇThe quick brown fox jumps over the lazy dog.
# keystrokes:
v i p
# neovim state:
«Tˇ»he quick brown fox jumps over the lazy dog.
# zed state:
ˇThe quick brown fox jumps over the lazy dog.
## vip should select at least the first character of the last line
# initial state:
ˇThe quick brown fox jumps
over the lazy dog.
# keystrokes:
v i p
# neovim state:
«The quick brown fox jumps
ˇ»over the lazy dog.
# zed state:
«The quick brown fox jumps
ˇ» over the lazy dog.
In this case the zed line-mode selection does not extend to the last line of the paragraph.
It is probably easier to create test-cases manually instead of using assert_binding_matches_all. That is a little overkill (though it's kind of nice for fuzz-testing, it's quite hard to develop against as you can't keep track of progress). I would suggest making some more concrete tests like:
#[gpui::test]
fn test_visual_paragrah_object(cx: &mut gpui:::TestAppContext) {
cx.assert_shared_state(indoc! {
"ˇThe quick brown
fox jumps over
the lazy dog
"})
.await;
cx.simulate_shared_keystrokes(["v", "i", "p"]).await;
cx.assert_shared_state(indoc! {
"«ˇThe quick brown
fox jumps over
t»he lazy dog
"})
.await;
}
Happy to work together on this if you want help getting it over the line: https://calendly.com/conradirwin/pairing.
Thank you very much! I understand the policy.
By building and using the app, I have understood the problem and have fixed the end position of the selection. I will also make changes for edge cases and make the code testable.
@noritada One comment for you, but this seems to work well for me manually. Is there anything else you want to do before I merge?
@ConradIrwin Sorry for my absence for a few days.
There was an issue with v i p for only one blank "paragraph" resulting in the selection including a subsequent line and I wanted to fix it and I have fixed this issue.
Thank you for your patience. I am glad you are merging.
Thanks for this @noritada!