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
Allow ignoring soft wraps when moving to line ends #11153
Allow ignoring soft wraps when moving to line ends #11153
Conversation
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'. |
@cla-bot check |
The cla-bot has been summoned, and re-checked this pull request! |
Looks like this broke some assertions in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks nice, let's add a small test, similar to what test_beginning_end_of_line
has (or just another few lines straight there)?
@SomeoneToIgnore I believe my PR is ready for re-review now. Thanks for taking a look! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, the changes look complete now — let's fix the tests and have it merged.
This allows configuring whether moving to the beginning or end of a line (for example, with the Home/End) keys should stop at soft wraps or not. By default, this is false -- i.e., movement to the beginning or end of a line will move to the logical position, rather than the position dictated by the nearest soft wrap.
Respect the `stop_at_soft_wraps` field for both `Editor::move_to_beginning_of_line` and `Editor::move_to_end_of_line`. This changes the default behavior: previously, this would set the cursor at the nearest soft wrap location in the respective direction. Now, it will set the cursor properly at the logical beginning/end by default.
Previously, this setting would default to `false` when navigating to line ends. Now, it will default to true.
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. |
ef12b2a
to
125b6cc
Compare
@SomeoneToIgnore tests have been updated |
Oh, we do not run tests on Ubuntu yet and I've just stumbled onto similar issues yesterday. Thank you for altering those nonetheless, I think it's fine to use mac values for now and fix the Linux tests later. |
There seems to be another failure, I'll look into this and fix it today. |
So, checked 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. 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)
);
}); |
@SomeoneToIgnore thanks so much for patching this up and getting it merged! |
Release Notes:
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 theeditor::MoveToBeginningOfLine
andeditor::MoveToEndOfLine
actions. Whentrue
, 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.