axkirillov / hbac.nvim

Heuristic buffer auto-close
MIT License
197 stars 10 forks source link

feat(opts)!: restructure mappings and set default picker opts in setup #14

Closed al-ce closed 11 months ago

al-ce commented 1 year ago

BREAKING CHANGES: In this PR we would restructure the mappings table in in the setup configuration to follow Telescope's structure ( ["key"] = function) for action mappings.

All actions are named functions so they'll have names in the Telescope 'which key' popup.

Changes:

Example user config (extends default in hbac.config):

hbac.setup({
  telescope = {
    sort_lastused = false,  -- overrides default
    layout_strategy = "vertical",  -- additional opts
    prompt_prefix = "Hbac! ",
    mappings = {  -- extends default mappings
      i = {
        ["<CR>"] = telescope_actions.select_drop,
        ["<C-CR>"] = telescope_actions.select_default,
        ["<M-z>"] = function() print("hello") end,
      },
    },
    pin_picker = {
      -- etc.
    },
  },
})

example keymap for picker (extends user config):

vim.keymap.set("n", ",ht", function()
  require("hbac").telescope(
    {
      ignore_current_buffer = true,
      file_ignore_patterns = { ".json" },
      mappings = {
        i = {
          ["<CR>"] = telescope_actions.select_default,  -- overrides the mapping in user setup config
        },
      },
    }
  )
end, { desc = "Hbac: pin picker", noremap = true })

Addresses #11 and builds on #9 and #12 (partially addressed by f041ebabcd27aaadaebaccc9390edb56d06af731)

axkirillov commented 1 year ago

Looks good to me. I'll test it on my setup.

axkirillov commented 1 year ago

Did you decide to make the current buffer hidden by default after all?

al-ce commented 1 year ago

Did you decide to make the current buffer hidden by default after all?

Sorry I left that comment hanging! I think I'm on the same page with what you were saying, that we should not hide the current buffer (right?). Currently the defaults are

sort_mru = true,
sort_lastused = true,
selection_strategy = "row",

so the current buffer should be in the first result, selector/cursor should be on the second result which is the previously used buffer, and the rest are sorted by most recently used (if I've implemented the options correctly)

axkirillov commented 1 year ago

Did you decide to make the current buffer hidden by default after all?

Sorry I left that comment hanging! I think I'm on the same page with what you were saying, that we should not hide the current buffer (right?). Currently the defaults are

sort_mru = true,
sort_lastused = true,
selection_strategy = "row",

so the current buffer should be in the first result, selector/cursor should be on the second result which is the previously used buffer, and the rest are sorted by most recently used (if I've implemented the options correctly)

huh, weird, I think I have the current buffer hidden can you check? my setup function is empy, so I should be getting all default behaviours

al-ce commented 1 year ago

can you check? my setup function is empy, so I should be getting all default behaviours

I'm guilty of creating a million branches here, are you using the default_opts branch on my fork instead of the picker_opts branch from the last PR? (sorry I did that, I changed the merge message of the last PR and had a merge conflict so I created this new one) Otherwise I'll debug

axkirillov commented 1 year ago

can you check? my setup function is empy, so I should be getting all default behaviours

I'm guilty of creating a million branches here, are you using the default_opts branch on my fork instead of the picker_opts branch from the last PR? (sorry I did that, I changed the merge message of the last PR and had a merge conflict so I created this new one) Otherwise I'll debug

Yeah, I'm using this branch

    {
        'al-ce/hbac.nvim',
        config = function()
            require("hbac").setup()
        end,
        branch = "default_opts"
    },
al-ce commented 1 year ago

Does passing opts to the setup call or to the telescope function call change the behavior or does it throw any errors? Like

  {
    'al-ce/hbac.nvim',
    config = function()
      require("hbac").setup(
      {
        telescope = {
          sort_lastused = true,
        },
        }
    )

      vim.keymap.set("n", ",ht", function()
        require("hbac").telescope({
            sort_mru = true,
            mappings = {
              i = {
                ["<cr>"] = telescope_actions.select_drop,
              },
            },
          })
      end)

    end,
    branch = "default_opts"
  }

Just confirming whether it's just the default behavior or if any opts are registering

axkirillov commented 1 year ago

Does passing opts to the setup call or to the telescope function call change the behavior or does it throw any errors? Like

  {
    'al-ce/hbac.nvim',
    config = function()
      require("hbac").setup(
      {
        telescope = {
          sort_lastused = true,
        },
        }
    )

      vim.keymap.set("n", ",ht", function()
        require("hbac").telescope({
            sort_mru = true,
            mappings = {
              i = {
                ["<cr>"] = telescope_actions.select_drop,
              },
            },
          })
      end)

    end,
    branch = "default_opts"
  }

Just confirming whether it's just the default behavior or if any opts are registering

For me, even if I paste this configuration, the current buffer is hidden.

al-ce commented 12 months ago

Sorry for lack of updates, I've been traveling to visit family and I've been trying to figure this out in some downtime. I've tried a few different setups and I'm getting the behavior I'm expecting so I think it might be some mistake I made when I merged the last PR into this branch or I'm making some bad assumption about my machine. Sorry for the hold up and I'll keep trying, hope all is well

axkirillov commented 12 months ago

Sorry! I just realized I was passing my own opts 🤦🏻

vim.keymap.set("n", "<leader>b", function()
    local default_opts = {
        show_all_buffers = true,
        sort_mru = true,
        sort_lastused = true,
        ignore_current_buffer = true,
        cwd_only = false,
        cwd = nil,
    }
    require("hbac").telescope(default_opts)
axkirillov commented 12 months ago

I changed it to this and it works as expected

vim.keymap.set("n", "<leader>b", ":Hbac telescope<CR>", ns)
al-ce commented 12 months ago

I changed it to this and it works as expected

Ok that's good! I integrated your suggestions above and made a small refactor in the config module to not repeat the lines that assign the mappings, since they're the same for both. Is this ok?

local action_mappings = {
    ["<M-c>"] = actions.close_unpinned,
    ["<M-x>"] = actions.delete_buffer,
    ["<M-a>"] = actions.pin_all,
    ["<M-u>"] = actions.unpin_all,
    ["<M-y>"] = actions.toggle_pin,
}
local defaults = {
    telescope = {
        mappings = {
            n = action_mappings,
            i = action_mappings,
                }
        -- etc
}
axkirillov commented 11 months ago

Looks good and works for me.

al-ce commented 11 months ago

Hey, I'm back home from my travels. I'm good with the PR as it is if you're happy with it and ready to merge it, I think we've addressed the open issues it references. Will you do the merge into main when you're ready? I didn't want to push the new mapping config structure without knowing your procedures for announcements or versioning. Thanks again for the collaboration!