feat(hooks): add support for close hooks

[sstallion]

This PR adds support for before/after close hooks, which is is especially useful for cleaning up after before/after load hooks. At the moment, I am using these hooks to implement per-session LSP configuration tied to a session. If there's interest in this, I'd be happy to contribute a plugin.

Example usage:

require("possession").setup {
  hooks = {
    before_load = function(name, user_data)
      require("lspconfig")["lua_ls"].setup {
        settings = {
          Lua = {
            runtime = {
              version = "LuaJIT"
            },
            diagnostics = {
              globals = {
                "vim"
              }
            },
            workspace = {
              checkThirdParty = false,
              library = vim.api.nvim_get_runtime_file("", true)
            }
          }
        }
      }
      vim.diagnostic.enable()
      return true
    end,
    after_close = function(name)
      vim.diagnostic.disable()
      require("lspconfig")["lua_ls"].setup {}
    end
  }
}

Thanks for a great plugin!

[jedrzejboczar]

Hi, thanks for PR.

This seems like a good idea to have close-hooks but I wonder how to best approach it. For example, with the current implementation from this PR the close-hooks won't fire when someone loads new session without invoking "close" for the current one. We would probably need to run M.close here if a session is currently loaded. However this would always call utils.delete_all_buffers which will be slightly different than the default, so M.close might have to be modified not to call utils.delete_all_buffers when we invoke it from M.load.

Is the example you provided the full use-case? As it seems to enable the nvim-specific LSP settings when loading any session. Or are you filtering based on names or some session data? In the case of LSP you might also try on_new_config hook for lspconfig, I was using something like:

lsp.lua_ls.setup {
    cmd = { 'lua-language-server' },
    settings = { Lua = lua_settings },
    root_dir = get_nvim_lua_root_dir,
    on_new_config = function(new_config, new_root_dir)
        -- Remove plugin dirs from library when not developing anything in neovim dirs
        if is_nvim_dir(new_root_dir) then
            new_config.settings = vim.tbl_deep_extend('force', new_config.settings or {},
                {Lua = {workspace = {library = initial_libs}}})
        end
    end
}

But whatever suits you best. I think that close-hooks might be a good idea, just want to explore how to best approach it and what might be the use cases.

[jedrzejboczar]

Just for completeness, here code extracted from my current Lua LSP config (not sure if it will work out-of-the-box), which adds library directories "dynamically" when opening files. I ended up with something like this because I have a lot of plugins and lua_ls becomes too slow if all are added. Maybe this could be useful to somebody.

Code

local luadev = pcall(require, 'neodev')

local lua_nvim = {
    main = vim.loop.fs_realpath(vim.fn.stdpath('config')),
    -- reduce amount of checks by considering only those directories that contain lua/
    dirs = vim.tbl_map(lspconfig_util.path.dirname, vim.api.nvim_get_runtime_file('lua/', true)),
    types = luadev and require('neodev.config').types(),
    runtime = vim.fn.expand('$VIMRUNTIME'),
}

-- Default implementation from lspconfig
local default_lua_root_dir = require('lspconfig.server_configurations.lua_ls').default_config.root_dir
local is_nvim_dir = function(path)
    for _, dir in ipairs(lua_nvim.dirs) do
        local candidates = { dir, vim.loop.fs_realpath(dir) }
        for _, candidate_dir in ipairs(candidates) do
            if candidate_dir == path or is_descendant(candidate_dir, path) then
                return true
            end
        end
    end
    return false
end

