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

Fix resize/preview toggles of the cursor layout #2718

Merged
merged 1 commit into from
Sep 27, 2023
Merged

Fix resize/preview toggles of the cursor layout #2718

merged 1 commit into from
Sep 27, 2023

Conversation

yorickpeterse
Copy link
Contributor

@yorickpeterse yorickpeterse commented Sep 26, 2023

Description

The cursor layout uses winline() and wincol() to calculate the cursor position. Both these functions operate on the currently active window. The first time the calculations are performed, that happens to be the window active before showing the Telescope window. However, if the editor is then resized or the preview window is toggled, the active window changes. The result is that recalculating the position is then done using the wrong window, resulting in the Telescope window moving around in an erratic manner.

To fix this, we have to scope the winline() and wincol() calls to the original window ID.

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

To test, you'll need to use the cursor layout. If you then show Telescope, followed by either resizing the editor window or toggling the preview panel on/off, you'll notice the Telescope window moving around in an erratic manner. This is no longer the case with these changes.

Configuration:

  • Neovim version (nvim --version): 0.9.2
  • Operating system and version: Fedora Silverblue 38

Checklist:

  • My code follows the style guidelines of this project (stylua)
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation (lua annotations)

@yorickpeterse
Copy link
Contributor Author

Before:

Screencast.from.2023-09-26.01-17-52.webm

After:

Screencast.from.2023-09-26.02-30-03.webm

@jamestrew jamestrew self-assigned this Sep 26, 2023
@max397574
Copy link
Contributor

just wondering what picker is this?

@yorickpeterse
Copy link
Contributor Author

@max397574 It's a custom one I'm working on for my dotfiles.

The cursor layout uses winline() and wincol() to calculate the cursor
position. Both these functions operate on the currently active window.
The first time the calculations are performed, that happens to be the
window active before showing the Telescope window. However, if the
editor is then resized or the preview window is toggled, the active
window changes. The result is that recalculating the position is then
done using the wrong window, resulting in the Telescope window moving
around in an erratic manner.

To fix this, we have to scope the winline() and wincol() calls to the
original window ID.
@yorickpeterse
Copy link
Contributor Author

It seems GitHub isn't updating the pull request for some reason, but either way the changes are indeed in my fork (https://github.com/yorickpeterse/telescope.nvim/commit/4ef39b23eb955cd49b6f129a12bc2ef3aaeddfe4). Perhaps GitHub is a bit slow with handling pushes at the moment...

Copy link
Contributor

@jamestrew jamestrew left a comment

Choose a reason for hiding this comment

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

looks good thanks!

@jamestrew jamestrew merged commit 5c91b85 into nvim-telescope:master Sep 27, 2023
6 checks passed
@Conni2461
Copy link
Member

@jamestrew again please update commit message on merge, if they are not conventional commits already.

@jamestrew
Copy link
Contributor

@jamestrew again please update commit message on merge, if they are not conventional commits already.

Ahh sorry I misunderstood your original request. I thought you meant clear and sensible commit messages, not specifically conventional commits 😅

I'll make sure to follow the conventional commit format in my future PRs.

Thoughts on adding a conventional commit linting ci?

@Conni2461
Copy link
Member

Thoughts on adding a conventional commit linting ci?

this doesnt help, because we squash the commits, and if a PR has multiple commits, it defaults to PR title, which again can be none conventional and then it hits master which is already too late. if you scroll though the commit history you notice that the current system works good enough. Once a new person with commit rights comes in, some slip through but after a quick reminder its goes back to mostly conventional.

I'll make sure to follow the conventional commit format in my future PRs.

Your PRs were fine, it was some PRs you merge. On "rebase and merge" you then can edit the message to be conventional.

Ahh sorry I misunderstood your original request

no harm done :D maybe i explained it wrong, currently cant recall 😆

njhoffman pushed a commit to njhoffman/telescope.nvim that referenced this pull request Sep 30, 2023
The cursor layout uses winline() and wincol() to calculate the cursor
position. Both these functions operate on the currently active window.
The first time the calculations are performed, that happens to be the
window active before showing the Telescope window. However, if the
editor is then resized or the preview window is toggled, the active
window changes. The result is that recalculating the position is then
done using the wrong window, resulting in the Telescope window moving
around in an erratic manner.

To fix this, we have to scope the winline() and wincol() calls to the
original window ID.
Conni2461 pushed a commit that referenced this pull request Oct 11, 2023
The cursor layout uses winline() and wincol() to calculate the cursor
position. Both these functions operate on the currently active window.
The first time the calculations are performed, that happens to be the
window active before showing the Telescope window. However, if the
editor is then resized or the preview window is toggled, the active
window changes. The result is that recalculating the position is then
done using the wrong window, resulting in the Telescope window moving
around in an erratic manner.

To fix this, we have to scope the winline() and wincol() calls to the
original window ID.

(cherry picked from commit 5c91b85)
rameshsanth pushed a commit to rameshsanth/telescope.nvim that referenced this pull request Nov 17, 2023
The cursor layout uses winline() and wincol() to calculate the cursor
position. Both these functions operate on the currently active window.
The first time the calculations are performed, that happens to be the
window active before showing the Telescope window. However, if the
editor is then resized or the preview window is toggled, the active
window changes. The result is that recalculating the position is then
done using the wrong window, resulting in the Telescope window moving
around in an erratic manner.

To fix this, we have to scope the winline() and wincol() calls to the
original window ID.
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.

4 participants