extend selection should include leading indentation

[matklad]

Consider this code and cursor placement:

In helix, extend selection (alt+o) would select the following:

whereas in IntelliJ the behavior is well, more intelligent :)

It considers leading indentation to be a part of the match expression and selects it. That is, selection splits whitespace token in two.

I'd argue is that IntelliJ's behavior is the correct one here:

• it's more conventient to select: cursor is often on the indentation before "the thing" and hitting Alt-O just once to select the entirety of the thing is very helpful
• it's more convenient to cut the whole thing with indentation, as it removes the thing completely, and alows to easly paste it elswhere, without annoyign whitesapce cleanup.

This is the logic implemented by rust-analyzer's selection ranges (source), though VS Code messes that up a bit by adding its own non-syntactic ranges to the hierarchy.

[pascalkuthe]

This seems like a nice QOL improvement, thank you for sharing your expertise here.

We probably want to cover this right inside select_node_impl in helix-core/src/objects.rs so it applies to all TS functions. Treesitter doesn't keep explicit whitespace nodes around as RA does so the best way to implement this seems to be to just look at the RopeSlice between the two child nodes that surround the selection start and use the same logic as in RA (although RopeSlice sadly lacks many of the convenience methods str has). This should happen before select_fn is called.

Ideally we would do something similar for textobjects aswell so maf also works for initial indentation. That would need to be added to textobject_treesitter in helix-core/src/textobject.rs. Probably inside the filter of the QueryCursor.

The first step would probably some kind of extend_ts_node_to_indent function that would implement logic similar to that in RA (but using a RopeSlice instead of SyntaxNode).

I would like to see this but sadly don't have the bandwith to work on a PR myself right now but I think with these instructions somebody else might be able to pick this up. Maybe you feel up to sending a PR yourself @matklad?

[matklad]

Maybe you feel up to sending a PR yourself

No promises one way or another, but pointers to code much appreciated, thanks!

On Tuesday, 20 December 2022, Pascal Kuthe @.***> wrote:

This seems like a nice QOL improvement, thank you for sharing your expertise here.

We probably want to cover this right inside select_node_impl in helix-core/src/objects.rs so it applied to all functions. Treesitter doesn't keep explicit whitespace nodes around as RA does. the best way to implement seems to be just look at the RopeSlice between the two child nodes that surround the selection start and use the same logic as in RA (although RopeSlice sadly lacks many of the convenience methods str has). This should happen before select_fn is called.

Ideally we would do something similar for textobjects aswell so maf also works for initial indentation. That would need to be added to textobject_treesitter in helix-core/src/textobject.rs. Probably inside the filter of the QueryCursor.

The first step would probably some kind of extend_ts_node_to_indent function that would implement logic similar to that in RA (but using a RopeSlice instead of SyntaxNode).

I would like to see this but sadly don't have the bandwith to work on a PR myself right now but I think with these instructions somebody else might be able to pick this up. Maybe you feel up to sending a PR yourself @matklad https://github.com/matklad?

— Reply to this email directly, view it on GitHub https://github.com/helix-editor/helix/issues/5231#issuecomment-1360389268, or unsubscribe https://github.com/notifications/unsubscribe-auth/AANB3M6O6YV3NE4VJXAWHITWOIVWHANCNFSM6AAAAAATEU2ERM . You are receiving this because you were mentioned.Message ID: @.***>

[the-mikedavis]

I could be biased by the implementation but I think IntelliJ's motion behaviors are confusing (also see https://github.com/helix-editor/helix/discussions/3785). For languages where the indentation is used for scoping this behavior is fine since the indentation is syntactically important, but here the indentation whitespace belongs to the block node rather than the match_expression.

There might be room for an intelligent command like expand_selection that can incorporate this behavior and maybe #3785 but I wouldn't want to see this change to the existing expand_selection command.

[pascalkuthe]

