helix icon indicating copy to clipboard operation
helix copied to clipboard

feat: new `:tree-sitter-tree` command

Open nik-rev opened this issue 1 year ago • 5 comments

Using % to select entire file and then using :tree-sitter-subtree which opens in a tiny window makes it difficult to inspect the entire tree.

This PR adds a new command, :tree-sitter-tree

This command displays the entire tree sitter tree and updates it live as you type. Additionally, it highlights syntax nodes

https://github.com/user-attachments/assets/cd4d8479-b444-40bd-972c-92eaeab02f2d

Closes https://github.com/helix-editor/helix/issues/12149

nik-rev avatar Dec 06 '24 20:12 nik-rev

For now - as the panel doesn't auto update - would it be possible for subsequent :ts-tree invocations to replace the existing (if it still exists) ts-tree buffer instead of creating a new one?

uncenter avatar Dec 06 '24 21:12 uncenter

For now - as the panel doesn't auto update - would it be possible for subsequent :ts-tree invocations to replace the existing (if it still exists) ts-tree buffer instead of creating a new one?

Ok, I just pushed a commit. it will replace the existing buffer now.

nik-rev avatar Dec 06 '24 22:12 nik-rev

And is it possible to prevent writing to the syntax tree buffer? Nvim's impl prevents modifying it, and I think especially now that we replace the buffer with a new one we should expect users not to edit it - and enforce that.

uncenter avatar Dec 06 '24 22:12 uncenter

And is it possible to prevent writing to the syntax tree buffer? Nvim's impl prevents modifying it, and I think especially now that we replace the buffer with a new one we should expect users not to edit it - and enforce that.

It's not possible to make a file unwritable from what I can tell (readonly will still allow editing, but not saving).

See also this comment by @the-mikedavis : https://github.com/helix-editor/helix/pull/7128#issuecomment-1581560237

nik-rev avatar Dec 06 '24 22:12 nik-rev

And is it possible to prevent writing to the syntax tree buffer? Nvim's impl prevents modifying it, and I think especially now that we replace the buffer with a new one we should expect users not to edit it - and enforce that.

I've actually ended up implementing this. Buffers can now be non-modifiable, which prevents running any commands like going into insert mode, delete, paste etc.

nik-rev avatar Jan 31 '25 00:01 nik-rev

tried this pr locally and (for me at least) the position in the tree does not update, when navigating with the mouse. not sure if this is intended, but i just wanted to note here.

otherwise this works really well, thanks for this feature!

short demo video

https://github.com/user-attachments/assets/2a6481f4-f88b-4571-bcad-ee672d8690cc

m4rch3n1ng avatar Oct 10 '25 20:10 m4rch3n1ng

tried this pr locally and (for me at least) the position in the tree does not update, when navigating with the mouse. not sure if this is intended, but i just wanted to note here

~Yeah sadly, there's no other way. At least with the current implementation. Essentially, we update the "tree-sitter tree" buffer every time that we detect movement. The thing is, if we did this for the actual tree-sitter tree buffer it would quickly grow to be infinite in size, because we would compute the tree-sitter tree for contents of the tree-sitter tree buffer.~

Oh, I just re-read your comment. The same issue applies. The way we're using the event system here is incredibly hacky and I honestly should've anticipated that this approach won't work.

    // Create the tree
    update_tree(cx.editor, false)?;

    // Update it until the user closes it
    helix_event::register_hook!(move |e: &mut PostCommand<'_, '_>| {
        update_tree(e.cx.editor, true)
    });

Yeah, this even leaves "dangling" hooks which don't get cleaned up... I believe each invocation of tree-sitter-tree creates a new hook without then un-registering it, I'm not sure what implications this really has, but certainly none positive.

Looking back at this PR, it is so, so hacky, I don't even have any words! I seriously doubt the maintainers will accept this into core, but if any maintainer wants to chime in and have any suggestions you are more than welcome to. I'll let this PR sit around fow a few more weeks or months, but I believe it must be re-written from scratch and this PR should serve as an example of how to not implement this feature

The reason I'm saying that is because I believe the way this PR uses the event system is not the way it's meant to be used. I haven't thought long enough about how exactly to properly implement this, but this is certainly not it!

nik-rev avatar Oct 10 '25 21:10 nik-rev