Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Make A-hjkl tree-sitter nav A-poin #2205

Merged
merged 1 commit into from
Apr 29, 2022

Conversation

pickfire
Copy link
Contributor

@pickfire pickfire commented Apr 20, 2022

A-hl currently is not very consistent with hl when next object is
selected, since it may go up/down or left/right and this behavior is
confusing such that some people think it should swap the keys with A-jk,
so it is better to use A-pn since that only specifies two direction.

A-jk have the same issue as in it usually moves right and is not
consistent with the behavior of jk so people may think A-hl is better,
maybe A-oi is better here since A-hl will be swapped to A-pn, A-oi can
convey the meaning of in and out, similar to some window manager keys?

cc @the-mikedavis since he came up with the original keys, but this was discussed with him in the chat

Note sure what to do with the existing A-left, A-right, A-up, A-down so I just leave it

Fix #2202

@pickfire pickfire force-pushed the consistent-treesitter-nav branch from 7cea752 to 3dd1fe6 Compare April 20, 2022 23:53
@the-mikedavis
Copy link
Member

I was curious which should be shrink vs. expand with i and o. I suppose i could be into the tree (shrink) and o out of the tree (expand), but OTOH i jumps forward in the jumplist and o backwards which might map better to i expand, o shrink.

I'm not sure which way makes more sense 🤔

A-hl currently is not very consistent with hl when next object is
selected, since it may go up/down or left/right and this behavior is
confusing such that some people think it should swap the keys with A-jk,
so it is better to use A-pn since that only specifies two direction.

A-jk have the same issue as in it usually moves right and is not
consistent with the behavior of jk so people may think A-hl is better,
maybe A-oi is better here since A-hl will be swapped to A-pn, A-oi can
convey the meaning of in and out, similar to some window manager keys?
@pickfire pickfire force-pushed the consistent-treesitter-nav branch from 3dd1fe6 to 04d8ce5 Compare April 20, 2022 23:54
@pickfire
Copy link
Contributor Author

pickfire commented Apr 20, 2022

I was curious which should be shrink vs. expand with i and o. I suppose i could be into the tree (shrink) and o out of the tree (expand), but OTOH i jumps forward in the jumplist and o backwards which might map better to i expand, o shrink.

Oh, wait I thought it was a typo. But shouldn't A-i go in (shrink) and A-o go out (expand)?

OTOH i jumps forward in the jumplist and o backwards which might map better to i expand, o shrink

I don't quite get that analogy there, that is forward and backward but I see very little relationship between forward with expand and backward with shrink, compared to in/into/inside (I use but you used into) with shrink and out/outside with expand. Or maybe I should just rename it to in and out to let other people understand the analogy easier?

@the-mikedavis
Copy link
Member

Oh actually yeah, let's do i for shrink and o for expand. I think that makes sense intuitively

@pickfire
Copy link
Contributor Author

pickfire commented Apr 21, 2022

Should I go change the docs to make it in/into/inside and out/outside to make it more intuitive? I also don't know which word to use among those.

@the-mikedavis
Copy link
Member

Nah I think the docs are fine as is. IMO mnemonics don't really belong there and "out" and "in" don't really make sense themselves.

@the-mikedavis
Copy link
Member

I say :shipit: for these bindings but I'm curious what others have to say

@sudormrfbin
Copy link
Member

Oh actually yeah, let's do i for shrink and o for expand. I think that makes sense intuitively

Agree with this, feels intuitive considering the default keybinds of ctrl-{i,o} for the jumplist. Alt-{n,p} sounds like a good change too 👍🏾.

@tomKPZ
Copy link
Contributor

tomKPZ commented Apr 24, 2022

This change looks good to me. I want to add a "select all children" binding (in addition to the current "select first child"), which doesn't fit into the HJKL model anyway.

@altsem
Copy link

altsem commented Apr 24, 2022

I'd love to use these bindings on a non-us layout Mac. It's a complete mess with binding the alt-key on a Mac with a Swedish keyboard layout. I digged the SpaceVim bindings for this: just keep pressing v (V to shrink?).

@archseer
Copy link
Member

I'd love to use these bindings on a non-us layout Mac. It's a complete mess with binding the alt-key on a Mac with a Swedish keyboard layout. I digged the SpaceVim bindings for this: just keep pressing v (V to shrink?).

Hmm, interesting. we could have some kind of submode you'd enter that would have these types of sticky mappings. For now though, you can remap these alt keys in your user config

@archseer archseer merged commit ab6a00e into helix-editor:master Apr 29, 2022
@helmesjo
Copy link

helmesjo commented Jun 2, 2022

Slight bug noticed (most likely not specifically related to this PR):

  1. Open a file.
  2. Keep hitting A-o until whole file selected.
  3. At this moment, as many times as you keep hitting A-o (everything is already selected, so nothing happens visually), the same amount of times you need to hit A-i to "undo" it before you start seeing things deselect (it basically buffers up, even though there is nothing else to select).

@the-mikedavis
Copy link
Member

Could you create a separate issue for that? Or if you'd like to work on a PR directly, I think this could be solved by checking the head of view.object_selections to see if we're pushing the same selection

// save current selection so it can be restored using shrink_selection
view.object_selections.push(current_selection.clone());

@pickfire pickfire deleted the consistent-treesitter-nav branch June 2, 2022 23:19
@helmesjo
Copy link

helmesjo commented Jun 8, 2022

@the-mikedavis Sorry for late reply. Added issue #2714.

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

Successfully merging this pull request may close these issues.

Swap alt-j with alt-l and alt-k with alt-h
7 participants