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

Mouse selection support #509

Merged
merged 29 commits into from
Jul 30, 2021
Merged

Mouse selection support #509

merged 29 commits into from
Jul 30, 2021

Conversation

dsseng
Copy link
Contributor

@dsseng dsseng commented Jul 26, 2021

TODO:

  • Multiple selections created by mouse using modifier keys
  • Double-clicking to select words (will probably need to add extra vars to state for counting)
  • Select lines by dragging on the left side
  • Automatically switch view if clicked out of the current view
  • Pick items from menus using mouse (may be out of scope)

Signed-off-by: Dmitry Sharshakov <d3dx12.xx@gmail.com>
Signed-off-by: Dmitry Sharshakov <d3dx12.xx@gmail.com>
@archseer
Copy link
Member

I think you can skip the double-clicking, it doesn't seem to be present on kakoune at least, not sure about vim.

Signed-off-by: Dmitry Sharshakov <d3dx12.xx@gmail.com>
Signed-off-by: Dmitry Sharshakov <d3dx12.xx@gmail.com>
@cessen
Copy link
Contributor

cessen commented Jul 27, 2021

Really looking forward to this!

I agree that double-click is probably not needed. If I want to select a bunch of words with the mouse, I'll probably just add cursors at each word with modifier-click, and then use a keyboard command to expand to word boundaries. That way it's "click, click, click, hotkey" rather than "click-click, click-click, click-click".

Similarly for selecting a range of lines: I'd probably just drag the selection to encompass the lines I want, and then hit X to expand to line boundaries. That also frees up clicking in the gutter for other things (not entirely sure what those would be, but maybe things related to debugging or version control).

helix-term/src/ui/editor.rs Outdated Show resolved Hide resolved
helix-term/src/ui/editor.rs Outdated Show resolved Hide resolved
helix-view/src/view.rs Outdated Show resolved Hide resolved
Signed-off-by: Dmitry Sharshakov <d3dx12.xx@gmail.com>
@dsseng
Copy link
Contributor Author

dsseng commented Jul 27, 2021

Similarly for selecting a range of lines: I'd probably just drag the selection to encompass the lines I want, and then hit X to expand to line boundaries. That also frees up clicking in the gutter for other things (not entirely sure what those would be, but maybe things related to debugging or version control).

You're right. If anyone wished that feature, right click can be used, but IMO that can be fully dropped really.

Signed-off-by: Dmitry Sharshakov <d3dx12.xx@gmail.com>
Signed-off-by: Dmitry Sharshakov <d3dx12.xx@gmail.com>
Signed-off-by: Dmitry Sharshakov <d3dx12.xx@gmail.com>
@dsseng
Copy link
Contributor Author

dsseng commented Jul 27, 2021

I've split out verify_screen_coords, but looks like there's no way to switch views from event handler. Looking for advice on how to do that the best.

@archseer
Copy link
Member

switch views from event handler

You can do this by setting cx.editor.tree.focus = view.id.

helix-view/src/view.rs Outdated Show resolved Hide resolved
Signed-off-by: Dmitry Sharshakov <d3dx12.xx@gmail.com>
Copy link
Contributor

@pickfire pickfire left a comment

Choose a reason for hiding this comment

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

For an initial implementation I think we can merge this step by step rather than all at once. Just left click is good enough as the first step. I think we can have them as separate PR?

@dsseng
Copy link
Contributor Author

dsseng commented Jul 28, 2021

Currently there's some work on view switching also, but that can be sent in another PR if it succeeds.

@dsseng dsseng requested a review from archseer July 28, 2021 10:45
@dsseng dsseng marked this pull request as ready for review July 28, 2021 10:46
@dsseng
Copy link
Contributor Author

dsseng commented Jul 28, 2021

View switching is done locally. Should I push it here or do that later?

@archseer
Copy link
Member

Yeah include it, I'd like to wait for at least @cessen's review too

Signed-off-by: Dmitry Sharshakov <d3dx12.xx@gmail.com>
@dsseng
Copy link
Contributor Author

dsseng commented Jul 28, 2021

