stevearc / resession.nvim

A replacement for mksession with a better API
MIT License
175 stars 13 forks source link

[FR/RFC] Extensions that don't have windows opened for them #40

Closed willothy closed 9 months ago

willothy commented 9 months ago

I'm attempting to create an extension for edgy.nvim, to save the layout state and reopen windows using edgy views' open function / command via string.dump and loadstring. The problem is that edgy, and plugins such as sidebars, create their own windows so when they are opened by the extension, a window is created in the layout and the extension creates its own window. This leaves blank / extra windows in the layout after loading.

It would be nice to have extensions that are able to save state of individual windows, but not open new windows when the session is loaded so that the extension can create its own window. It is currently possible to get this behavior by manually closing the original window in load_win, but that leads to some janky behavior and flicker.

I'd be interested in contributing a PR for this, but figured I'd open an issue to discuss it first.

stevearc commented 9 months ago

I'm not sure how much sense it makes to try to make an extension for edgy, since all that plugin is doing is placing windows. Wouldn't it make more sense to create extensions for the plugins that own those windows? It doesn't seem like saving and loading a window layout should have anything to do with edgy. I don't personally use edgy though, so I might be missing something here.

As for saving/loading state of individual windows, this is the intended behavior for load_win. The problem is that the only way for us to restore the same window layout is to do a bunch of manual split/vsplit calls to create the layout, then go window-by-window and restore the contents of each. If a plugin supports opening/restoring in a targeted window, then it works great (e.g. aerial).

If the plugin doesn't support that (e.g. the quickfix extension), then we have to do some "open a new window and close the original" hacks. I would be interested if you have any ideas for how to avoid these kinds of hacks, but AFAICT the only way to do that would be to make PRs upstream to these sidebar plugins to add an API to open them in a pre-existing window.

willothy commented 9 months ago

I'm not sure how much sense it makes to try to make an extension for edgy,

The reason why I think an extension for edgy is a good idea is that it provides a unified interface for functions to open a particular view/filetype, and the data for a window can be retrieved with edgy.get_win(winid) which makes saving the layout state easy and allows supporting a wide range of plugins all at once. It would essentially allow users to add support for any plugin they want. The only data that really needs to be saved is the filetype, and the dumped bytecode for the open function.

As for saving/loading state of individual windows, this is the intended behavior for load_win

Yeah, it definitely makes things more difficult if plugins don't support that, which unfortunately many don't.

I would be interested if you have any ideas for how to avoid these kinds of hacks