@the-mikedavis Thanks for the pointers. Sorry for jumping the gun here this seemed like a improvement to me as it only applies to start of line indentation which is (at least to me) always part of the text that follows it (especially as it's automatically inserted).

I think keeping this behavior behind a flag would be easy to achieve. we could enable that flag based on whether editor.textobjects.include_leading_indent is set to true. I think that option should be separate from the changes in #3785 (which could be behind a separate config option and also implemented in a seperate PR)

[matklad]

I'd obviously defer to @the-mikedavis to do the judgement call here, but my personal level of confidence that IJ behavior is better is pretty high (0.8), for thee following reasons

• the selection behavior is more useful, on key to select "thee current" thing is pretty huge
• the "cut&paste" behavior is more useful, as you don't leave an extra blank like in the old place
• if I take my compiler writer hat off, and just look at the source code as text, it's natural that indentation is a part of a "thing"
• implementation wise, this depends on the implementation! In particular, Roslyn & Swift attach indentation to the node itself in the tree: https://github.com/apple/swift/tree/5e2c815edfd758f9b1309ce07bfc01c4bc20ec23/lib/Syntax#trivia. Curiously, rust-analyzer didn't do that, and I know consider that to be a mistake: https://github.com/rust-lang/rust-analyzer/issues/6584. To clarify: today, IntelliJ and rust-analyzer do attach whitespace to block, and, for extend selection, have a bunch of custom logic to patch the ranges up. With roslyn model, a naive algorithm of just picking a node wiht appropriate range would do this thing.

EDIT: a nice example would be the task of duplicating a function. Today, doing that with 3x is significantly less fiddly than with Alt-o:

• https://asciinema.org/a/QOvBIcRp4vtzGhLMQMvfJA5lz (Alt-o)
• https://asciinema.org/a/r5KWmu3FhLvVCxeC0SMJfyXg1 (3x)

[the-mikedavis]

Playing around with IntelliJ's C-w, I think this would be implementation-wise a bit complicated because the leading/trailing whitespace is selected for statements but not expressions.

For example...

```java LOGGER.debug( "message", value1, value2 ); ``` If you repeatedly hit C-w anywhere on line 2 (`"message",` or its indentation), you eventually select the inside of the parens rather than the leading whitespace for line 2.

I'm guessing that IntelliJ has language-specific code to do this since it doesn't work on non-Java/Kotlin languages (I tried Rust and Erlang). You could use tree-sitter queries to control which syntax nodes should have their leading/trailing whitespace selected, though this would be a lot of query code for a somewhat small behavior change. So I'm tempted to say that this should be a plugin.

For that example you can make any selection linewise with X so you could do <A-o>X or ]fX. I think it's better - and sort of unix-philosophy-like - to have these selection primitives that select specific things (syntax nodes / textobjects / lines / paragraphs / etc) and to combine them together as needed rather than nuanced motions.

[matklad]

I'm guessing that IntelliJ has language-specific code to do this since it doesn't work on non-Java/Kotlin languages (I tried Rust and Erlang).

I wasn't able to quickly find the relevant impl in the source code, but in my testing the logic seems to be simpler: we don't consider indentation to be part of the element, if there are several elements on the line. In the logger example, "message" and , are on the same line, so we can't say that indentation belongs to "message". Though, I bet IJ has language-specific logic as well.

For that example you can make any selection linewise with X

Ah, I didn't realize that x, X can be made to extend current selection to whole lines this way. Agree that this make selecting indentation somewhat less useful.

I think it's better - and sort of unix-philosophy-like - to have these selection primitives that select specific things (syntax nodes / textobjects / lines / paragraphs / etc) and to combine them together as needed rather than nuanced motions.

Yeah, though, to my eyes, indentation is a part of a specific object (the same way as, eg, mip would include leading indentation). Like,

An if is either the thing in red border, or the thing in yellow border. Including only some of the indents feels exactly like cutting an object in an add-hoc way.