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

Add stop_at_first_char option to the move/select/delete to beginning of line actions #25346

Conversation

felixpackard
Copy link

Closes #7863

In order to make the behaviour of similarly named "[x] to beginning of line" actions consistent, I've added the stop_at_first_char option which, if enabled, will treat the first non-whitespace character as the start of the line, and otherwise will treat the first character (whitespace or otherwise) as the start. This option defaults to false, as based on my testing the default behaviour in VS Code, Vim, and Emacs is to use the "true" start of the line as opposed to the first non-whitespace character.

This option essentially overrides the stop_at_soft_wraps option if enabled, as if the movement::indented_line_beginning method is called with a stop_at_soft_wraps value of false, the actual line start will always be returned, rather than the indented line start. This feels somewhat counterintuitive, so if there's a better way I should be handling the case where stop_at_soft_wraps is false and stop_at_first_char is true, I'd be happy to implement it.

I've tried to be thorough with test cases, and I've included the new option in the default keymaps everywhere that the stop_at_soft_wraps option is specified, but it's entirely possible I've missed something, with this being my first contribution to the project. I'm always open to feedback and happy to pair.

Release Notes:

  • Added stop_at_first_char option to the move/select/delete to beginning of line actions

make move/select/delete to beginning of line actions consistent with other beginning of line actions by adding a `stop_at_first_char` option

closes zed-industries#7863

This comment was marked as resolved.

@felixpackard

This comment was marked as resolved.

@cla-bot cla-bot bot added the cla-signed The user has signed the Contributor License Agreement label Feb 21, 2025

This comment was marked as resolved.

@notpeter
Copy link
Member

I think there is signficant overlap between this PR and one I created yesterday (great minds!):

I used stop_at_indent instead of stop_at_first_char (which I think is clearer?)
Looks like you also added this to DeleteToBeginningOfLine which seems logical and added some more tests.

Let me take a look and see if I can integrate the two.

@notpeter notpeter self-assigned this Feb 24, 2025
notpeter added a commit that referenced this pull request Feb 26, 2025
Added test_beginning_of_line_stop_at_indent editor test

Ported from #25346
Follow-up to: #25428

Co-Authored-By: Felix Packard <felix@rigr.gg>
@notpeter
Copy link
Member

Closing in favor of:

Which has everything from here that was not included in my PR.

@notpeter notpeter closed this Feb 26, 2025
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.

Separate Beginning of Line and Beginning of None Whitespace Character
2 participants