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 a variant of sub-selection (s) that uses textobjects instead of regex #3007

Open
EpocSquadron opened this issue Jul 8, 2022 · 6 comments
Labels
A-tree-sitter Area: Tree-sitter C-enhancement Category: Improvements

Comments

@EpocSquadron
Copy link
Contributor

I think it could be very ergonomic to be able to have a selection broken down by tree-sitter driven textobjects that can give us commonly desired selections.

Consider a selection like the following:

//                  [                   selection                           |]
function doSomething($firstParam, MyTypeHint $secondParam, $thirdParam = null) {
    // do something
}

If you want to select all parameters to the function, say so you could reorder them with Alt-) or append a new line to each to begin reformatting them, you would need to split on spaces with S <enter>, but this will incorrectly split the selection where the default value is defined, and where the typehint is included.

With this proposal, you could enter the new subselection mode (keybind to be determined), and enter a meaning the textobject for parameters/arguements, resulting in the following selections:

//                   [   sel1  |] [         sel2        |] [      sel3     |]
function doSomething($firstParam, MyTypeHint $secondParam, $thirdParam = null) {
    // do something
}

Similar to the mi/ma and ]/[ binds, this new bind would show an autoinfo of possible textobjects, rather than prompting for a regex as the current s and S keybind does.

@EpocSquadron EpocSquadron added the C-enhancement Category: Improvements label Jul 8, 2022
@sudormrfbin sudormrfbin added the A-tree-sitter Area: Tree-sitter label Jul 8, 2022
@CBenoit CBenoit self-assigned this Sep 22, 2022
@CBenoit
Copy link
Member

CBenoit commented Sep 22, 2022

Hi @the-mikedavis
FYI I was thinking about implementing this if it’s okay (I see you assigned yourself for #3263)

@the-mikedavis
Copy link
Member

Sweet! I think the changes for #3263 will end up being separate from this so I don't think we'll step on each others' toes 🙂

@EpocSquadron
Copy link
Contributor Author

For keybinds, I wonder if the existing behavior of s/S can be moved to the autoinfo alongside the textobjects, with / being the triggering option.

@pascalkuthe
Copy link
Member

I would love to see this, @CBenoit did you make any progress on this otherwise I might take this on soonish (altough I quite a few other things I want to finish first)

For keybinds, I wonder if the existing behavior of s/S can be moved to the autoinfo alongside the textobjects, with / being the triggering option.

I would rather see this as a seperate keybinding. s is likely one of the most common keybindings I use in helix, it's so insanely useful and I would want to keep it as a single-key hotkey. Perhaphs an uppercase version might be an option: mI/mA uppercase characters for alternative behaviour seems to be a pattern we are moving towrads in general it sems. What do you think @the-mikedavis?

@EpocSquadron
Copy link
Contributor Author

I would suggest alt-s but I think it's a bit to unergonomic for a potentially often used feature, plus there's discussion around using that binding for #2413.

@the-mikedavis
Copy link
Member

the-mikedavis commented Dec 3, 2022

Figuring out a good keybind for this is tricky. I have a slight preference for putting this under m (maybe mS or m/?). We may also want a "keep" variant of this command like K though which makes it even tricker to find a good binding.

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

No branches or pull requests

5 participants