Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Let's talk about treemacs and spacemacs #9285

Closed
Alexander-Miller opened this Issue Jul 26, 2017 · 34 comments

Comments

Projects
None yet
7 participants

Remember treemacs? There's been a bit of talk about integrating it in spacemacs on the gitter a couple weeks ago, bit it quickly fizzled out since treemacs wasn't out on MELPA then. Well it's out now, and I'm currently busy cleaning up and writing tests for a big new feature - showing imenu (and potentially other) tags - so I'll have some free capacity soon.

I like to think that my readme presents good enough a case for treemacs on its own (and I don't like attacking the competition to make a better case for myself), so if the interest on the side of spacemacs is still there I'd take a crack at integrating treemacs and spacemacs soon. I just need to know whether it should be done as an extra layer, or if we're going for a full replacement in the core. IMO the latter would be easier since then there would be no conflicts with e.g. winum-assign-func.

And of course if someone has an idea for icons I can use I'm all ears.

Collaborator

JAremko commented Jul 26, 2017

Collaborator

bmag commented Jul 26, 2017

I see there's a dependency on Emacs 25.1. Currently Spacemacs supports also 24.4 and 24.5, so we can't just replace Neotree with Treemacs in spacemacs-navigation layer (this is what counts as "full replacement in the core"). We have the tools to have both Neotree and Treemacs in the same layer, and only install and load one of them depending on Emacs version, but we probably don't want this complexity to be part of our default Spacemacs experience. (I'm not saying there's 0% chance, but personally I'm against it)

For now, I think the best way is to provide Treemacs as a separate optional layer (similar to imenu-list). I don't remember the exact semantics, but a layer in Spacemacs can cause other packages/layers to be excluded (meaning, they won't be installed). Treemacs can exclude Neotree by using :excluded in the layer's package list. I see that spacemacs-navigation/init-winum is going to set winum-assign-func even if Neotree is excluded - but this can be fixed.

Note that even a full replacement doesn't necessarily mean we get rid of Neotree - we could (and IMO should) just move it to an optional layer in this case. That means there's still a potential conflict with winum-assign-func. By the way, I'm not sure there's going to be any Treemacs-Neotree conflicts other than Winum integration and key-binding.

If you need help with the Imenu part, then you can check out how imenu-list collects and displays Imenu entries. You're welcome to ping me on gitter if you have questions (although it's been a while since I wrote that code).

Not sure what kind of icons you're looking for, but all-the-icons have a few links in their readme.

Anyway, I think Treemacs will be a good addition, even if we can't make it part of the default distribution.

Currently Spacemacs supports also 24.4 and 24.5, so we can't just replace Neotree with Treemacs in spacemacs-navigation layer

That dependency is for pfuture, which I use to asynchronously fetch git info. If some can help me rewrite this function in an emacs24 compatible way the dependency can be dropped.

That means there's still a potential conflict with winum-assign-func

I've opened a PR against winum to avoid such conflicts. When it gets merged we'll have a list of assign functions instead. I'll also make the necessary adjustments in spacemacs then.

By the way, I'm not sure there's going to be any Treemacs-Neotree conflicts other than Winum integration and key-binding.

Maybe not with Neotree, but there's bound to be one or two surprises that need fixing. For example spacemacs/alternate-buffer will sometimes show the treemacs buffer, even though treemacs tries to make sure to stick to the bottom of the buffer list. I'll have to dig deep and see where that buffer-predicate frame param comes from.

Be great if someone could play around with treemacs and see if there's anything else that needs tuning. I hardly cover all possible use-cases in my own daily usage.

If you need help with the Imenu part, then you can check out how imenu-list collects and displays Imenu entries.

I've no problems with the displaying bits. If I keep generalizing what I have I'll probably end up with my own tree widget framework. AFAIK we also use different approaches, imenu-list uses overlays to hide and show a static tree while treemacs eagerly builds and destroys its branches. I would however be very much interested in knowing what you use to fetch the tag point is at - I could use that to make follow-mode follow tags as well as files.

Not sure what kind of icons you're looking for, but all-the-icons have a few links in their readme.

