search

alphapapa / burly.el

Save and restore frames and windows with their buffers in Emacs
GNU General Public License v3.0
301 stars 14 forks source link

Emacs 27 tab bar support #16

Open alphapapa opened 3 years ago

alphapapa commented 3 years ago

See discussion and ideas at https://github.com/alphapapa/burly.el/issues/15#issuecomment-719995982

alphapapa commented 3 years ago

@tpeacock19 If you have time, would you mind testing this branch? https://github.com/alphapapa/burly.el/tree/wip/tab-bar I added a hook and a function that, when added to the hook, opens a new tab when a Burly windows bookmark is opened. (Just M-x customize-option on the option, select the function, and save the option.) It seems to work as desired on Emacs 27 with tab-bar-mode enabled.

alphapapa commented 3 years ago

The function now also selects an existing tab by that name rather than opening a new one by the same name.

tpeacock19 commented 3 years ago

Yep this works just like my workaround. I can't think of any issues with it.

tpeacock19 commented 3 years ago

Actually the only other potential suggestion would be around saving bookmarks.

Currently, I have a command that runs through all my open tabs and saves them as Burly bookmarks, overwriting the existing ones. I find it useful if I need to restart emacs. However, I understand that that may be catering too directly to tab-bar-mode users.

tpeacock19 commented 3 years ago

Also instead of opening a new issue i'll mention this here

burly-kill-buffer-url should have a prompt after the interactive keyword. Currently it just shows up as blank in the minibuffer until you begin to type for completion. Also the README states it copies the current buffer when in reality you must select it.

alphapapa commented 3 years ago

I'm not opposed to having burly-bookmark-tab and/or burly-bookmark-tabs commands. That sounds useful.

One concern I have with the current implementation would be this workflow:

  1. User installs Burly.
  2. User bookmarks a tab or all tabs.
  3. User opens that bookmark, expecting windows to be restored to one or more tabs. But since the user hasn't customized the hook to enable the new-tab functionality, the windows are not restored to tabs.

An idea for resolving that: Add a new tab and/or tabs bookmark type that would explicitly bookmark tabs rather than windows. This would probably also entail associated commands like burly-bookmark-tab, burly-bookmark-tabs, etc. Those commands would call the new-tab function explicitly. The hook could still be used to restore windows bookmarks to tabs, and/or maybe that could be done with a prefix argument to the bookmark-opening commands.

On the other hand, the only difference between a windows and a tabs bookmark would be that the latter always restores to a new tab. I'm not sure that minor difference justifies a new bookmark type. And what if a user wanted to restore a tabs bookmark to the current tab? I suppose a prefix argument could do that, but that starts to feel a little complicated to me.

WDYT? Thanks.

tpeacock19 commented 3 years ago

Add a new tab and/or tabs bookmark type that would explicitly bookmark tabs rather than windows.

This is essentially restricting the hook/function you've just created to a bookmark type, right? I imagine like the below: 

