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

adds movement for start of word, regardless of stop characters #6445

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

rwese
Copy link

@rwese rwese commented Nov 29, 2024

regarding #195 #1954 (comment)

Sample test/with-characters that cause stopps
                    -(cursor)
       -(after hitting MoveBackwardWordStart)

otherwise would need 2 MoveBackwardWord actions.

Sample test/with-characters that cause stopps
                    -(cursor)
       -(after hitting MoveBackwardWordStart)
@rwese
Copy link
Author

rwese commented Nov 29, 2024

it's missing documentation, I just wanted to get some feedback if this is the proper way to implement the new movement.

@bew
Copy link
Contributor

bew commented Nov 29, 2024

About naming, I think MoveBackwardWordStart & MoveBackwardWord are not different enough (I thought they were the same at the beginning).
Maybe rename to MoveBackwardWORD (but that breaks camelcase-ing..), or MoveBackwardBigWord ?

Also, instead of basically re-implementing move_backward_one_word with slightly different behavior, I think it would be better to refactor the function if possible, for example to make it accept a predicate function for the separator to stop at 🤔

@rwese
Copy link
Author

rwese commented Nov 29, 2024

Naming is always hard, I thought it would fit well with "moveForwardEnd" > "BackwardStart".

In terms of code, I took the easy way, not wanting to have to deal with breaking changes and refactoring. But I can give it a refactor, need to look into the test-cases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants