buffers being restored in the wrong windows

diamond-lizard commented 3 years ago

When I use bookmark-bmenu-this-window on a bookmark created using burly-bookmark-windows, the restored window layout is as it was when the bookmark was created, but the buffers it restores are placed in the wrong windows.

The minimal test setup I have is as follows:

Three files:

/tmp/left.txt containing: "left"
/tmp/upper-right.txt containing: "upper right"
/tmp/lower-right.txt containing: "lower right"

I place them in to three windows as seen in this screenshot, taken right before I used burly-bookmark-windows.

Here is a screenshot taken right after I used bookmark-bmenu-this-window on the bookmark created by burly-bookmark-windows.

Here is my bookmarks file.

I am using emacs 27.1 on Gentoo Linux, with Burly and map.el 2.1 installed manually.

alphapapa commented 3 years ago

Yes, thanks, I know. It's a very early version. :)

clemera commented 3 years ago

After some research I discovered that:

(setq wc
      (window-state-get (frame-root-window (selected-frame))
                        'writable))

Can be used to save a window configuration. Once the buffers are restored one can use:

(window-state-put wc
                  (frame-root-window (selected-frame))
                  'safe)

to restore the windows. That might be an alternative to the current approach used by burly-revive. I haven't looked how burly uses it, maybe there are reasons I'm unaware of which make it preferable?

alphapapa commented 3 years ago

@clemera I borrowed the code from revive.el because I knew that current-window-configuration returns an unprintable value. I was planning to replace its code with something using more modern Emacs features if possible.

I was going to try iterating over window-tree, but it looks like window-state-get/put works just as well, and I can use a window parameter to store the windows' URLs before calling window-state-get to serialize them. Seems to work great, and lets me delete the entire burly-revive library.

FYI, since burly-revive is under a different license, and this package is at such an early version, I'll probably reset this repo to completely remove the code from revive.el; so if you're tracking this repo, you'll need to hard reset it.

Thanks for letting me know about those functions. You probably saved me a lot of time going down lesser paths. :)

alphapapa commented 3 years ago

Well, on closer examination, it doesn't quite do the job: if a saved window's buffer doesn't exist anymore, its window is not restored by window-state-put. :(

clemera commented 3 years ago

Yeah, I noticed that as well. The buffers of the windows need to be saved separately and restored before calling window-state-put.

alphapapa commented 3 years ago

Yep, that took a little coding, but pcase helps with the destructuring. I should be pushing this change soon.

alphapapa commented 3 years ago

@clemera I think it's working, but before pushing to master, I pushed to this branch: https://github.com/alphapapa/burly.el/tree/wip/window-state-get Please let me know how it works for you. (It won't restore bookmarks created before this commit.)

clemera commented 3 years ago

Thanks, seems to work great! One thing to note is that when the bookmark is opened from the bookmark list buffer the current window will keep the bookmark list buffer.

alphapapa commented 3 years ago

Yeah, I'm banging my head against that one right now. That problem is caused by bookmark--jump-via, which calls this after handling the bookmark:

(save-current-buffer
    (funcall display-function (current-buffer)))

Maybe I can hack something by binding bookmark-after-jump-hook...

clemera commented 3 years ago

I see...don't have any good ideas how to solve that, there also some other commands which are unclear how to handle like bookmark-bmenu-other-window. Maybe it would be acceptable to use advices to handle burly bookmarks differently.

alphapapa commented 3 years ago

I seem to have fixed it by using a function in bookmark-after-jump-hook to restore the window state after bookmark--jump-via changes it.

I don't know about bookmark-bmenu-other-window or other commands. Apparently the bookmark system is not designed for single buffers in single windows, not calling commands that affect the window configuration. But this will probably be good enough.

Please pull, reset the branch (sorry for the trouble), and let me know how it works for you.

alphapapa commented 3 years ago

bookmark-bmenu-other-window seems to work as well.

clemera commented 3 years ago

Please pull, reset the branch (sorry for the trouble), and let me know how it works for you.

No worries, helping testing the functionality I want to use is the least I can do :) I can confirm that it works.

alphapapa commented 3 years ago

Thanks, I'll probably push this to master soon then.

alphapapa commented 3 years ago

Done! Thanks for your help.

clemera commented 3 years ago

Thank you, too, I think this is a very useful package!

alphapapa commented 3 years ago

Yes, it's something I wanted for a long time. It always seemed like a missing feature in the workspace/perspective packages. And since it leverages bookmarks, it doesn't even require much code.

clemera commented 3 years ago

Yes, I think the same, a "simple" way to save the current window config as a bookmark is exactly what I was looking for.

alphapapa / burly.el

buffers being restored in the wrong windows #3