(pcase-exhaustive subtype
      ((or "bookmark" "file" "name") (pop-to-buffer (burly-url-buffer url)))
      ("frames" (burly--frameset-restore urlobj))
      ("windows" (burly--windows-set urlobj))
      ("tab"
       (run-hook-with-args 'burly-before-open-windows-hook :name name)
       (burly--windows-set urlobj)))

Its a good and explicit approach, which I'm always for. Additionally, I think you would want to avoid the prefix argument in order to continue working with base emacs bookmark commands.

It also may be worth having different prefixes for each bookmark type, so you would know what to expect re: window/frame/tab.

And what if a user wanted to restore a tabs bookmark to the current tab? I suppose a prefix argument could do that, but that starts to feel a little complicated to me.

Yeah that was one of the things I was worried may impact my workflow. However, I'm not sure what the limit on the number of tabs is, I quickly checked and got up to 15, but they are cheap to create and kill. The one similar issue I've run into is the very first tab/bookmark, since it is not opened with Burly it will remain named Scratch most likely. I had a check that would ask if I wanted to open a new tab if only one was open, but there's no smooth way to handle it.

alphapapa commented 3 years ago

This is essentially restricting the hook/function you've just created to a bookmark type, right?

Sort of, yeah, but maybe not exclusively. It seems like, if a user bookmarks a set of windows, he should still be able to restore them into a tab, even if it wasn't a "tab bookmark."

I suppose that, if we want to have a command to save and restore multiple tabs, an explicit tabs bookmark type will be necessary, because it will have to save and restore the tab name.

It starts to feel a little complicated, accounting for edge cases and such in a way that doesn't seem inflexible to the user. It will probably require some experimenting.

Please keep the great feedback coming!

tpeacock19 commented 3 years ago

I suppose that, if we want to have a command to save and restore multiple tabs, an explicit tabs bookmark type will be necessary, because it will have to save and restore the tab name.

I think I would push back against this really. One of the things I've enjoyed about using this workflow so far (vs desktop-mode) is that when I am opening a Burly bookmark I am getting a single and explicitly chosen workspace. There isn't any chance for me to be distracted by something in another tab I've left open in a previous setting.

I think the saving of all tabs is definitely warranted, but I wouldn't think restoring them as completely necessary. Plus it would probably reduce the complexity.

Sort of, yeah, but maybe not exclusively. It seems like, if a user bookmarks a set of windows, he should still be able to restore them into a tab, even if it wasn't a "tab bookmark."

This makes me think of using a prefix-argument for this specific instance, as you mentioned earlier.

alphapapa commented 3 years ago

I think the saving of all tabs is definitely warranted, but I wouldn't think restoring them as completely necessary.

So you mean that there should be a command to bookmark all open tabs, but rather than saving them to a single, multi-tab bookmark, it should save them in separate, single-tab bookmarks?

tpeacock19 commented 3 years ago

Yes that's right, I think it would suffice.

alphapapa commented 2 years ago

FYI, I've been working on burly-tabs-mode in this branch: https://github.com/alphapapa/burly.el/tree/wip/tabs It opens Burly bookmarks in a new tab-bar tab, and the command burly-reset-tab resets a tab to the state of the bookmark that opened it. It's working well, but it depends on a change in Emacs 28, so when that branch is merged, it might be necessary to depend on Emacs 28 (because conditionally supporting Emacs features tends to be messy).

tpeacock19 commented 2 years ago

I actually had just moved to that branch yesterday after having used the previous tab-bar branch. It works very well and is much simpler than our earlier discussions. I especially like the burly-reset-tab.

One suggestion is adding an optional timer to automatically save a list of, or all, burly tabs when in burly-tabs-mode. I am not certain if it is really necessary but it may be a nice option.

alphapapa commented 2 years ago

I actually had just moved to that branch yesterday after having used the previous tab-bar branch. It works very well and is much simpler than our earlier discussions. I especially like the burly-reset-tab.

Great. Please let me know if you have any feedback.

One suggestion is adding an optional timer to automatically save a list of, or all, burly tabs when in burly-tabs-mode. I am not certain if it is really necessary but it may be a nice option.

Would you explain that idea in a bit more detail? e.g. do you mean to bookmark all of the tabs into a single bookmark, or to bookmark each tab individually?

I don't think I'd use that, but I could see it being useful to others. Maybe it could be a mode like burly-auto-save-tabs-mode.

tpeacock19 commented 2 years ago

A single bookmark does sound interesting, but I was thinking individually. So there could be a variable that contains the symbol all or a list of bookmarks to be auto-saved. Or perhaps an auto-save key in the bookmark alist if that wouldn't conflict with anything.

Honestly, I had made a function to handle this myself and more often than not I prefer to explicitly save my tabs/workspaces. So I may just be creating a 'problem' to solve.

alphapapa commented 2 years ago

I don't think I'll implement anything like that soon, but maybe someone will experiment with the idea and come up with something generally useful that could be added someday.

ParetoOptimalDev commented 2 years ago

So you mean that there should be a command to bookmark all open tabs, but rather than saving them to a single, multi-tab bookmark, it should save them in separate, single-tab bookmarks?

I think this is fine so long as there is a "save all tabs" function and a "load all tabs" function.

My use-case is opening 3 tabs:

So upon closing emacs I'd like to be able to load those again with burly.

It might be useful to also have the granularity to only load one or more of those projects al a carte, but the most important functionality for my workflow would be the core tabs that are always the same being there.

I'll give the branch a try.

rswgnu commented 2 years ago

FYI, I've been working on burly-tabs-mode ... it might be necessary to depend on Emacs 28 (because conditionally supporting Emacs features tends to be messy).

I would really encourage you to go back to your latest release now and make it compatible with Emacs 27.1 since this is still in wide use and having two different versions for Emacs 27.1 and Emacs 28.1 makes it difficult for the user or to use your package in other packages that want compatibility with multiple versions of Emacs. I have been doing this for years and don't find it that hard or that ugly, just a few conditionals in key places, to get a much larger user base. I, for example, wanted to try your new release but was using Emacs 28.0.50 and found that was not compatible either. Your user base will expand if you allow for this I would think.

rswgnu commented 2 years ago

I looked at the tab-bar support in 0.3-pre and it looks like all you would have to do to make it Emacs 27.1 compatible is to implement the one line bodied function, tab-bar--current-tab-find, in terms of tab-bar--current-tab-index, so pretty minimal to gain a whole other version of backward compatibility.

alphapapa commented 2 years ago

@rswgnu Hi Bob, yeah, I didn't want to drop Emacs 27.1 support yet, but I wasn't able to get Emacs 27.1 installed on my system to test it with, so I just went with 28.1, which is the version I'm developing with. But if that's all that's needed, and you can vouch that it works, I'll be glad to change the version requirement back to 27.1.

rswgnu commented 2 years ago

If you make that change, and just send me the snippet, I’ll test it and verify for you.

Thanks,

-- Bob

On Jul 3, 2022, at 9:20 AM, Adam Porter @.***> wrote:

 @rswgnu Hi Bob, yeah, I didn't want to drop Emacs 27.1 support yet, but I wasn't able to get Emacs 27.1 installed on my system to test it with, so I just went with 28.1, which is the version I'm developing with. But if that's all that's needed, and you can vouch that it works, I'll be glad to change the version requirement back to 27.1.

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.

rswgnu commented 2 years ago

Hi Adam:

If you add this to burly-tabs.el after the (require 'tab-bar), then you'll be able to drop the Emacs version requirement back to 27.1. This will make burly load properly and the bookmark handling will all work. Tab-bars still won't display burly tabs but all the other functionality should work. Enabling burly-tabs-mode seems to just do nothing.

(unless (fboundp #'tab-bar--current-tab-find)
  (defun tab-bar--current-tab-find (&optional tabs frame)
    ;; Find the current tab as a pointer to its data structure.
    (assq 'current-tab (or tabs (funcall tab-bar-tabs-function frame)))))
alphapapa commented 2 years ago

Hi Bob,

I remembered another issue related to this: Juri Linkov pushed this fix to Emacs 28 last year: https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=b5395127626c1caaca8f80edcee77647e6eb9bb0 Without that, burly-tabs-mode won't work properly. So if we drop the requirement for Burly back to 27.1, we'll have to add a version check to burly-tabs-mode so that it refuses to activate on Emacs < 28.1. I guess that's a good tradeoff, so at least Emacs 27 users can use other features of Burly.

rswgnu commented 1 year ago

Yes, that sounds good. Thanks.

-- Bob

On Jul 14, 2022, at 4:49 PM, Adam Porter @.***> wrote:

 Hi Bob,

I remembered another issue related to this: Juri Linkov pushed this fix to Emacs 28 last year: https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=b5395127626c1caaca8f80edcee77647e6eb9bb0 Without that, burly-tabs-mode won't work properly. So if we drop the requirement for Burly back to 27.1, we'll have to add a version check to burly-tabs-mode so that it refuses to activate on Emacs < 28.1. I guess that's a good tradeoff, so at least Emacs 27 users can use other features of Burly.

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.