Something that looks reasonably nice as a 22x22 pixel png with both bright and dark themes. I do not use all-the-icons because its iconic fonts are not monospaced, though I do have plans for a simplification for my icon handling that should make it possible to add something like an all-the-icons-fallback-mode, or to easily define new treemacs icons as strings without touching png files.

Collaborator

d12frosted commented Jul 26, 2017 edited

There's been a bit of talk about integrating it in spacemacs on the gitter a couple weeks ago, bit it quickly fizzled out since treemacs wasn't out on MELPA then.

I don't see any problems 😸 There is a GitHub fetcher for packages.


To be honest, I don't use Neotree. And it's hard for me to tell the difference between Treemacs and Neotree. Could you please share some comparisons? I understand that you don't want to attack Neotree, but I bet there are objective differences you could point out. And thanks in advance!

If you wish to find some 'corner' use cases, just take a look at the list of issues related to Notree. Could insight you a little 😸

P. S. Congratulates with MELPA release 😸

P. P. S. Going through README makes me want to give it a try 😸

Collaborator

JAremko commented Jul 26, 2017

like @d12frosted I don't use file trees and stuff (except imenu) but I do think that nice flashy things look good on screenshots and can sell Spacemacs to people on the fences. This is why we need treemacs and MiniMap :shipit:

Could you please share some comparisons?

Oh well, this is off the top of my head, and someone do correct me if I'm wrong, my knowledge of neotree's codebase may well be outdated:

  • treemacs looks nicer by using real pngs (quantity is another matter, hence my asking here for icon ideas). Yes, there's stuff like all-the-icons, but see above screenshot.

  • Both use git integration, but neotree uses emacs' builtin vc functions which cache their results for performance. This means they're not aware of actions from other places like magit or the command line. Treemacs always launches a new asynchronous git process.

  • treemacs-follow-mode lets the treemacs view always move to the currently selected file. In neotree this is only possible manually.

  • treemacs-filewatch-mode lets treemacs refresh its view on file changes. Neotree does something similar, but it's an indiscriminate refresh on a timer, while treemacs does actually watch the part of your file system it is displaying.

  • AFAIK neotree doesn't have as many ways to open a file as treemacs.

  • Neat upcoming stuff like the display of tags. Here, have a preview

  • Treemacs gets along with eyebrowse. AFAIK Neotree still doesn't, and that's the reason I began work on treemacs.

  • Better performance. Only matters if your view is >1000 nodes.

  • I document my features. On neotree I had to look at the code to find out that neo-vc-integration is a list.

If you wish to find some 'corner' use cases, just take a look at the list of issues related to Notree.

From a cursory overview of the first page I didn't find anything affecting treemacs. But I'll keep these in mind when I'm testing my work in the future.

Collaborator

bmag commented Jul 26, 2017

That dependency is for pfuture

Is it just make-process? Looks like start-process can be used instead (and exists in 24.4, 24.5).

For example spacemacs/alternate-buffer will sometimes show the treemacs buffer

Changing the buffer name from Treemacs to *Treemacs* should fix that. The buffer-predicate comes from spacemacs-base/config.el, by the way.

what you use to fetch the tag point is at

The function is imenu-list--current-entry. It just scans the static tree and compares each entry's position (in the indexed buffer) to the current position of point (in the indexed buffer). It stops when entry-position <= current-point-position, and IIRC assumes the order of entries in the tree is the same of their order in the indexed buffer. Extracting the entry's position can be tricky (e.g. when Semantic generates the index), and there's also a guard of (point-min-marker) in case of a narrowed buffer.

Looks like start-process can be used instead (and exists in 24.4, 24.5).

You sure its start-process? I looked it up and the first lines are