Now two TODOs are left for another PR:

  • Multiple selections created by mouse using modifier keys
  • Pick items from menus using mouse (may be out of scope)

@archseer
Copy link
Member

archseer commented Jul 28, 2021

  • Multiple selections created by mouse using modifier keys

This shouldn't be too hard. If shift is pressed, instead of making a new selection clone the old one then use selection.push to add a new range. It will automatically become the new primary one, so the dragging code needs no changes.

I'd also like to see this behavior be configurable, if mouse = false in the config then the events should just not get enabled (I personally don't use the mouse and will disable it to reduce the amount of incoming crossterm events)

The menu one isn't necessary and I think it's better to skip for term.

helix-term/src/ui/editor.rs Outdated Show resolved Hide resolved
helix-term/src/ui/editor.rs Outdated Show resolved Hide resolved
Signed-off-by: Dmitry Sharshakov <d3dx12.xx@gmail.com>
@dsseng
Copy link
Contributor Author

dsseng commented Jul 28, 2021

Maybe it would be nice to have a runtime command to change mouse behavior?

helix-view/src/view.rs Outdated Show resolved Hide resolved
helix-view/src/view.rs Outdated Show resolved Hide resolved
helix-view/src/view.rs Outdated Show resolved Hide resolved
@Omnikar
Copy link
Contributor

Omnikar commented Jul 29, 2021

Is it intended for drag-selection to create multiple cursors, or is that a bug?

@pickfire
Copy link
Contributor

Is it intended for drag-selection to create multiple cursors?

Not by default but probably possible with some combinations like ctrl. Like ctrl + drag a few stuff.

@dsseng
Copy link
Contributor Author

dsseng commented Jul 29, 2021

Alt+Click makes a new cursor. Drag is a bit broken atm, see #509 (comment)

Copy link
Member

@archseer archseer left a comment

Choose a reason for hiding this comment

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

Looking better now, a couple more changes

helix-term/src/ui/editor.rs Outdated Show resolved Hide resolved
helix-term/src/ui/editor.rs Outdated Show resolved Hide resolved
helix-term/src/ui/editor.rs Outdated Show resolved Hide resolved
Signed-off-by: Dmitry Sharshakov <d3dx12.xx@gmail.com>
Signed-off-by: Dmitry Sharshakov <d3dx12.xx@gmail.com>
Signed-off-by: Dmitry Sharshakov <d3dx12.xx@gmail.com>
Signed-off-by: Dmitry Sharshakov <d3dx12.xx@gmail.com>
Signed-off-by: Dmitry Sharshakov <d3dx12.xx@gmail.com>
Signed-off-by: Dmitry Sharshakov <d3dx12.xx@gmail.com>
Signed-off-by: Dmitry Sharshakov <d3dx12.xx@gmail.com>
helix-view/src/view.rs Outdated Show resolved Hide resolved
Signed-off-by: Dmitry Sharshakov <d3dx12.xx@gmail.com>
Signed-off-by: Dmitry Sharshakov <d3dx12.xx@gmail.com>
@dsseng dsseng requested review from cessen and archseer July 29, 2021 13:58
Copy link
Contributor

@cessen cessen left a comment

Choose a reason for hiding this comment

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

Looks good to me! I also tested locally, and everything seems to work great.

(I think we could use some more thorough tests, including e.g. CJK characters, graphemes, etc. but I can make a PR for that later myself. The basics are there.)

Note: the lint failures in the recent CI run are due to #525, not this PR.

@cessen
Copy link
Contributor

cessen commented Jul 30, 2021

@sh7dm
If you rebase on current master now, the lints should pass.

@archseer archseer merged commit 8361de4 into helix-editor:master Jul 30, 2021
@archseer
Copy link
Member

Great work! I followed up with some refactoring (6bb744a, 62eb8c6)

@dsseng dsseng deleted the mouse-support branch July 30, 2021 07:54
@dsseng
Copy link
Contributor Author

dsseng commented Jul 30, 2021

Huge thanks to all the reviewers for the great help!

@cessen
Copy link
Contributor

cessen commented Jul 30, 2021

Great work, @sh7dm! Thanks for taking the time and effort for 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.

6 participants