I'm not sure, but would it be reasonable to save some windows separately in a list of "detached" windows, and essentially ignore them in the layout? Then, when a session is loaded these can be restored after the main layout is setup. That way the layout can be reconstructed and then the plugins that manage their own layout can do so without breaking things. This may lead to some nondeterminism for plugins that don't always attach to the edge of the screen, so it would pretty much entirely be for sidebar-type plugins (and maybe floating windows? not sure if they're supported atm).

Another issue I ran into with my extension is that when using :q when focusing the "main" buffer (whatever file I'm editing), nvim quits and the only layout restored is the sidebar window. This is because resession saves the state just before closing (in my config), so if the sidebar auto-closes nvim when it's the last window then the main buffer won't be open anymore when the layout is saved. :qa works around this, and I'm not sure there's any other solution, but it's worth noting.

stevearc commented 9 months ago

So you're saying that edgy effectively provides this "open plugin in window" API for any plugin? If that's the case then that definitely sounds interesting. We could use that as some sort of a "fallback" extension if there is no specific extension configured for a window.

I'm not sure, but would it be reasonable to save some windows separately in a list of "detached" windows, and essentially ignore them in the layout?

I don't think this will work in all cases. If a window it a botright split or topleft vsplit or similar, then maybe it could work. But it requires a lot of special case code and doesn't handle cases like below where we would want to restore sidebar A

┌───────────┌─────────────┐
│           │             │
│           │             │
│  file1    │  file2      │
│           │             │
│           │             │
┌───────────┌───┐─────────┘
│           │   │         │
│  file3    │ A │ file4   │
│           │   │         │
│           │   │         │
└───────────└───┘─────────┘

I'm not keen to make the window restore logic more complex, but doubly so if it doesn't even handle general cases.

and maybe floating windows? not sure if they're supported atm

Floating windows are not supported. I don't think they're worth supporting, as it would be a decent amount of work and most of the time their contents are ephemeral and don't need to be saved.

Another issue I ran into with my extension is that when using :q when focusing the "main" buffer (whatever file I'm editing), nvim quits and the only layout restored is the sidebar window.

No easy fix for this, I'm afraid. Resession is intentionally trying to be just a dumb API with minimal magic. It saves what you tell it to save, so if you call save when there's just a sidebar window open, it does what you tell it. You could try putting some logic in BufHidden or similar to catch when you're closing the last "main" buffer and will only have a sidebar left, and save at that point while also keeping track of it so you can avoid saving when vim exits. But fundamentally you'll either have to do something like :qa or you'll have to "look ahead" and detect when your sidebar plugin will quit vim in the future, and save before that happens.

willothy commented 9 months ago

So you're saying that edgy effectively provides this "open plugin in window" API for any plugin?

It's part of the config so users would have to add their own open functions, but yes.

The open function (or vim command) can be retrieved easily, we can just ignore windows without open functions. My is_win_supported function looks something like this:

local win = require("edgy").get_win(winid)
-- win.view is the thing we actually want, `open` is the user-provided open fn
-- not every view is given an `open` function, so we need to check for it to
-- see if the window can be restored.
return win ~= nil and win.view.open ~= nil

But it requires a lot of special case code and doesn't handle cases like below where we would want to restore sidebar A ... I'm not keen to make the window restore logic more complex, but doubly so if it doesn't even handle general cases.

That definitely makes sense. You're right, in retrospect that wouldn't be a very good solution. Maybe a better solution would be saving the entire edgy state in the extension using on_save and restoring with on_post_load? That way no windows will be created and no changes would be needed. I'll try that out and update here.

Floating windows are not supported. I don't think they're worth supporting, as it would be a decent amount of work and most of the time their contents are ephemeral and don't need to be saved.

I agree, definitely not necessary. I don't think it's worth any extra code but if development of some other feature made it work then that wouldn't be a bad thing.

You could try putting some logic in BufHidden or similar to catch when you're closing the last "main" buffer and will only have a sidebar left

That's what I was thinking, but have opted for just using :qa. I just disabled the feature that closes edgy windows automatically, so I have to use :qa. With that setting enabled, saving a session with only an edgy window open will lock the session into a "loop" where nvim quits as soon as it's opened because edgy sees there are no other windows and tries to quit immediately. Disabling the feature ensures this will never happen. The lookahead idea is interesting though, I may try that. Maybe a WinClosed autocmd that loops through other windows and saves the session if it finds only edgy windows would work, and I could remove my VimLeavePre autocmd.

willothy commented 9 months ago

I don't think there's actually any change needed after discussing this, if I just save state that's not specific to a window in the layout then it seems to work fine, as long as I use :qa.

Thanks for your input! I'll link my edgy extension when it's more complete. I would be happy to submit it as a PR if you want, or I can just keep it in my config :)

willothy commented 9 months ago

Edgy extension works pretty well as long as :qa is used. I'm attempting to figure out the lookahead to allow :q to be used as well.

Restoring DAP UI:

https://github.com/stevearc/resession.nvim/assets/38540736/0c49648a-6c1a-4a61-808e-2be4c486045f

Restoring ToggleTerm terminal and Trouble diagnostics:

https://github.com/stevearc/resession.nvim/assets/38540736/e5be9481-0a3b-483d-95e3-ece27d4296ab

stevearc commented 9 months ago

Ahhh interesting. This does seem like a neat extension! I'd be interested in bringing this upstream to either edgy or resession.

There a few things I'd want to hammer out first. Not all of these need to be fixed, but there should at least be some conscious decisions made.

willothy commented 9 months ago

Awesome, sounds good!

It only works for the current tabpage

Good to know, since I've only tested with tab-scoped sessions. Definitely should work with multiple tabpages, that may be doable by saving tabpage number with each window so it can be reopened in the appropriate tab, or ignored if the tab is not reopened?

Seems like disabling the animation didn't work (from the ToggleTerm + Trouble video)

Yeah, still ironing that out, but I believe that animation was from mini.animate and focus.nvim and the edgy.nvim animation is actually disabled here. I've since added a toggle of focus.nvim to my edgy config (not to the extension itself) and it seems much smoother.

The second arg to vim.notify should be vim.log.levels.WARN

For sure, I'll make that change. I've never been sure which it's supposed to be as I see people using both strings and vim.log.levels and both seem to work haha.

We need some way to make sure it doesn't conflict with existing extensions (i.e. aerial has a specific extension, so it should be managed by that even if there is an edgy config for it)

Not sure about this one yet, but maybe extensions could be configured with a priority? We could just table.sort before looping over available extensions and give edgy a zero priority. Maybe something simpler would be more ideal though, I'll keep thinking on this.

willothy commented 9 months ago

Also, I have since gotten :q to work via a (somewhat hacky) QuitPre autocmd.

Edit: This still seems to hang nvim sometimes, working on fixing it right now. Will add the code back when it works properly.

Edit 2: The hang was related to another resession extension, not this one. Here's the working autocmd:

vim.api.nvim_create_autocmd("QuitPre", {
  group = au,
  callback = function()
    local curwin = vim.api.nvim_get_current_win()
    local wins = vim.api.nvim_list_wins()
    local has_normal = false
    local is_last = true

    for i = 1, #wins do
      local win = wins[i]
      if
        vim.api.nvim_win_get_config(win).zindex == nil
        and require("edgy").get_win(win) == nil
      then
        has_normal = true
        if win ~= curwin then
          is_last = false
          break
        end
      end
    end
    if has_normal then
      local name = vim.fs.basename(vim.fn.getcwd(-1) --[[@as string]])
      resession.save_tab(name, { notify = false })
      if is_last then
        vim.cmd("qa!")
      end
    end
  end,
})
stevearc commented 9 months ago

Definitely should work with multiple tabpages

I agree. It will probably work if you just iterate over the tabpages when you save & restore, but will just need to double check that it works

the edgy.nvim animation is actually disabled here

cool, that seems fine then

I see people using both strings and vim.log.levels and both seem to work

The default implementation uses vim.log.levels

https://github.com/neovim/neovim/blob/e46f5aab895f333d23d6adf490e13177c8d3c1d8/runtime/lua/vim/_editor.lua#L595-L603

But nvim-notify accepts either

https://github.com/rcarriga/nvim-notify/blob/e4a2022f4fec2d5ebc79afa612f96d8b11c627b3/lua/notify/service/notification.lua#L22-L24

Not sure about this one yet, but maybe extensions could be configured with a priority?

I think a priority would work. Seems like a pretty reasonable solution to me

willothy commented 9 months ago

It will probably work if you just iterate over the tabpages when you save & restore

My only concern with this is that for tab-scoped sessions, I'm not sure there's a way to get the tab number of the currently saving session, so that may need some special handling.

cool, that seems fine then

Actually not sure that's fully working, I'm still figuring it out but it may depend on whether edgy is loaded before resession or not. Current implementation is usually smooth but every now and then you can see the plugin windows open before edgy "takes control" of them.

But nvim-notify accepts either

Gotcha, that's why both are working for me. Good to know, I'll always use vim.log in the future. Thanks!

I think a priority would work. Seems like a pretty reasonable solution to me

Cool, sounds good. I'll start working on a PR for this soon :)

stevearc commented 9 months ago

I'm not sure there's a way to get the tab number of the currently saving session, so that may need some special handling

I think nvim_list_tabpages returns them in order, so if you iterate over them you get the tabnr, tabpage.

willothy commented 9 months ago

Yes, but the current tabpage may not always be the currently saving tabpage, and on_save is not passed the tabnr of the currently saving session. So not sure how I'd determine which of those results should be saved. Non-scoped sessions should be all good though. I'll try to come up with a way to handle this.

willothy commented 9 months ago

Edgy extension now works with multiple tabpages:

https://github.com/stevearc/resession.nvim/assets/38540736/ae14e185-e824-461d-ae25-d0338e45ad16