Skip to content

feat: allow switching into hover window via ctrl+w w for long translasion#7

Open
code-dom wants to merge 3 commits intonoir4y:mainfrom
code-dom:main
Open

feat: allow switching into hover window via ctrl+w w for long translasion#7
code-dom wants to merge 3 commits intonoir4y:mainfrom
code-dom:main

Conversation

@code-dom
Copy link
Copy Markdown

Title:
feat: allow switching into hover window via ctrl+w w

Body:

Problem

The hover window was closed immediately when entering it via ctrl+w w,
making it impossible to scroll or read long translations.

Changes

  • Replace BufLeave/WinLeave autocmds with WinEnter to fix premature hover closure
  • Bind q/<Esc> to close the hover window when focused
  • Remove BufWipeout autocmd whose logic incorrectly closed hover when an unrelated buffer was deleted

…tions

    1. Replace BufLeave/WinLeave with WinEnter to trigger hover cleanup.
    2. Remove BufWipeout autocmd as closing hover on unrelated buffer deletion is wrong — hover should persist when cursor has not moved.
    3. Bind q/<Esc> to close the hover window on creation.
@code-dom code-dom changed the title feat: allow switching into hover window via ctrl+w w for long transla… feat: allow switching into hover window via ctrl+w w for long translasion Mar 30, 2026
Copy link
Copy Markdown
Owner

@noir4y noir4y left a comment

Choose a reason for hiding this comment

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

@code-dom
Thanks for working on this — the overall direction makes sense, especially the change to avoid immediately closing when focus enters the hover window, and the addition of q / <Esc> to close it.

That said, I think there is still a timer cleanup issue here.

Previously, cleanup_timer(args.buf) ran on BufLeave / WinLeave, so the timer attached to the source buffer was cleaned up when leaving that buffer. After this change, cleanup runs on WinEnter using the current buffer, which means a timer from the previous buffer can remain alive and fire later.

Since the timer callback calls parser.get_text_at_cursor() without an explicit bufnr, that stale timer may end up reading from the current window/buffer instead of the original source buffer.

I think the safer approach would be to:

  • keep the CursorMoved guard for the hover buffer
  • keep BufLeave / WinLeave for timer cleanup only
  • use WinEnter only for hover close behavior

There also don’t seem to be regression tests covering this timer/cleanup behavior, so I don’t think this is safe to merge as-is.

@code-dom
Copy link
Copy Markdown
Author

Thanks for the review — I’ve addressed the cleanup issue in the latest commits.

  • WinEnter now only handles hover closing
  • BufLeave / WinLeave are used for timer cleanup again
  • the hover-buffer CursorMoved guard is kept

I also added regression tests for stale timer cleanup and hover-window switching behavior, and the autocmd tests pass locally.

Copy link
Copy Markdown
Owner

@noir4y noir4y left a comment

Choose a reason for hiding this comment

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

Thanks for the follow-up. I still see two hover lifecycle regressions in the current PR head that should be fixed before merge:

  • Closing hover only on WinEnter misses same-window buffer changes such as :bnext / :edit. Those fire buffer events but not WinEnter, so the existing hover can remain visible over the next buffer.
  • Auto-hover is still active while focused in the hover buffer. CursorHold can create a timer for the hover buffer, and CursorMoved returns before cleaning that timer, so it can survive during interactive use and fire unexpectedly later.

Concretely, this needs:

  • a buffer-based close path for non-hover buffers, not just WinEnter
  • preventing CursorHold from creating timers for the hover buffer, or cleaning those timers before the hover-buffer CursorMoved guard returns

Both cases should have regression tests before merge.

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.

2 participants