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

Tree-sitter movements do not respect extend mode #1999

Open
oati opened this issue Apr 7, 2022 · 9 comments
Open

Tree-sitter movements do not respect extend mode #1999

oati opened this issue Apr 7, 2022 · 9 comments
Labels
A-helix-term Area: Helix term improvements C-enhancement Category: Improvements

Comments

@oati
Copy link
Contributor

oati commented Apr 7, 2022

Summary

The behavior of <alt>+left <alt>+right <alt>+up <alt>+down in select/extend mode is the exact same as in normal mode.

I expected these commands to extend my selection in select mode, by remembering my original starting point.

Reproduction Steps

I tried this:

(In a supported language)

  1. press <alt>+up and select a syntax node
  2. go into select mode with v
  3. press <alt>+right

I expected my selection to be extended to the next tree-sitter node.

Instead, Helix forgets my selection and selects the next tree-sitter node.

Helix log

No response

Platform

Linux

Terminal Emulator

GNOME Console

Helix Version

22.03-49-gb03421a8

@oati oati added the C-bug Category: This is a bug label Apr 7, 2022
@the-mikedavis the-mikedavis added the A-helix-term Area: Helix term improvements label Apr 7, 2022
@andreytkachenko
Copy link
Contributor

I would add Shift-Alt-Left and Shift-Alt-Right for extend to left and right.

@the-mikedavis
Copy link
Member

Extending with shift is kakoune-like and would be inconsistent with the rest of the selection extending commands. This should work with selection mode v instead of new bindings.

@the-mikedavis
Copy link
Member

the-mikedavis commented Apr 8, 2022

Implementation-wise, here are the commands for moving the selection to siblings. Instead of naively replacing the selection, we need to union the selection between the existing selection and the new node when in selection mode.

I think extending makes sense for select_next_sibling and select_prev_sibling but not for expand_selection (Alt-k or Alt-up: normal mode vs. select mode wouldn't have a difference) or shrink_selection (Alt-j or Alt-down: the command wouldn't have an effect).

@oati
Copy link
Contributor Author

oati commented Apr 8, 2022

I think <alt>+up and <alt>+down would still be useful in select mode if you wanted to use it to, for example, extend your selection to a smaller node.

But trying to think about these commands as navigation raises the question,

"should tree-sitter movements in select mode

  1. extend from selection,
  2. extend from cursor, or
  3. take a union (create new selections)?"

@the-mikedavis
Copy link
Member

Oh hmm that's a good question. Originally I was thinking the union, so if you had something like

myfunc(a, [b], c) # selection is around `b`

with a tree like

(call
  (argument)
  (argument) ; selection is wrapping this
  (argument))

v Alt-l (or v Alt-Right) would change it to

myfunc(a, [b], [c]) # one selection for `b`, another for `c`
(call
  (argument)
  (argument) ; one here
  (argument)) ; another here

But it might be useful to have it extend the range of the current selection, so:

myfunc(a, [b, c]) # selection starting at `b`, ending on `c`

I suppose I would lean towards the union that makes new selections which is similar to how vn and vp work when adding selections with search.

@archseer
Copy link
Member

Union: put_cursor should be enough for extending downwards with the extend flag, it'll move the head but retain the anchor. For extending upwards, construct a new range for the sibling then use put_cursor on it with the current head value?

@oati
Copy link
Contributor Author

oati commented May 6, 2022

I think <alt>+up and <alt>+down would still be useful in select mode if you wanted to use it to, for example, extend your selection to a smaller node.

But trying to think about these commands as navigation raises the question,

"should tree-sitter movements in select mode

1. extend from selection,

2. extend from cursor, or

3. take a union (create new selections)?"

@the-mikedavis To me, it seems like both 2 and 3 would be useful in different situations, while 1 could be emulated using 2 with a cursor / anchor swap.

The inconsistency of vn and vp behavior, and the case for number 3 here seems to suggest that we should maybe consider a new mode for "add new selection" / "control primary selection":

This mode would allow you to "freeze" all non-primary selections while you manipulate the primary selection or add a new selection (which would be useful anyway in a multiple-selection-based editing model).

In one possible implementation of this, behavior 3 could be executed as

  1. control mode (lock all non-primary selections)
  2. add selection (create a non-primary selection in our place)
  3. next sibling
  4. escape to normal mode

This allows for more granular control, allowing us, for example, to skip siblings (like operators in an expression), or make selections for more complex non-sibling relationships.

The current behavior of vn and vp could easily be moved to this mode, with more intuitive behaviors.

This would be a pretty big change to Helix's editing model, and there are some problems to consider.

  • What do we do when the user tries to add overlapping no-primary selections in this mode?
  • What changes do we have to make to allow the primary selection to overlap non-primary selections while we navigate in this mode?
  • How does this affect the role of "keep/remove selections" and "keep/remove primary selection"?
    • Can we extend the behavior of these to allow for powerful filter_map-like operations? Maybe we could enter "control mode" with multiple active selections.

@EpocSquadron
Copy link
Contributor

@oati I outlined just such a proposal in #943 ! Would love to see more engagement on it.

@EpocSquadron
Copy link
Contributor

EpocSquadron commented May 6, 2022

For what is worth I really like the idea of this creating a new selection rather than extending, as @the-mikedavis describes. I'm thinking back to kakoune and the ability to merge selection marks and how something like that might allow us to get the both of best worlds -- v alt-n creates a second selection, then some other keybind allows us to make a single selection out of the multiple ones that spans all of them.

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

No branches or pull requests

5 participants