telescope.nvim icon indicating copy to clipboard operation
telescope.nvim copied to clipboard

layout_strategy=vertical & layout_config.anchor='S' leaves one line

Open m-deg opened this issue 1 year ago • 7 comments

Description

This is driving me crazy. It would be awesome if i can vertical stack prompt, previewer, results and snap it to the bottom of the page. I don't want to use ivy because it doesn't allow vertical stacking. Take a look at the screenshot below. That line "27: opts.window = opts.window or {}" is the actual buffer i am on. Telescope is on top of that line, and status line is on the bottom. I expected vertical layout strategy with anchor=S to leave no line between telescope and statusline (this is what ivy does)

image

Neovim version

deg2@DESKTOP-S6KAR7R:~/.install/nvim/nvim-linux64/bin$ ./nvim --version
NVIM v0.9.5
Build type: Release
LuaJIT 2.1.1692716794

   system vimrc file: "$VIM/sysinit.vim"
  fall-back for $VIM: "/__w/neovim/neovim/build/nvim.AppDir/usr/share/nvim"

Run :checkhealth for more info

Operating system and version

Debian 11

Telescope version / branch / rev

telescope master

checkhealth telescope

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

Checking for required plugins ~
- OK plenary installed.
- WARNING nvim-treesitter not found. (Required for `:Telescope treesitter`.)

Checking external dependencies ~
- OK rg: found ripgrep 13.0.0
- WARNING fd: not found. Install [sharkdp/fd](https://github.com/sharkdp/fd) for extended capabilities

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

Telescope Extension: `fzf` ~
- OK lib working as expected
- OK file_sorter correctly configured
- OK generic_sorter correctly configured

Telescope Extension: `live_grep_args` ~
- No healthcheck provided

Steps to reproduce

opts.layout_strategy = "vertical" opts.border = true opts.layout_config.anchor = "S" opts.layout_config.prompt_position = "bottom" opts.layout_config.width = {padding = 0} opts.layout_config.height = 0.5 opts.layout_config.preview_height = 0.5

Expected behavior

Just like ivy theme, vertical layout strategy that is anchored to bottom of the window should be snapped right on top of statusline

Actual behavior

There is one annoying line between statusline and telescope

Minimal config

  opts.layout_strategy = "vertical"
  opts.border = true
  opts.layout_config.anchor = "S"
  opts.layout_config.prompt_position = "bottom"
  opts.layout_config.width = {padding = 0}
  opts.layout_config.height = 0.5
  opts.layout_config.preview_height = 0.5

m-deg avatar Jan 06 '24 21:01 m-deg

It looks to be by design (with tests) https://github.com/nvim-telescope/telescope.nvim/blob/87e92ea31b2b61d45ad044cf7b2d9b66dad2a618/lua/telescope/config/resolve.lua#L268

That - 1 being the difference maker. I'm a little shaky on adding more config options to layout_config but I suppose it's possible to make this configurable. PR welcome.

jamestrew avatar Jan 07 '24 04:01 jamestrew

Thanks @jamestrew , would it make sense to create a config like anchor_delta, with default value 0, and apply like below?

pos[2] = math.ceil((max_lines - p_height) / 2) - 1 + anchor_delta

I can then pass 1 to negate the -1. What do you think?

m-deg avatar Jan 07 '24 15:01 m-deg

I'm thinking maybe something more like anchor_padding that's a default of 1. Then just do something like

  if anchor:find "W" then
    pos[1] = math.ceil((p_width - max_columns) / 2) + anchor_padding
  elseif anchor:find "E" then
    pos[1] = math.ceil((max_columns - p_width) / 2) - anchor_padding
  end
  if anchor:find "N" then
    pos[2] = math.ceil((p_height - max_lines) / 2) + anchor_padding
  elseif anchor:find "S" then
    pos[2] = math.ceil((max_lines - p_height) / 2) - anchor_padding
  end

jamestrew avatar Jan 13 '24 03:01 jamestrew

I am interested in contributing this as my first pr/contribution, looks straightforward to me. Is there anyone working at it at the moment?

Weyaaron avatar Mar 07 '24 10:03 Weyaaron

I am not working on this @Weyaaron . Looking forward to this getting fixed tho :)

m-deg avatar Mar 17 '24 16:03 m-deg

I started work on it, I got some promising results and aim to open a pr this week :)

Weyaaron avatar Mar 19 '24 09:03 Weyaaron

Real-Life got in my way, but I am back on track. I got a quick question on my progress. After some digging, my current approach is adding the parameter to the resolve_anchor_pos function like this: (Somewhat cut short for brevity, it is analogous to the snippet done above).

resolver.resolve_anchor_pos = function(anchor, p_width, p_height, max_columns, max_lines, anchor_padding)
  anchor = anchor:upper()
  local pos = { 0, 0 }
  if anchor == "CENTER" then
    return pos
  end
  if anchor:find "W" then
    pos[1] = math.ceil((p_width - max_columns) / 2) + anchor_padding
  elseif anchor:find "E" then

This forces me to update all the tests. Is it sufficient to just call all the tests with 1 to mimic the old behavior, or should I add new tests?

Weyaaron avatar Apr 07 '24 15:04 Weyaaron

looks good, thanks Weyaaron. @jamestrew do you think you can take a look at PR as well?

m-deg avatar May 21 '24 03:05 m-deg