Fix python indentation

[Triton171]

This introduces 2 new capture types for indent queries. With this, the python indent queries have been improved and re-enabled. It should fix most of the issues from #763. We still don't update the indentation retroactively, so there is still some room for improvements:

if condition:
    do_smth()
    else:

Here the else should ideally be outdented automatically, once one has finished typing it. This is tricky for 2 reasons:

• We have to figure out when and for which lines to run indentation queries. For performance reasons, we don't want to do it too often and it also shouldn't "correct" the indentation after the user has manually changed it.
• With python, the tree-sitter grammar doesn't even recognize the else statement since it is indented (see the discussion in #763). Either the tree-sitter grammar needs to improve here or we need a special case to recognize this and similar situations.

If you have ideas for this, please let me know. Also, if there's other languages than python that can be fixed with the new captures (i.e. languages where functions, classes, etc. end simply where the indentation level decreases), I'm happy to update them as well

[AceofSpades5757]

It's also causing an issue where it'll automatically update at the line before indentation, such as if you put your cursor at the beginning of if condition: in your example and press enter to move it down a line.

P.S. This may have also been in the current implementation. I don't usually code Python with Helix due to the really annoying indentation issues. And thanks for your work!

[Triton171]

@AceofSpades5757 good find! The issue already existed in the old version but it's a somewhat rare edge case, so that's probably why it hasn't popped up before. The latest commit should fix it.

[AceofSpades5757]

No problem. I've had this version installed while working with Python to test it out. Python is a real sore spot for Helix. It feels like I'm constantly fighting it.

Hopefully this PR will be able to change that 😄

[AceofSpades5757]

This has been working pretty well, except for an issue where dedenting with < will be multiple indents rather than a single one. It sounds like some other people have this happen with other languages as well.

[Triton171]

That's interesting, the code for dedenting is pretty simple and doesn't use the indentation heuristic. If you can reproduce this, it'd probably be best to create a new issue for this.

[AceofSpades5757]

@Triton171 I haven't been able to reproduce it and I've been using it pretty extensively. I may have accidentally re-installed master as my default Helix. Everything has been going swimmingly recently but if something reproducible comes up, I'll comment here (or make a new issue).

[Dispersia]

Scala has the same issue, and this appears to work mostly correct for it too, though I am getting a strange behavior

When trying to add the indent functionality to scala's indents.scm, i'm currently adding

[
    (function_definition)
    (val_definition)
] @extend-indented

with just this, it seems to work correctly, however when trying to add stop-extend

[
    (identifier)
] @stop-extend

it seems to not work at all and was like its previous behavior. Even having an empty @stop-extend like [ ] @stop-extend does this, which I would expect does nothing

[Triton171]

Could you provide an example where the current indentation for Scala is incorrect? I'm not familiar with the language but from a quick look it seems like Scala uses blocks (delimited by curly braces) to define the range of functions. Because of this, I don't think we need @extend-indented here. Adding (function_definition) @extend-indented would cause indented lines that come after the function definition to be considered part of the function, which is incorrect in this case.

EDIT: I just saw that Scala 3 has multi-line methods without blocks. In this case, we do need the @extend-indented the way you used it. The issue with your @stop-extend query is that not every identifier marks the end of a function. I think we only need to add "}" @stop-extend, which should prevent any issues with the block syntax. We can't use stop-extend the same way I did for Python since Scala doesn't have a dedicated return statement.

I'm also not sure if the Tree-Sitter grammar actually supports multi-line methods: Judging from the syntax highlighting and the :tree-sitter-scopes command, there seems to be some weirdness going on there (the val keyword is parsed as an identifier). Maybe you could check that this actually works correctly. I have never used Scala so maybe I'm just misinterpreting something about the syntax.

[Dispersia]

No you're correct, that syntax for scala 3 is new and not supported by the current tree-sitter syntax, I will be working on that next, I'm just trying to get it to work for what is the current scala 2+ syntax. In scala, you will typically write a one line function as lamda's in C#, something like

val mappedUnit =
    ZIO.succeed(ZStream.fromStream(stream))

etc, where its identifier will go on a new line, not on its current, so it should always be indented. If the user chooses to wrap it in a block, e.g.

val mappedUnit = {
    ZIO.succeed(ZStream.fromStream(stream))
}

it should still count as only one indentation, which is what currently happens correctly. However, when I add even an empty stop-extend, the original extend-indented stops working completely, if I put that line at all in the indents file the val mappedUnit =\n doesn't indent anymore (or if i put a random value, just incase it doesn't like being empty). I am trying to make it work like how current neovim does, which will end the indentation after the first line with a new identifier, effectively making the first statement "return" even if it isnt.

How I might try to achieve this in the long run is the last identifier for tree-sitter will be replaced with a wrapped return type, and then i could do that behavior in here, but im more worried about the common case (as multi line blocks for classes/objects/functions/vals aren't common yet)

[Triton171]

The weird behavior with @stop-extend happens because you're writing an invalid indent query: Tree-Sitter doesn't allow empty alternations (for a description of the query syntax, see their documentation). Unfortunately, this currently fails silently and simply uses the fallback method of copying the indentation of the current line.

For getting the indentation right, you should definitely try to improve the Tree-Sitter grammar before tinkering with it in Helix. Once multiline functions are parsed correctly, you could also use something like (function_definition (identifier) @stop-extend), which would only match an identifier node nested directly inside a function. This prevents e.g. an identifier that's part of an assignment expression to be matched. Basically, you have to think about what exactly makes an expression a "return expression" after which the function ends. Tree-Sitter queries are surprisingly powerful, so once you have an idea of what exactly you want in your head, it often isn't too difficult to write a query for it.

If you manage to fix the grammar and need help with indent queries in Helix, feel free to ping me.

[the-mikedavis]

Unfortunately, this currently fails silently and simply uses the fallback method of copying the indentation of the current line.

Recently we added an error log message for this. There isn't a visual indication as there is for syntax highlighting (i.e. no highlights) but you should be able to find out if your indent queries are invalid either by checking the log file or running cargo xtask query-check if you are building from source and the modified injections are under this repo's runtime/ dir

[Triton171]

I've found an additional issue when there are multiple nested @extend-indented nodes:

def func():
    if condition:
        return
    | # <--- Cursor

In this situation, pressing enter would result in a line that's not indented at all. This happens since we're not inside the if_statement, so the extension prevented by the return_statement is the extension of the function. The latest commit slightly changes the behavior of the @stop-extend capture to fix this.