Better mappings handling

[tjdevries]

No description provided.

[tjdevries]

Fixes: nvim-telescope/telescope-file-browser.nvim#5 (comment)

[Conni2461]

rebased branch. i hoped this would fix my problems. sadly it didnt 😭

[tjdevries]

Which problems didn't it fix @Conni2461 ?

[Conni2461]

I merged #1143 and it fixed t1.a + t2.b but it broke :enhance #1482 #1485
So at some point of the debugging process i thought maybe its something with how we map actions so i rebased this branch and tried if it magically fixed my issues. It was late for me, i just wanted to sleep 😆

Eventually i said i cant leave it like that and reverted #1486 went to bed, realized why its broken, got up again (i know) and i think i fixed it in #1487: 9297883

I think this PR is good. Lets merge it :)

[fdschmidt93]

From what I see, this indeed partially fixes the issue from telescope-file-browser :) We should merge it.

As a heads up, we currently have another whacky issue with extensions.

    local fb_actions = require 'telescope'.extensions.file_browser.actions
    local imaps = {
      ['<C-f>'] = { '<Right>',  type = 'command' },
    }
    require'telescope'.setup {
      extensions = {
        file_browser = { mappings = { i = imaps, }, },
      },
    }

doesn't work because

telescope.nvim/lua/telescope/_extensions/init.lua

Lines 7 to 24 in 1c57cc6

	extensions.manager = setmetatable({}, {
	__index = function(t, k)
	-- See if this extension exists.
	local ok, ext = pcall(require, "telescope._extensions." .. k)
	if not ok then
	error("This extension doesn't exist or is not installed: " .. k .. "\n" .. ext)
	end
	if ext.setup then
	ext.setup(extensions._config[k] or {}, require("telescope.config").values)
	end
	t[k] = ext.exports or {}
	extensions._health[k] = ext.health
	return t[k]
	end,
	})

setups up the extension essentially in local fb_actions = require 'telescope'.extensions.file_browser.actions as the extensions.manager is triggered before the actual setup.

I'll make a PR to fix that eventually.

[tjdevries]

So I can just merge as is? haha @Conni2461 you should follow my lead and take some vacation 😆

[fdschmidt93]

Ah, I found one case that is not yet fixed.

Take builtin.git_branches for instance, which attaches mappings on picker:find.

telescope.nvim/lua/telescope/builtin/git.lua

Lines 276 to 296 in 1c57cc6

	attach_mappings = function(_, map)
	actions.select_default:replace(actions.git_checkout)
	map("i", "<c-t>", actions.git_track_branch)
	map("n", "<c-t>", actions.git_track_branch)
	map("i", "<c-r>", actions.git_rebase_branch)
	map("n", "<c-r>", actions.git_rebase_branch)
	map("i", "<c-a>", actions.git_create_branch)
	map("n", "<c-a>", actions.git_create_branch)
	map("i", "<c-s>", actions.git_switch_branch)
	map("n", "<c-s>", actions.git_switch_branch)
	map("i", "<c-d>", actions.git_delete_branch)
	map("n", "<c-d>", actions.git_delete_branch)
	map("i", "<c-y>", actions.git_merge_branch)
	map("n", "<c-y>", actions.git_merge_branch)
	return true
	end,

Neither of the below cases seem to work:

require "telescope".setup {
  pickers = {
    git_branches = {
      -- attach_mappings = function(b, map)
      --   map("i", "<C-a>", false)
      --   return true
      -- end,
      mappings = {
        i = {
          ["<C-a>"] = function()
            print "test"
          end,
          ["<C-a>"] = false,
        },
      },
    },
  },
}

I think the cleanest solution would be to move the default mappings for an individual pickers to config.lua? Otherwise tricky to fix. I'm pretty much doing that in telescope-file-browser and then it can be cleanly overridden.

[tjdevries]

The expected case is that the mapping does not happen?

[tjdevries]

(In the example you sent, the code wouldn't run -- so I'm just confused what part is supposed to be happening)

[tjdevries]

OK, I pushed something that should work I think @fdschmidt93

[tjdevries]

(Did this fix everything?)

[fdschmidt93]

No, I'm afraid it doesn't seem to be the case. What I'd expect that I can unmap or map my own function onto something that is bound via attach_mappings (just to reconfirm, the below examples work perfectly fine with global mappings like <C-l>). Does that clarify? 😅

(In the example you sent, the code wouldn't run -- so I'm just confused what part is supposed to be happening)

I'm also a bit confused and sorry for the late response.

require "telescope".setup {
  pickers = {
    git_branches = {
      mappings = {
        i = {
          -- I was maybe confusing: this is just one thing that should work
          ["<C-a>"] = function()
            print "test"
          end,
          --and this another
          ["<C-a>"] = false,
        },
      },
    },
  },
}

[Conni2461]

This will be a nightmare to rebase. Can we not do this again with these unrelated changes. And do smaller PRs in the future, if its possible? (I know something like fps would not be possible, in a small PR)

I'll try to split it up into smaller parts and get the good stuff merged. @tjdevries Ill ping you in the smaller PR for the stuff that still needs to be resolved regarding fdschmidts last comment

[Conni2461]

@tjdevries this is rebased and the unrelated changes are cherry picked here #1850 I'll write some docs for entry_index and merge that over there.

Can you finish up this PR? (Can we even more simplify it with vim.keymap). I am currently preping a bump to 0.7 (#1851)

[Conni2461]

I pull out the document changes for 0.1.0 The rest will be addressed at a later point in time