nvim-telescope / telescope.nvim

Find, Filter, Preview, Pick. All lua, all the time.
MIT License
15.95k stars 838 forks source link

overriding select_default with select_drop breakes actions that replace `select_default` or do something special like man_pages/help_tags #2940

Open Giovanni0114 opened 8 months ago

Giovanni0114 commented 8 months ago

Description

as in title, when i'm trying to configure telescope to open in new buffer rather then in splitted window, it does not work

Neovim version

NVIM v0.9.5
Build type: Release
LuaJIT 2.1.1692716794

Operating system and version

Linux 6.7.4-arch1-1 # x86_64 GNU/Linux

Telescope version / branch / rev

branch master commit ba64547

checkhealth telescope

telescope: require("telescope.health").check()

Checking for required plugins ~
- OK plenary installed.
- OK nvim-treesitter installed.

Checking external dependencies ~
- OK rg: found ripgrep 14.1.0
- OK fd: found fd 9.0.0

===== Installed extensions ===== ~

Telescope Extension: `terms` ~
- No healthcheck provided

Telescope Extension: `themes` ~
- No healthcheck provided

Steps to reproduce

  1. set the mapping below to telescope config
    mappings = {
      i = { ["<CR>"] = require("telescope.actions").select_drop, },
    },
  2. run telescope command for configured picker (":lua require("telescope.builtin").man_pages()" for example)
  3. hit enter

Expected behavior

the selection will open in new buffer in current window

Actual behavior

nothing happend, selection is ignored

Minimal config

vim.cmd [[set runtimepath=$VIMRUNTIME]]
vim.cmd [[set packpath=/tmp/nvim/site]]
local package_root = '/tmp/nvim/site/pack'
local install_path = package_root .. '/packer/start/packer.nvim'
local function load_plugins()
  require('packer').startup {
    {
      'wbthomason/packer.nvim',
      {
        'nvim-telescope/telescope.nvim',
        requires = {
          'nvim-lua/plenary.nvim',
          { 'nvim-telescope/telescope-fzf-native.nvim', run = 'make' },
        },
      },
      -- ADD PLUGINS THAT ARE _NECESSARY_ FOR REPRODUCING THE ISSUE
    },
    config = {
      package_root = package_root,
      compile_path = install_path .. '/plugin/packer_compiled.lua',
      display = { non_interactive = true },
    },
  }
end

_G.load_config = function()
  require("telescope").setup {
    pickers = {
      man_pages = {
        mappings = {
          i = {
        ["<CR>"] = require("telescope.actions").select_drop, 
          },
        },
      },
    },
 }
require('telescope').load_extension('fzf')
  -- ADD INIT.LUA SETTINGS THAT ARE _NECESSARY_ FOR REPRODUCING THE ISSUE
end
if vim.fn.isdirectory(install_path) == 0 then
  print("Installing Telescope and dependencies.")
  vim.fn.system { 'git', 'clone', '--depth=1', 'https://github.com/wbthomason/packer.nvim', install_path }
end
load_plugins()
require('packer').sync()
vim.cmd [[autocmd User PackerComplete ++once echo "Ready!" | lua load_config()]]
jamestrew commented 8 months ago

Doing :drop Man printf (or any other man page) doesn't do anything for me so I'm not sure if there's something telescope can do about this unless I'm mistaking how to use drop with Man.

jamestrew commented 8 months ago

I believe select_drop works for other pickers like find_files though. Let me know if this is not the case for you.

Giovanni0114 commented 8 months ago

@jamestrew for find_files it works, indeed, but i'm rarely using it. I really just need an way to put Man page into buffer. if it hard to do with this feature i will find another way :smile: just the way it was documented didn't get me impression that this is right behaviour.

Giovanni0114 commented 8 months ago

sorry, missclick :smiling_face_with_tear:

jamestrew commented 8 months ago

Yeah I think :drop only works with files so your only option is to just use the other select actions for the man_pages picker.

require("telescope").setup({
  defaults = {
    mappings = {
      i = {
        ["<CR>"] = require("telescope.actions").select_drop,
      },
    },
  },
  pickers = {
    man_pages = {
      mappings = {
        i = {
          ["<CR>"] = require("telescope.actions").select_default,
        },
      },
    },
  },
})
Conni2461 commented 8 months ago

This is simpley just a bug https://github.com/nvim-telescope/telescope.nvim/pull/2944 fixes it.

Offloading this to the users isnt that great. can you confirm that this PR fixes this issue?

jamestrew commented 8 months ago

actually thinking about this more, silently making drop work as its non-drop counter part might be considered unintuitive or buggy.

I don't really have a strong opinion on this though but just a thought.

Giovanni0114 commented 8 months ago

This is simpley just a bug #2944 fixes it.

Offloading this to the users isnt that great. can you confirm that this PR fixes this issue?

It does not directly fix my problem, but certainly helps with no behavior at all. I think that intuitive would be that drop opens an help_tags and man_pages in a new buffer, on full screen, as drop would do with any other file. Now it shows up on splited screen, and even in the case of help_tags no new buffer is created.

This may be some problem to solve with Man and help commands, not with telescope itself.

PS: I discovered that Man command SOMETIMES opens man_tags page in fullscreen, and sometimes in splitted horizontal screen, but I cannot make reproducible scanario.

Giovanni0114 commented 8 months ago

actually thinking about this more, silently making drop work as its non-drop counter part might be considered unintuitive or buggy.

I don't really have a strong opinion on this though but just a thought.

I can see your point, but to be honest I prefer it rather than no behavior at all.

Conni2461 commented 8 months ago

i've been thinking about this some more and i think we (it was probably me) messed up when merging drop and tab drop, it doesnt work and breaks a lot of things and we might wanna go back to the drawing board for this one. Just saying but what about select_default:replace! Every picker that does this for there actions is broken once you do override <CR> with select_drop for default mappings and then drop select_default.

This includes basically everything that doesnt operate on files. thats not good.

we have actios.file_edit .... which never got adapted and should have been used for drop and tab_drop. select should have never been extended!

So yeah i think drawing board it is for actions again, maybe we/i can find a solution that satisfies everything (also mutliselection, which we need to tackle at some point anyway)