local nvim_lua_root_dir = function(fname)
    if is_nvim_dir(fname) then
        return lua_nvim.main
    end
    -- Fallback to default root dir resolving
    -- Fix: sometimes it does not add '/' at the end, sometimes it does, which unnecessarily spawns 2 servers.
    local path = default_lua_root_dir(fname)
    if path and path:sub(#path, #path) == '/' then
        path = path:sub(1, #path - 1)
    end
    return path
end


local lua_settings = {
    runtime = {
        version = 'LuaJIT',
    },
    diagnostics = {
        globals = {'vim'},
        unusedLocalExclude = { '_*' },
        -- -- FIXME: Ignoring doesn't seem to work at all
        -- ignoredFiles = 'Disable', -- never diagnose ignored files
    },
    workspace = {
        library = {},
        checkThirdParty = false,
    },
    hint = {
        enable = true,
        arrayIndex = 'Disable', -- no need to know idexes in an array...
    },
    completion = {
        callSnippet = 'Replace',
    },
    telemetry = { enable = false },
}

-- Add library directories to settings only after a file from this directory has been opened
vim.api.nvim_create_autocmd('LspAttach', {
    group = vim.api.nvim_create_augroup('PluginsLspConfig'),
    callback = function(args)
        local client = vim.lsp.get_client_by_id(args.data.client_id)
        if not client or client.config.name ~= 'lua_ls' then
            return
        end

        local libs = vim.tbl_get(client, 'config', 'settings', 'Lua', 'workspace', 'library')
        if not libs then
            vim.notify(client.config.name .. ': settings.Lua.workspace.library does not exist', vim.log.levels.WARN)
            return
        end

        local added = {}
        local add_lib = function(dir)
            if not vim.tbl_contains(libs, dir) and not vim.tbl_contains(added, dir) then
                changed = true
                table.insert(libs, dir)
                table.insert(added, dir)
            end
        end

        local file = vim.api.nvim_buf_get_name(args.buf)
        for _, dir in ipairs(lua_nvim.dirs) do
            if is_descendant(dir, file) then
                add_lib(dir)
            end
        end

        -- Setup common nvim dirs
        if not vim.tbl_contains(libs, lua_nvim.main) then
            add_lib(lua_nvim.main)
        end
        if lua_nvim.types and not vim.tbl_contains(libs, lua_nvim.types) then
            add_lib(lua_nvim.types)
        end
        if not vim.tbl_contains(libs, lua_nvim.runtime) then -- for all the api functions
            add_lib(lua_nvim.runtime)
        end

        -- Setup plenary.busted
        if vim.endswith(vim.fn.fnamemodify(file, ':r:t'), '_spec') then
            for _, dir in ipairs(lua_nvim.dirs) do
                if vim.fn.fnamemodify(dir, ':t') == 'plenary.nvim' then
                    add_lib(dir .. '/lua/plenary/busted.lua')
                    add_lib(dir .. '/lua/luassert')
                    break
                end
            end
        end

        if #added > 0 then
            local paths = vim.tbl_map(function(dir)
                return vim.fn.fnamemodify(dir, ':~')
            end, added)
            vim.notify(('%s: added libraries:\n%s'):format(client.config.name, table.concat(paths, '\n')))
            client.notify('workspace/didChangeConfiguration', { settings = client.config.settings })
        end   
    end
})

lsp.lua_ls.setup {
    cmd = {'lua-language-server'},
    settings = { Lua = lua_settings },
    root_dir = nvim_lua_root_dir,
}

[sstallion]

But whatever suits you best. I think that close-hooks might be a good idea, just want to explore how to best approach it and what might be the use cases.

Sounds good! I've noticed this as well and as hard as I try, I almost always forget to close the session before opening a new one. Work has been a little hectic lately, but I should be able to dig into it this week.

[sstallion]

Is the example you provided the full use-case? As it seems to enable the nvim-specific LSP settings when loading any session

It's not - it seemed to be the simplest way to express an idea 😁

In reality, I have a module in my nvim config named lspsession that is called by possession.nvim when a session is loaded or closed. It will load a chunk from stdpath("data") which will in turn return a table that can be used to enable or disable a given session that looks similar to the above.

In the above case, I have an nvim session, which loads stdpath("data") / lspsession / nvim.lua:

return {
  diagnostic = true,
  enable = function(name)
    require("lspsession")["lua_ls"].extend {
      settings = {
        Lua = {
          runtime = {
            version = "LuaJIT"
          },
          diagnostics = {
            globals = {"vim"}
          },
          workspace = {
            library = vim.api.nvim_get_runtime_file("", true)
          }
        }
      }
    }
  end,
  disable = function(name)
    require("lspsession")["lua_ls"].restore()
  end
}

[sstallion]

I've added the missing call to M.close(); I experimented with removing utils.delete_all_buffers, however this didn't feel quite right to me. It was nice having the previous session's buffers closed for me automatically when opening a new session.

[jedrzejboczar]

Hi, sorry for the delay but I don't have much time lately.

As for the changes, I was thinking about it and I understand that it works well for you, but I don't think it's a good idea to have M.close call utils.delete_all_buffers during M.load. The reasons are:

• it means that the plugin delete_buffers (with force=false) will now be called always regardless of user configuration
• user before_load hooks may break, as they may depend on the fact that old buffers are there (hard to say what people use before_load for)
• if someone is using plugins.delete_buffers=true then this will break their configuration, e.g. let's say you have a terminal buffer open and you want to load new session, currently this would make the "delete_buffer" plugin forcefully close the buffer, but with this PR it will first call M.close which will fail to delete "buffer with unsaved changes"

At the current state of the plugin and my limited time I would really like to avoid such breaking changes, that's why I'm hesitating with this PR. I don't see a good solution yet, I'm open for suggestions. One thing that comes to my mind would be to actually make the breaking change, but add notification for users about the change, but this might require adding quite a bit of code for handling this and e.g. some config option to "acknowledge" the change, which creates additional problems :/