search

emacs-lsp / lsp-treemacs

lsp-mode :heart: treemacs
GNU General Public License v3.0
398 stars 47 forks source link

Added depth argument to lsp-treemacs--expand #42

Closed kira-bruneau closed 4 years ago

kira-bruneau commented 4 years ago

Replaced expand? with expand-depth in lsp-treemacs-render and added a depth argument to lsp-treemacs--expand to support https://github.com/emacs-lsp/dap-mode/issues/230.

ericdallo commented 4 years ago

Thanks, I was testing it with the depth branch of treemacs, but it looks like it's now on master.

ericdallo commented 4 years ago

Sorry, I think I saw it wrong, the depth-expand code is not on master yet and I just tested and it's not working :pensive: I'll revert the PR and reopen a new PR

ericdallo commented 4 years ago

Sorry for the misunderstanding @MetaDark : https://github.com/emacs-lsp/lsp-treemacs/pull/44

We still need the depth-expand branch on treemacs on master, but I'm not sure if it's working correctly: https://github.com/Alexander-Miller/treemacs/issues/441

kira-bruneau commented 4 years ago

@ericdallo Oh sorry, I didn't realize there was a PR already open for this, my change was specifically for dap-ui-locals, but it looks like the changes on the depth-expand branch are more general.

ericdallo commented 4 years ago

Yes, sorry for not explaining that before. I guess you can start testing the depth-expand branch with a minor change on lsp-treemacs to support the number arg and give any feedback to AlexanderMiller on that issue if does not work correctly, what do you think?

kira-bruneau commented 4 years ago

@ericdallo Ok, yeah I will try it out.

kira-bruneau commented 4 years ago

It looks like the problem on depth-expand is a typo on https://github.com/Alexander-Miller/treemacs/blob/698a403c89ee038debbda70cba4945d73c0ae014/src/elisp/treemacs-extensions.el#L379

It should be recursive not recusrive. Even with that fix though, my commit doesn't depend on anything in depth-expand, and it doesn't seem like it should. Is there a reason why we need to wait on depth-expand to be merged?

ericdallo commented 4 years ago

Yeah, I see what you did, you just call the recursive function one time (given the depth = 1), but we have another PR that adds a custom expand option.