Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add cursor layout #878

Merged
merged 18 commits into from
Jul 16, 2021
Merged

Conversation

Luxed
Copy link
Contributor

@Luxed Luxed commented May 31, 2021

Closes #496.

Here's how it looks like when space is not an issue, my cursor is on the first "t" of the "test" variable name:
image

Here's how it looks like when space is restricted in the bottom right (cursor at the same place):
image

What happens when there is not enough room is that the floating window gets put above the cursor line and gets fixed to the right (with 1 extra space to give it some room).

@dhruvmanila
Copy link
Contributor

Nice job! Are you calculating the corner the window should be anchored to? Like in floating windows either one of the corners (NW, NE, SW, SE) is made an anchor point to the cursor. This is calculated using the space available for the telescope window w.r.t. the main Neovim window. You might want to take a look at vim.lsp.util.make_floating_popup_options()

Also, I think the layout updates should be put on hold for now until #823 is finalized and merged.

@Luxed
Copy link
Contributor Author

Luxed commented May 31, 2021

Nice job! Are you calculating the corner the window should be anchored to? Like in floating windows either one of the corners (NW, NE, SW, SE) is made an anchor point to the cursor.

I am calculating the position so that the cursor is above the NW corner.
If the size of the floating window would be bigger than the available width, I then anchor it to the right of the screen (leaving 1 character of space).
If the size of the floating window would be bigger than the available height, I then anchor the bottom of the floating window above the cursor line.

Maybe it would be better for the window to always anchor to the NW, NE and SE points?

You might want to take a look at vim.lsp.util.make_floating_popup_options()

I remember looking into it, but it was so long ago that I'll look into it again.

Also, I think the layout updates should be put on hold for now until #823 is finalized and merged.

Of course, I didn't see this was ongoing.

@l-kershaw
Copy link
Contributor

l-kershaw commented Jul 12, 2021

@Luxed just a heads up that #823 has now been merged (or at least its successor #922 has), so this PR is no longer blocked.

If you need any help with the new layout_config stuff, feel free to ping me 🙂

@Luxed
Copy link
Contributor Author

Luxed commented Jul 13, 2021

@Luxed just a heads up that #823 has now been merged (or at least its successor #922 has), so this PR is no longer blocked.

If you need any help with the new layout_config stuff, feel free to ping me slightly_smiling_face

It ended up being a lot easier than I expected. Everything is working as it did before now.

Copy link
Contributor

@l-kershaw l-kershaw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One small nitpick/question regarding preview_width, but overall looks very good 🙂

lua/telescope/pickers/layout_strategies.lua Outdated Show resolved Hide resolved
Copy link
Contributor

@l-kershaw l-kershaw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I'm happy with this now 👍

I'll run this PR for a little bit and if I don't find any problems I'll merge this later today/tomorrow.

@l-kershaw l-kershaw merged commit b13306e into nvim-telescope:master Jul 16, 2021
@mawkler
Copy link

mawkler commented Dec 10, 2021

This looks awesome. How do I tell vim.lsp.buf.code_action() to use this layout?

@gegoune
Copy link

gegoune commented Dec 10, 2021

@Melkster https://github.com/nvim-telescope/telescope-ui-select.nvim

@mawkler
Copy link

mawkler commented Dec 10, 2021

@gegoune Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat: layout relative to current cursor position
5 participants