(unless (fboundp 'make-process)
    (error "Emacs was compiled without subprocess support"))

Or does it have a different implementation in earlier versions?

Extracting the entry's position can be tricky (e.g. when Semantic generates the index)

Ok, definitely complex enough that I'll put it on my todo list to try later.

Collaborator

bmag commented Jul 26, 2017 edited

In earlier versions start-process is a C function.

I've now rewritten pfuture to use start-process. Is anyone here actually on emacs24 and can verify it's working? The code is quite short, so you can just copy-paste it into the scratch buffer.

If the rewrite went well I guess the replacement's back on the table? Of course if replacing neotree involves putting it in a layer I'll need to write one anyway. IIRC the idea of replacement was first brought up by @syl20bnr on gitter - so if you could share your opinion here that'd be great.

I've also now pushed a new treemacs version, including a beta of the tag display feature, if anyone wants to take a look.

I'm starting work on a treemacs layer now. Treemacs has its own evil compatibility layer in the form of the treemacs-evil package. Is there an idiomatic way for a layer to install and load a package based on dotspacemacs-editing-style, or should I just use nested use-package statements?

Collaborator

bmag commented Aug 6, 2017

If dotspacemacs-editing-style is set early enough (I don't remember if it is), then you can use :toggle keyword in the layer's packages list:

(setq <layer>-packages '(<package1>
                         <package2>
                         (treemacs-evil :toggle (memq dotspacemacs-editing-style '(vim hybrid)))
                         <package3> ...))

Another option is to use the :if keyword in a use-package declaration, like in spacemacs-evil/init-evil-ediff.

If dotspacemacs-editing-style is set early enough (I don't remember if it is)

Looks like it isn't, adding the :toggle seems to do nothing. Use-package works fine, but it does install treemacs-evil when treemacs is first launched. It'd be nicer if treemacs was installed at boot if needed, but was loaded after treemacs.

Another question: what do you think I should do about treemacs-delete-other-windows? It's the same as delete-other-windows, but doesn't delete treemacs. Should it have it's own keybind (if so which?) or should I try to integrate it withspacemacs/toggle-maximize-buffer?

Collaborator

bmag commented Aug 7, 2017

what do you think I should do about treemacs-delete-other-windows?

I think you should get rid of it and use a window parameter instead: either set the delete-other-windows window parameter of treemacs' window, or make treemacs' window a side window. You might want to make the window dedicated as well. (see [1], [2])

[1] deleting windows
[2] dedicated windows

Use-package works fine, but it does install treemacs-evil when treemacs is first launched. It'd be nicer if treemacs was installed at boot if needed, but was loaded after treemacs.

(in last senetence, I'm assuming you meant "... nicer if treemacs-evil was ... after treemacs")

I'm surprised by this behavior. I expected with use-package's :if approach both treemacs and treemacs-evil will be installed at boot, and making treemacs-evil load after treemacs shouldn't be hard (after understanding how Spacemacs loads and configures packages). Guess you're doing something else than what I had in mind:

;; ignoring treemacs package in this example; assuming layer's name is "treemacs"
;; based on evil-ediff's configuration
(setq treemacs-packages '(treemacs-evil))

(defun treemacs/init-treemacs-evil ()
  (use-package treemacs-evil
    :after (treemacs)
    :if (memq dotspacemacs-editing-style '(hybrid vim))))

Guess you're doing something else than what I had in mind:

Yeah, for whatever reason I completely missed giving treemacs-evil its own init function and just placed the use-package call in init-treemacs. My bad. Using above code works with :toggle.

either set the delete-other-windows window parameter of treemacs' window,

I played around with it and either I'm doing something wrong, or it's not what I need. If I set the delete-other-window param of the treemacs window to a function it is only invoked when delete-other-windows is called from within the treemacs window. treemacs-delete-other-windows is meant to be called from a window you want to maximize without killing the treemacs window.

or make treemacs' window a side window.

Not sure what that means exactly. Google only shows the gnu docs about splitting windows. Treemacs already uses (display-buffer-in-side-window (get-buffer-create treemacs--buffer-name) '((side . left))) to appear, so I guess this is already done? If so, what does that have to do with preventing the deletion of the treemacs window?

you might want to make the window dedicated as well.

That's already in place.

Collaborator

bmag commented Aug 8, 2017

either set the delete-other-windows window parameter of treemacs' window,

I played around with it and either I'm doing something wrong, or it's not what I need. If I set the delete-other-window param of the treemacs window to a function it is only invoked when delete-other-windows is called from within the treemacs window.

You wrote that you set delete-other-window - while it should be delete-other-windows. Not sure if it's a typo or something else. Anyway, you don't need it if you're utilizing side windows already.

Treemacs already uses (display-buffer-in-side-window (get-buffer-create treemacs--buffer-name) '((side . left))) to appear, so I guess this is already done? If so, what does that have to do with preventing the deletion of the treemacs window?

The function delete-other-windows doesn't delete side windows. Since you say treemacs displays itself as a side window (uses display-buffer-in-side-window), then delete-other-windows isn't supposed to delete treemacs window in the first place. This carries over to spacemacs/toggle-maximize-buffer, since it uses delete-other-windows internally.

If the treemacs windows doesn't survive delete-other-windows, then it most likely lost its status as a side window at some point beforehand. (some packages can do that, e.g. popwin)

Collaborator

bmag commented Aug 8, 2017

Well, I'm testing treemacs a bit. I added treemacs and treemacs-evil as additional packages, and a (require 'treemacs-evil) in user-config. First impressions:

  • delete-other-windows and spacemacs/toggle-maximize-buffer don't delete treemacs window 👍 Also, there's a bug in spacemacs/toggle-maximize-buffer that it doesn't restore the window configuration, because of the side window.
  • In treemacs buffer, mode line is empty and cursor is invisible. That's because Spacemacs has its own cursor configuration that doesn't know about evil-treemacs-state (see spacemacs-evil-cursors). Judging by the code, I don't understand why you can't use just motion state for treemacs buffer. Is it just for having a different cursor shape? (is :cursor (bar . 0) supposed to make the cursor invisible?)

while it should be delete-other-windows. Not sure if it's a typo or something else.

Yes, it's a typo.

delete-other-windows and spacemacs/toggle-maximize-buffer don't delete treemacs window

They do for me. Maybe it's an emacs version difference? I'm using a self-compiled emacs 26. If I run the following code in emacs -Q I'm left with only 1 window:

  (display-buffer-in-side-window
   (get-buffer-create "TEST") '((side . left)))

  (call-interactively 'delete-other-windows)

mode line is empty

I didn't do much with the mode line. If spaceline is installed it look like in the screenshot in the readme. If not it should just say "Treemacs".

I don't understand why you can't use just motion state for treemacs buffer. Is it just for having a different cursor shape?

It's been a while since I wrote that. IIRC using a custom state was the easiest way to use a custom cursor. The cursor is supposed to be invisible - treemacs navigation and selection works on lines, having a cursor is no use here, so I use hl-line mode instead.

Collaborator

bmag commented Aug 8, 2017

Well, I'm testing on Emacs 25.2. There are some significant changes in Emacs 26 regarding side windows, but I haven't been following them for the past several months. Apparently display-buffer-in-side-window doesn't install the delete-window parameter anymore, instead you need to set the no-delete-other-window parameter:

(display-buffer-in-side-window buffer
			       '((side . left)
				 (window-parameters . ((no-delete-other-window . t)))))

It's been a while since I wrote that. IIRC using a custom state was the easiest way to use a custom cursor. The cursor is supposed to be invisible - treemacs navigation and selection works on lines, having a cursor is no use here, so I use hl-line mode instead.

Hmm, I think that really is the easiest solution. For it to work in Spacemacs we will need to add an entry to spacemacs-evil-cursors (it's very easy)

Just to jump in - I haven't seen this mentioned anywhere, but will Treemacs support multiple frames?

Neotree right now is borderline unusable (I need to file an issue about it when I get time), because it creates a dedicated window that interrupts window restoration if a layout switches. Judging by the issue tracker, this seems to regularly re-occur when Neotree gets updated, and the developer has shown little interest in adding a permanent fix.

I'm very interested in Treemacs as someone who heavily relies on a file browser during development, and @Alexander-Miller the new features and fixes you're talking about all sound pretty great. But they won't be useful to me if I can't use layouts.

If we're planning on switching the default package, or even endorsing a new layer, my vote is we should take advantage of the chance to fix that issue, and to do it in a way that we're relatively confident it won't get reintroduced as a regression later (ie. not by putting a hack or fix on Spacemacs' side).

It's possible the issue is more complicated than that, and I really don't want to derail anything. But I don't think the improvements will matter if a layer can't be used alongside other core parts of Spacemacs' functionality.

instead you need to set the no-delete-other-window parameter:

Yes, that works. Thanks.

For it to work in Spacemacs we will need to add an entry to spacemacs-evil-cursors (it's very easy)

Why? You already said that the cursor is invisible on your end so everything is working as intended.

but will Treemacs support multiple frames?

Depends on how you define support. As of now there is only one global treemacs buffer, which all frames share. It's limited, but it works.

because it creates a dedicated window that interrupts window restoration if a layout switches

I don't use layouts very much myself, but having quickly switched between the treemacs project and std layout everything seems to work fine. Eyebrowse works fine, too (neotree not working with it is why I started treemacs). There seems to be some occasional bug where after switching desktops the wrong line seems to be marked in treemacs, I'll be looking into that eventually.

It's possible the issue is more complicated than that,

Many people want something that's not restricted to one buffer, so I'll do it eventually, but yes, it is very, very complicated. I'll need redo all variable access, from the current view root to the cache that stores which files and tags are open, for various configurations like the current layout, which is what you want, or the current frame, which others have mentioned on reddit. That touches just about every single feature and part of the code base. Then there's issues like having filewatch-mode figure out which buffer it needs to run its refresh in and whether it's even possible to assign treemacs buffers to frames/layouts after they're restored with desktop-mode. I'll figure something out eventually, but it really is a lot of work.

Mind you it'd be a lot easier if I had someone else to test treemacs with the changes I'm making. There's only so much ground unit test and using treemacs treemacs myself can cover. I've become quite cautious about pushing features that arent't properly tested after the premature imenu integration release left treemacs broken for a few days, so if you don't mind pulling a dev version from git and telling me when things go wrong the features you want will come around much faster.

I've also an idea for something of a stopgap feature that I think I should be able to get done quickly: adding other parts of the file system to the treemacs view. Integrate it with projectile and you can add all the projects you're working with into the same buffer, similar to an eclipse workspace. Only restriction is that you must insert distinct parts of the file system. If a file can appear twice its absolute path is no longer a unique key and that would break many parts of treemacs.

But they won't be useful to me if I can't use layouts.

You can use them. Other than the one treemacs buffer limitation treemacs should get along with layouts just fine. And if it doesn't, it's a bug, and you should open a ticket so I can fix it. In fact I encourage you to go ahead and try, there's a use-package example in the readme that you should be able to use as is, and I much prefer that treemacs' layout compatibility be something other than "it works for me".

my vote is we should take advantage of the chance to fix that issue, and to do it in a way that we're relatively confident it won't get reintroduced as a regression later (ie. not by putting a hack or fix on Spacemacs' side).

The way you talk about treemacs and layers sounds like you're under the impression there's some bug here. There is not, or at least there should not be (hence my wanting you to try). Treemacs cannot display one file tree per layout or frame, but that's not a bug, that's the lack of a feature.

There also won't be hacks or fixes on the side of spacemacs. I have every intention of writing a thin layer, adding nothing, but what's needing for a smooth integration. If there's some nontrivial work needed to make things happen, it should be done in treemacs.

danShumway commented Aug 8, 2017 edited

Thanks for the information! If Treemacs won't clobber my layouts, then I am 100% on board with this.

I don't actually have any particular wish or need for multiple-frame support except that it seems to have come up in the past as a reason for Neotree bugs. Same with my comment about fixes on the side of Spacemacs.

I'm a barely functioning lisp developer, so take everything I say with a grain of salt, but bugs like #5696, #7446, and #7343 seem to indicate that something with Neotree's approach, be it lack of multiple-frame support, or (more likely) its reliance on dedicated windows, or something else entirely makes these types of regressions fairly common, and they're by far the biggest, most regular annoyances I have using Spacemacs.

Mind you it'd be a lot easier if I had someone else to test treemacs with the changes I'm making.

I haven't tested Treemacs (yet), so I've got no expectations at all about bugs - just interjecting that bugs, not features, are the biggest reason I'd want a new layer, and the biggest reason I'd vote to replace Neotree as a default package. I'll check out your dev branch and try it out. Right now, layouts are completely unusable for me if I have Neotree open, so if Treemacs works even marginally better, I will happily switch 😃.

be it lack of multiple-frame support, or (more likely) its reliance on dedicated windows, or something else entirely makes these types of regressions fairly common

It's the dedicated windows that are the cause of issues, dedicated in the sense that there's a global variable that is supposed to be always bound to the global neotree window. Switch layouts and back and the window is still there and looks the same, but it's no longer the same object in memory. That's when neotree starts acting weird. Treemacs' approach is much simpler, if I need its window I just ask for the window of the buffer named treemacs--buffer-name. Given there's at most one such buffer that always works.

I'll check out your dev branch and try it out.

Not right now! If a dev branch is even around on github it's terribly outdated (I need to clean up my origin remote). I was making plans for the future, when I'd start working on frame/layout support. For now I want to cleanup current issues and get this layer working. If you're really up for it I'll open up an issue in treemacs and we'll do the testing there.

@bmag Switching eyebrowse desktops swallows custom window parameters like no delete-other-window. I opened a ticket, but it's a nontrivial issue, so treemacs needs to deal with it on its own. Persp is affected as well.

Does the same happen to you on emacs25? To check you just need to see if delete-other-windows keeps sparing treemacs after switching eyebrowse desktops or layouts and back.

Collaborator

bmag commented Aug 12, 2017

Yes, the same happens with emacs25, however the window parameters are different: delete-window, window-side and window-slot. You can use window-persistent-parameters. IIRC emacs26 added window-side and window-slot to the default value of window-persistent-parameters (in emacs25 they're not persistent by default)

On a side note, don't you have a release version of Emacs (25.x or 24x) in addition to emacs26?

On a side note, don't you have a release version of Emacs (25.x or 24x) in addition to emacs26?

Nope, I always want to play with the new toys in master, like threads and native line numbers. Now that I need it I'll get myself an emacs25 installtion.

You can use window-persistent-parameters.

That's one way to do it, bit I'm weary of changing global windows parameters while juggling multiple emacs versions. Who knows what behaviour this might affect elsewhere? It's probably safer for treemacs to grab hooks after layout and desktop switches and manually restore those parameters.

I've pushed a first version of the layer now, if anyone wants to take a look. Still need to write a readme and put up some config options.

Collaborator

bmag commented Aug 12, 2017

juggling multiple emacs versions

It's not really juggling:

;; window params that need to be persistent
(if (version< emacs-version "26")
    '(delete-window window-side window-slot)
  '(no-delete-other-windows))

I'm weary of changing global windows parameters

From a package point of view I'd be weary as well, but probably do it anyway. From Spacemacs point of view, it's totally fine to set these parameters in the layer.

It's probably safer for treemacs to grab hooks after layout and desktop switches and manually restore those parameters.

That's also a viable solution. If you prefer that one then ok (as long as it works, of course).

I'll test the layer in the coming days.

I'd like some input on a design decision, specifically whether treemacs should even be a side window.

So far making it one has been nothing but trouble. When I made it work for my own emacs26 setup it threw my magit window management in a loop, it needs non-trivial workarounds for eyebrowse and persp and maybe others and with issues like Alexander-Miller/treemacs#17 the treemacs layer won't be so thin after all, since it'll need to wrap functionality from at least one other layer.

So, the question is: does some see a reason to keep treemacs as a side window that's good enough to make up for all the trouble?

And I'm back. Been busy with features and bugs and what not, but now it's back to the layering business.

I've now added a readme and several config options to the layer, and integrated the new projectile package.

Now I would like some critique w.r.t completeness & clarity of the readme, breadth of the config vars and the keybinds. I've put the projectile stuff onto SPC f p/P now, since SPC p t is taken by projectile-find-tag. There's also still the question of where a good place for treemacs-delete-other-windows would be.

@Alexander-Miller would it be possible to put the layer into its own repo? I'd love to give it a try and provide feedback, but its a little difficult to include the layer when its nested inside a full spacemacs checkout.

I just tried it out and I must say it's got a better flow than neotree(which I really dislike so it's a low bar) I will play around with it but yeah it was a bit of a pain to clone and copy the layer so it would be nice to have as a separate layer until it's merged into core.

Pushing the layer around now is overkill, I'm done with preparations (including losing the emacs25 requirement on pfuture, if anyone wants to try that), so I'll be moving on to making a MR soon. Relatively soon at least, advertising on reddit gave me quite a few new user and issues to work through and fix.

At any rate, a mostly painless way for you to try the layer is go to your spacemacs installation, git remote add my fork, git fetch, and then git cherry-pick the specific treemacs commit.

Pull request is a go now, so this issue is done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment