Allow for custom Telescope mappings (or provide a way to customize the <CR> action)

[griwes]

I like my Telescope <CR> action to be select_drop as opposed to just select, but this plugin currently does not give me a way to specify that. Ideally there'd be a way to just add custom mappings somewhere, but for my purpose just a way to select the <CR> action would be sufficient.

I'll currently work around this by hijacking attach_mappings to inject my own mappings, but I recognize that this is wildly fragile and would like to not need to do that.

Given that the customization for actions in this plugin is somewhat different from what I'm used to (i.e., there's a set of functions you assign mappings to, as opposed to assigning functions to mappings), I don't really know how (and if) you'd like to do this, but I can take a crack at a PR if a direction for how a user would specify this in the config is suggested.

[al-ce]

Hey there! You're right that the current customization scheme is unconventional. That structure would have helped me avoid some of the ugly repetitions in the attach_mappings module that I couldn't figure out at the time.

I took a shot at addressing this, but ran into some issues. Here's the branch where I'm experimenting, but this isn't meant to be the base for a PR and I'd rather see what you or @axkirillov 's approach would be from the start if he wants to forward with this. This would be breaking change for the config. (I also had to move some module calls around to avoid circular dependencies)

https://github.com/al-ce/hbac.nvim/commit/f3aa4ca50955ce46fc5159bebf3634eb050881f9 (EDIT: force pushed a better attempt)

In the original picker, I wanted to customize the display of the results (pin icon and filepath + previewer) and have the picker refresh after each action instead of closing it, so I wrapped each action in a function that calls the refresh. Those 'hbac' actions seem to work fine in this new branch.But if I try to add some mappings that attempt to close the picker (e.g. telescope.actions.select_drop), those actions are trying to close a picker by using the pre-refresh prompt_bufnr, so the action fails (picker is nil). For example:

local telescope_actions = require("telescope.actions")
local hello = function()
  vim.notify("hello")
end
hbac.setup({
  telescope = {
    mappings = {
      i = {
        ["<CR>"] = telescope_actions.select_drop,
        ["<C-CR>"] = telescope_actions.select_default,
        ["<M-z>"] = hello,
      },
    },
  },
})

We have a conventional structure in place, but those first two mappings won't work unless we remove the picker-refreshing logic. You could flip what's commented/not-commented in these lines and see. (a function like in the third mapping, that doesn't try to use an old bufnr is fine)

So idk how we might check to see if a customized action wants to close the picker vs. refreshing it and act accordingly. Maybe there's a better way to hook all this into a built in picker, but I've had a hard time fully understanding Telescope.

Anyway just wanted to throw some thoughts out there. Would you mind sharing your current workaround? Thanks for bringing this up!

[axkirillov]

Hi! Thank you for bringing this up! Would you be willing to open a PR?

[al-ce]

Mulled this over today and attempted three approaches, settled on this: https://github.com/al-ce/hbac.nvim/commit/7d40c71de0bec762bdaa6cdda4dd8e07853870a5 TLDR; remapping <CR> to select_drop and all the Hbac telescope actions should work in that fork's restruct_mappings branch

I had forgotten that part of the reason for the repetitive code I mentioned above was that we needed to create named functions. This way they would appear in the Telescope which-key pop up with their proper names instead of as <anonymous function>s. That's why we define them with a name that wraps around execute_telescope_function and add them to a table rather than just looping over the config table in attach_mappings. I looked back at our comment history in the original PR and noticed we talked about this, but it slipped my mind. I'm sure there's a cleaner way and I'm still eager for suggestions or a new approach from the ground up.

So I moved the attach_mappings function to hbac.telescope, left all these defined name actions where they were and renamed their module to 'actions' and assigned all the mappings in the setup config like this, which is a little verbose. Again, some variables that require modules were moved such that they're lazy loaded to prevent circular dependencies.

@griwes would appreciate any feedback and criticism! Thanks for spurring this

[axkirillov]

@griwes Hi! The features discussed here are now in the latest 2.0.0 release! Could you please update your config to use the master branch (also check out the readme, the way you set up mappings has been changed) and check that everything works for you? If yes, we can close the issue.

[griwes]

@axkirillov that works perfectly, thank you!