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 word motions to text input #1256

Merged
merged 4 commits into from Sep 8, 2022
Merged

Conversation

Rodrigodd
Copy link
Contributor

@Rodrigodd Rodrigodd commented Jul 13, 2022

It changes the following:

  • Add common word motions to the text input component, more precisely Ctrl-Left, Ctrl-Right, Ctrl-Backspace/Ctrl-W and Ctrl-Delete. A word motion move/delete until the next/previous start of a word. A word is being defined as a continuous sequence of alphanumeric characters.

I followed the checklist:

  • I added unittests
  • I ran make check without errors
  • I tested the overall application
  • I added an appropriate item to the changelog

@extrawurst
Copy link
Owner

this does not work on macos

@Rodrigodd
Copy link
Contributor Author

I have tested this on Windows and Linux (WSL). Unfortunately I don't have access to a macOS to test this. Can you tell me how exactly this is not working? The only platform dependent code I think might not be working is the ctrl modifier check, but this seems to be equivalent to the others in the codebase.

@extrawurst
Copy link
Owner

can you please record how this should look like?

@Rodrigodd
Copy link
Contributor Author

@extrawurst I recorded it inside visual studio code for the keystrokes overlay. I notice that Ctrl-Delete don't work in vscode integrated terminal, but it does work in Windows Terminal.

gitui.mp4

@extrawurst
Copy link
Owner

extrawurst commented Sep 2, 2022

it breaks on unicode text with multibyte content, try some emojis. it will run into some endless loop.

edit: try this commit message ❤️🤯 adasd asdasd and try jumping words from left to right. please write a unittest for it

@Rodrigodd
Copy link
Contributor Author

@extrawurst the infinite loop is fixed.

}
return Ok(EventState::Consumed);
}
KeyCode::Backspace | KeyCode::Char('w')
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am on the fence about the special case of ctrl+w. I am probably going to remove it for now. its hard to customize this way and I am not aware this being some standard

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I know, Ctrl-W is the standart shortcut for "delete word" in a linux terminal (Ctrl-Delete don't work).
Because gitui is a terminal app, I tought this shortcut would be pertinent.
Vim, for example, has the same shortcut (it is where I learned about it).

@extrawurst
Copy link
Owner

also the changelog entry is still missing

CHANGELOG.md Outdated
@@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
* submodules support ([#1087](https://github.com/extrawurst/gitui/issues/1087))
* customizable `cmdbar_bg` theme color & screen spanning selected line bg [[@gigitsu](https://github.com/gigitsu)] ([#1299](https://github.com/extrawurst/gitui/pull/1299))
* use filewatcher instead of polling updates ([#1](https://github.com/extrawurst/gitui/issues/1))
* word motions to text input ([#1256](https://github.com/extrawurst/gitui/issues/1256))
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see the format of other contributions please - your handle is missing

Copy link
Owner

@extrawurst extrawurst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not much left to do. could you record a nice gif we can put in the changelog of the new functionality?

@Rodrigodd
Copy link
Contributor Author

could you record a nice gif we can put in the changelog of the new functionality?

I don't have a easy recording workflow setup.

@extrawurst extrawurst added this to the v0.22 milestone Sep 8, 2022
@extrawurst extrawurst merged commit 67fa456 into extrawurst:master Sep 8, 2022
@extrawurst
Copy link
Owner

Thanks for implementing this!

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.

None yet

2 participants