Extension API feedback/questions

[yyoncho]

Hi, I will use this PR for feedback/question on Extension API. Please let me know whether this is the preffered format for you.

1. How to define a function to return icon for treemacs-define-expandable-node like this:

(treemacs-define-expandable-node lsp-projects
  :icon-open (treemacs-icon-for-file (first item))
  :icon-closed (treemacs-icon-for-file (first item))
  :query-function (lsp-treemacs--get-files (treemacs-button-get btn :key))
  :render-action
  (treemacs-render-node ...)

2. How to define multiple top level roots when using treemacs to create standalone control? (the roots should be dynamic)

[Alexander-Miller]

"Node at path (:custom LSP-Errors) cannot be found"

That should never even happen, right? The one top node is supposed to always be there.

I have tried around, but had no luck producing this error. Let's keep our eyes open for a recipe and hope this isn't some impossible to consistently reproduce race condition.

[yyoncho]

I have tried around, but had no luck producing this error. Let's keep our eyes open for a recipe and hope this isn't some impossible to consistently reproduce race condition.

I am reproducing this issue consistently, I had short debug session and I noticed that at some point treemacs-dom has no entries, here it is tracing of ht-remove! treemacs-on-collapse

======================================================================
1 -> (treemacs-on-collapse (:custom LSP-Errors))
| 2 -> (ht-remove! #s(hash-table size 300 test equal rehash-size 1.5 rehash-threshold 0.8125 data (#1=(:custom LSP-Errors) #s(treemacs-dom-node #1# nil nil #<marker (moves after insertion) at 3 in *LSP Error List*> nil nil))) #1#)
| 2 <- ht-remove!: nil
1 <- treemacs-on-collapse: nil
======================================================================
1 -> (treemacs-find-in-dom (:custom LSP-Errors))
1 <- treemacs-find-in-dom: nil
======================================================================
1 -> (treemacs-find-in-dom (:custom LSP-Errors))
1 <- treemacs-find-in-dom: #s(treemacs-dom-node (:custom LSP-Errors) nil nil #<marker (moves after insertion) at 3 in *LSP Error List*> nil nil)
======================================================================
1 -> (treemacs-on-collapse (:custom LSP-Errors))
| 2 -> (ht-remove! #s(hash-table size 300 test equal rehash-size 1.5 rehash-threshold 0.8125 data (#1=(:custom LSP-Errors) #s(treemacs-dom-node #1# nil nil #<marker (moves after insertion) at 3 in *LSP Error List*> nil nil))) #1#)
| 2 <- ht-remove!: nil
1 <- treemacs-on-collapse: nil
======================================================================
1 -> (treemacs-on-collapse (:custom LSP-Errors))
| 2 -> (ht-remove! #s(hash-table size 300 test equal rehash-size 1.5 rehash-threshold 0.8125 data (#1=(:custom LSP-Errors) #s(treemacs-dom-node #1# nil nil #<marker (moves after insertion) at 3 in *LSP Error List*> nil nil))) #1#)
| 2 <- ht-remove!: nil
1 <- treemacs-on-collapse: nil
======================================================================
1 -> (treemacs-find-in-dom (:custom LSP-Errors))
1 <- treemacs-find-in-dom: nil

[Alexander-Miller]

Does it still happen after https://github.com/Alexander-Miller/treemacs/pull/393 was merged?

I'm not sure if there's a reason for clearing the DOM when adding the first project?

With a bit of luck this'll be your answer @Jasu. (I assume I was being overly cautious, and tried to make sure the dom won't contain any remnants from a previous cycle.)

[yyoncho]

@Alexander-Miller I was unable to reproduce the issue. Seems like the only outstanding issue is the ability to define multiple (dynamic) top level extensions.

[Alexander-Miller]

It's been a few weeks, but now I have some capacity again, so let's get back to extensions. I don't know if you know (it's been too long and I had certainly forgotten myself and had to look at the code), but variadic extensions can already be created if :top-level-marker is 'variadic.

The resulting treemacs-%name-extension function will then accept a list of cons cells where the car is the label and the cdr the key of the top level extension node. So setup is already possible and you can start experimenting.

Only updating remains a completely open issue. And no matter how much I think about this I see no way to update things without supplying some kind of new dom. I do not have a conclusive implementation yet, but it's most likely going to require a nested list, similar to what is produced by (imenu--make-index-alist t), except without the markers.

I'll let you know when I have come up with something.

[yyoncho]

The resulting treemacs-%name-extension function will then accept a list of cons cells where the car is the label and the cdr the key of the top level extension node. So setup is already possible and you can start experimenting.

But I still need to define a top-level extension node? And when the node is removed I will have to delete it?

Only updating remains a completely open issue. And no matter how much I think about this I see no way to update things without supplying some kind of new dom. I do not have a conclusive implementation yet, but it's most likely going to require a nested list, similar to what is produced by (imenu--make-index-alist t), except without the markers.

Didn't we end up with deciding just to hide the root node and use the same code?

[Alexander-Miller]

Didn't we end up with deciding just to hide the root node and use the same code?

I had previously considered it too awkward a workaround, but after all the digging I've now made I have changed certainly changed my mind. Thanks for the reminder. It does however mean that the current variadic implementation does not fit, so I'll will have to redo that part.

But I still need to define a top-level extension node? And when the node is removed I will have to delete it?

There will be a redesign for this. Here's my current plan: the treemacs-%name-extension will take no arguments, the variadic top-level nodes are created with the query-function like all other nodes. Calling it will set some buffer-local value that you can update with some variation of (treemacs-update-node foo).

[yyoncho]

I am assuming foo is the new list of top nodes?

[Alexander-Miller]

No, it's not the list, it's whatever variable is set when the extension is rendered, so it's basically your "handle" for the extension. The update will work with usual close-and-open plan, so the new nodes are just called up from the query-function.

[yyoncho]

Oh, sorry, now I got it. I am very tired right now.

[Alexander-Miller]

@yyoncho, @Jasu I am stuck and could use a second or third set of eyes.

I have tried hiding the first line, where our fake extension root would sit, with both overlays and text-properties. For the largest part it works, but point can still somehow move to the first invisible line, when navigating with j/k, or when calling something like evil-goto-first-line. That makes hl-line disappear and makes it possible to trigger the hidden extension node with TAB.

I would rather not use narrowing since I am not sure when exactly I would need to widen the buffer again, and the only other solution I can think of is a crude post command hook that would always move out of the way of such a situation like treemacs--evade-image.

So can you guys think of anything else I can try? I have uploaded the current state to the hide-first-line branch, so to see for yourself just load treemacs, Extensions.el and call showcase-display-buffer-list. You can find the code create the overlay at https://github.com/Alexander-Miller/treemacs/blob/hide-first-line/src/elisp/treemacs-extensions.el#L469.

[yyoncho]

Before digging in your code, have you considered using https://www.gnu.org/software/emacs/manual/html_node/elisp/Invisible-Text.html (I recently fixed lsp-mode bug related)?

[Alexander-Miller]

Yep, tried that as well. In the lines I pointed out you can swap the 2 overlay lines for (put-text-property (point-at-bol) (1+ (point-at-eol)) 'invisible t) and the result is almost the same.

[yyoncho]

That makes hl-line disappear and makes it possible to trigger the hidden extension node with TAB.

What about disabling tab action on this node?

[Alexander-Miller]

That'd leave us in a position where TAB does nothing. The real problem is that pressing k when on the first line looks like it just removes the hl-line overlay, while in recent versions navigation should wrap to the last line of the buffer.

[yyoncho]

What about this -

(with-current-buffer "*LSP Error List*"
  (let ((inhibit-read-only t))
    (goto-char (point-min))
    (delete-region (point-min) (1+ (point-at-eol)))))

... with treemacs-update-node restoring the deleted text? It is not very elegant, but it might work.

[Jasu]

I managed to make this work in #416 in a rather hacky way. I will update the specifics in the PR comments.

Edit: Updated the comments in the PR.

[Jasu]

Oops, I just realized that the button is positioned after the contents...

[Jasu]

Fixed that one as well.

[Jasu]

I fixed an issue with treemacs-next-line and treemacs-previous-line.

Buttons can have a skip property that makes next-button and previous-button skip them. This makes the cursor-sensor-functions trigger only when navigating without next-button or previous-button.

[Jasu]

I just pushed one more fix, makes commands that go to buffer start work and fixes spacing if the extension is at 'bottom.

[Alexander-Miller]

with treemacs-update-node restoring the deleted text? It is not very elegant, but it might work.

Same problem as narrowing. The invisible node's text properties are referenced at least when its children are expanded to make the necessary entry in the dom.

Looks like we need to go with a post command hook after all.

@Jasu Thanks for the feedback :+1: .

I'll be rather busy this week, but I'm hoping I'll be able to have this in master over the weekend, unless I run into any other surprises.

[Alexander-Miller]

First version of variadic extensions is now pushed. See treemacs-define-variadic-node. (Mind that it's a force push, I had to fix a commit message)

I think this is the final big piece of the extension api, so next up is cleanup and taking care of the pfuture PR.

[yyoncho]

@Alexander-Miller thank you!

What is the way in the new variadic extensions to update the nodes?

[Alexander-Miller]

Basically the usual should do. I experimented with the following form:

(treemacs-define-variadic-node buffers-root-top
  :query-function (showcase--get-buffer-groups)
  :root-key-form 'Buffers
  ...

So the update code would look like this:

(with-selected-window (get-buffer-window (get-buffer "*Showcase Buffer List*"))
  (treemacs-update-node '(:custom Buffers)))

However it looks like something's off with node states right now, so it won't work yet.

[Alexander-Miller]

I had a load of trouble with newlines in the hidden node, but it should be fixed now.

[yyoncho]

I updated lsp-treemacs and everything seems to be working fine except the following case:

Expand Error -> Fix All Errors -> Introduce new errors

Actual: the list is not refreshed(it is empty).

[Alexander-Miller]

Completely empty or just not expanded? As far as I can see there's some issue with the dom not sticking around or the re-open not working after things are rebuilt.

[yyoncho]

Completely empty.

[Alexander-Miller]

Before I start digging deep: try the new force-expand parameter in treemacs-update-node. My guess is that the hidden node got closed somehow and appears empty because updating a closed node does nothing.

You also don't need to call treemacs-expand-lsp-error-list, variadic extensions start off expanded by default.

[yyoncho]

It fixed the issue, thank you! I think that we can close that issue. I will wait for the themes support and I will proceed with porting the dap-ui to treemacs.

[yyoncho]

@Alexander-Miller it seems like the selection is moved to the first item when I do treemacs-update-node. Is there a way to preserve or eventually restore the selection?

PS: have you considered creating a gitter chat for treemacs?

[Alexander-Miller]

it seems like the selection is moved to the first item when I do treemacs-update-node.

That depends on the before and after state of the tree. Treemacs does attempt to stay either on the same node, or somewhere nearby, or at least on the same screen-line. So maybe my macros cannot quite handle a pure custom-node environment, or maybe the nodes' paths change subtly in between updates and treemacs cannot find the way back (do you use lsp-mode's internal structures as path elements?).

Can you give me a minimal case based on some of the python scripts in treemacs, or some other trivial project that won't require me installing a full-on jdk?

have you considered creating a gitter chat for treemacs?

Nope, I've no idea what goes into that, and depending on the volume I am not sure I could or would even want to answer to all requests, I prefer working with tickets. But I guess the proof of the pudding is in the eating.

[yyoncho]

paths change subtly in between updates and treemacs cannot find the way back

The issue was that for one of the nodes I was using cl-struct which was breaking the logic. After changing the key now it works better. There is still one minor issue which I guess could be addressed in the future - when I edit the file and the diagnostic is changed and I have no way to preserve the same key so the selection is moved to the parent of the item - I would prefer selecting the element with the same index under the same parent which in most of the cases will be the same diagnostic but on a different line (I could work on that in the future)

Nope, I've no idea what goes into that, and depending on the volume I am not sure I could or would even want to answer to all requests, I prefer working with tickets.

It is a matter of 1-2 clicks and IMO it is useful for asking quick questions and also the power users usually help with answering the recurring questions.

[Alexander-Miller]

I would prefer selecting the element with the same index under the same parent which in most of the cases will be the same diagnostic but on a different line

Sounds workable, though I don't think it's an extension issue, but a matter of tuning the behaviour of the -save-position macro. Best open up a new issue and I'll take a look at it.

It is a matter of 1-2 clicks and IMO it is useful for asking quick questions and also the power users usually help with answering the recurring questions.

Done, and invited you as well.