Allow ignoring soft wraps when moving to line ends

[tverghis]

Release Notes:

• Fixed #10888

This patch addresses behavior of Editor::move_to_{beginning|end}_of_line. It adds a setting, stop_at_soft_wraps when defining a keymap for the editor::MoveToBeginningOfLine and editor::MoveToEndOfLine actions. When true, it causes movement to the either end of the line (via, for example Home or End), to go to the logical end, as opposed to the nearest soft wrap point in the respective direction.

[cla-bot[bot]]

We require contributors to sign our Contributor License Agreement, and we don't have @tverghis on file. You can sign our CLA at https://zed.dev/cla. Once you've signed, post a comment here that says '@cla-bot check'.

[tverghis]

@cla-bot check

[cla-bot[bot]]

The cla-bot has been summoned, and re-checked this pull request!

[tverghis]

Looks like this broke some assertions in editor_tests::test_beginning_end_of_line. I'll try to fix that, and add some additional tests for when stop_at_soft_wraps is true (old behavior).

[tverghis]

@SomeoneToIgnore I believe my PR is ready for re-review now. Thanks for taking a look!

[tverghis]

Sorry for the delay on this.

Unfortunately, it seems that some tests have different results based on whether it runs on CI or on my machine. The tests that are affected by my changes pass on my machine. FWIW, I am on Ubuntu.

failures:
    display_map::block_map::tests::test_blocks_on_wrapped_lines
    display_map::tests::test_chunks_with_soft_wrapping
    editor_tests::test_add_selection_above_below
    editor_tests::test_beginning_end_of_line
    editor_tests::test_move_cursor_multibyte
    editor_tests::test_prev_next_word_bounds_with_soft_wrap
    hover_links::tests::test_hover_type_links

test result: FAILED. 183 passed; 7 failed; 0 ignored; 0 measured; 0 filtered out; finished in 149.87s

It appears that many of the failures likely have something to do with wrapping. Is text wrapping dependent on the machine or OS it runs on?

Anyway, to fix the problem, I will update the expected values to try and make CI happy.

[tverghis]

@SomeoneToIgnore tests have been updated

[SomeoneToIgnore]

Oh, we do not run tests on Ubuntu yet and I've just stumbled onto similar issues yesterday. Interesting to see how few tests are left to fix, and sorry for the bad experience on your side — our Linux support is still in the experimental phase.

Thank you for altering those nonetheless, I think it's fine to use mac values for now and fix the Linux tests later.

[SomeoneToIgnore]

There seems to be another failure, I'll look into this and fix it today. Hopefully you've ticked the "allow edits from maintainers" in your PR, or you'll have to apply a patch from me 🙂

[SomeoneToIgnore]

So, checked map.max_point() in the fn line_end and effectively failed the test on CI, and it returns DisplayPoint(2, 5) whilst on your system, that was DisplayPoint(2, 6) presumably.

Given the particular set of tests failing, I wonder if that's an off-by-one or other bug in the way we treat display width somewhere in the depth of Linux platform code. We initialize the tests with the same screen width on both platforms, so it's not the test infra code. I will try to look at it, as interested in fixing Linux tests soon, but no promises yet.

The end diff is just

diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs
index de2161b91..17f2a3c36 100644
--- a/crates/editor/src/editor_tests.rs
+++ b/crates/editor/src/editor_tests.rs
@@ -1288,14 +1288,14 @@ fn test_beginning_end_of_line_ignore_soft_wrap(cx: &mut TestAppContext) {
         // next display line.
         view.move_to_end_of_line(&move_to_end, cx);
         assert_eq!(
-            vec![DisplayPoint::new(2, 6)..DisplayPoint::new(2, 6),],
+            vec![DisplayPoint::new(2, 5)..DisplayPoint::new(2, 5),],
             view.selections.display_ranges(cx)
         );
 
         // Moving to the end of the line again should be a no-op.
         view.move_to_end_of_line(&move_to_end, cx);
         assert_eq!(
-            vec![DisplayPoint::new(2, 6)..DisplayPoint::new(2, 6),],
+            vec![DisplayPoint::new(2, 5)..DisplayPoint::new(2, 5),],
             view.selections.display_ranges(cx)
         );
     });

[tverghis]

@SomeoneToIgnore thanks so much for patching this up and getting it merged!