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 click should extend selections in select mode #5425

Closed
xJonathanLEI opened this issue Jan 6, 2023 · 6 comments · Fixed by #5436
Closed

Mouse click should extend selections in select mode #5425

xJonathanLEI opened this issue Jan 6, 2023 · 6 comments · Fixed by #5436
Labels
A-helix-term Area: Helix term improvements C-enhancement Category: Improvements

Comments

@xJonathanLEI
Copy link
Contributor

Currently a mouse click on buffer always sets the selection to be the single char being clicked. IMO it would be a much better UX if the selection gets extended instead when the mode is set to select mode.

@xJonathanLEI xJonathanLEI added the C-enhancement Category: Improvements label Jan 6, 2023
@archseer
Copy link
Member

archseer commented Jan 6, 2023

Hmm good point. There is already shift click though, that should extend an existing selection too?

@xJonathanLEI
Copy link
Contributor Author

I failed to find the shift click thing you're referring to. Maybe you meant alt click? This seems to be the code for that:

if modifiers == KeyModifiers::ALT {
let selection = doc.selection(view_id).clone();
doc.set_selection(view_id, selection.push(Range::point(pos)));
} else {
doc.set_selection(view_id, Selection::point(pos));
}

From the code it seems to add the new point clicked as a new selection? In that case I think it does a different thing than I was proposing. What I suggested was for the current selection(s) to set one of its ends to the point clicked.

Basically what I wish I could do is:

  • click the start point
  • enter select mode (v)
  • scroll, scroll, scroll
  • click the end point

then I have the desired range selected. Right now at the last step the whole selection collapses into the point clicked instead.

IMO this function makes sense and is likely expected. After all, select mode is exactly for extending selections. Navigating by mouse or keyboard shouldn't make a difference.

@xJonathanLEI
Copy link
Contributor Author

As for the existing drag-to-yank functionality, instead of always yanking the range dragged, we should instead yank the extended range if select mode is on. This should require no change on the drag handler code though, as it simply always takes the main selection.

@xJonathanLEI
Copy link
Contributor Author

As for the case when multiple selections are active, IMO we should simply remove the secondary selections and extend the primary selection only. Reasons being:

  • Currently the editor already collapses all selections into one when clicked. Doing this aligns with the existing behavior.
  • Extending all selections into the same point would give you one big selection, which is probably not what the user is trying to do.

@xJonathanLEI
Copy link
Contributor Author

Tried a naive implementation of only changing the Down event handler to extend on selection - doesn't work as expected:

  • a simple click would result in content being yanked, as if a drag has been performed
  • editor exits select mode and goes back to normal mode

This is because currently the editor isn't really capable of distinguishing between a click and a drag. It's able to not yank content on a mouse click now only because it does a hacky workaround: it treats a single char selection as a click, and a drag otherwise.

To resolve this, I think we need to really be able to tell the difference between clicks and drags. Maybe we can add a slight delay (buffering) of mouse down event handling like in many GUI applications?

@archseer wdyt?

@xJonathanLEI
Copy link
Contributor Author

A workaround that builds on the current workaround would be to remember the position when Down is received, and compare it against the new position when an Up is handled. I think this would serve the purpose sufficiently well without introducing the complexity (and the tiny delay) involved with the buffering approach.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-helix-term Area: Helix term improvements C-enhancement Category: Improvements
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants