Reimplement hover window with floating window for Neovim 0.4.0 or later #767

rhysd · 2019-03-12T18:10:03Z

Hi,

I have experimentally reimplemented a hover window for LanguageClient#textDocument_hover() with Neovim's new floating window feature.

Here is a screencast with Neovim 0.4.0-dev and macOS 10.12:

This PR is still work in progress. I have not added any tests yet and I think there are some discussion points as follows:

There is no way to move cursor into float window so user cannot scroll the contents. Some mappings might be needed.
There is no way to close float window by user (No command like :pclose is not provided at this point). I added functionality to close the floating window automatically when cursor is moved. But I'm not sure this behavior makes all users happy
Some people may prefer preview window. Using floating window might be opt-in or opt-out
CursorLine is used for highlighting the floating window, but I'm not sure it is a good choice to modify background color slightly for every colorscheme. The highlight color might be customizable

autozimu · 2019-03-13T01:43:08Z

Awesome!

theHamsta · 2019-03-13T14:47:32Z

Nice work!

Currently, one should not switch buffers (e.g. ) when hover is visible. The float is only closed when moving the cursor in the original buffer.

rhysd · 2019-03-14T17:14:05Z

I added test for this feature (though I'm not sure it's sufficient).

There is no way to move cursor into float window so user cannot scroll the contents. Some mappings might be needed.

Regarding to this, I feel providing a mapping to move cursor into floating window would be enough. User can scroll the window and yank contents by moving cursor to the window.

Currently, one should not switch buffers (e.g. ) when hover is visible. The float is only closed when moving the cursor in the original buffer.

Thank you for the point. I think WinLeave should trigger closing float window as well. But it is also run when moving cursor into floating window. I think WinEnter would be available instead. Let me try.

since WinEnter cannot close floating hover when the current buffer switches to another buffer within the same window.

rhysd · 2019-03-16T19:50:30Z

I noticed WinEnter is not sufficient since it cannot close floating window when user switches current buffer to another buffer within the same window. I changed it to BufEnter and added one more test.

Note: the API was changed at neovim/neovim@96edbe7

autozimu · 2019-03-19T22:33:46Z

Is this ready for review?

rhysd · 2019-03-20T01:01:41Z

Let me add a mapping to move cursor into a floating window. Tomorrow is holiday in Japan. So I think I can have some progress on this branch tomorrow (I'm sorry for the delay).

autozimu · 2019-03-20T01:27:39Z

Not at all. Please take your pace.

rhysd · 2019-03-21T11:23:45Z

I finished the work.

Instead of adding a new mapping, I slightly changed the behavior of LanguageClient#textDocument_hover(). When opening a hover and calling it again before moving cursor, it moves the cursor into the floating window. Then user can scroll content and yank some text. :close or :quit in the window simply close the hover window.

I also added

g:LanguageClient_useFloatingHover
Documentation
More test for scenario moving the cursor into floating window

Now I believe this branch is ready for review 👍

lorenzo · 2019-03-21T11:43:15Z

I think having another function we can call to force the hover to be in a separate panel would be ideal. While most of the times I would use this floating window, there are also times that I want to leave the docs for a function open as I keep coding. Having both possibilities would make this the ideal feature

rhysd · 2019-03-21T11:50:33Z

Sounds good. It may be more useful than moving cursor into floating window but I'm not sure. Anyway, I'll try to implement the function to close floating window and reopen the content in preview window again.

to reopen floating hover in separate preview window

rhysd · 2019-03-21T12:43:20Z

I implemented LanguageClient#reopenHoverInSeparateWindow(). Could you check the behavior?

…penHoverInSeparateWindow

lorenzo · 2019-03-21T14:26:06Z

Why is it reopenIn and not just openIn?

rhysd · 2019-03-21T14:55:33Z

Why is it reopenIn and not just openIn?

Since it is only effective when floating window is already open. If openIn is more natural, I'll revert the commit. I'm not native English user. So the naming may be not proper. If my English usage is not proper, please let me know. It's very helpful for me (in terms of learning English).

JeanMertz · 2019-03-21T16:46:04Z

I think what @lorenzo was getting at is that it perhaps makes more sense to have two functions:

openHoverInFloatingWIndow – this opens the "hover" definition in the new floating window
openHoverInRegularWindow – this opens the "hover" definition in a regular window

But I'm not sure it makes sense to have this. There's already LanguageClient_useFloatingHover, so you could have your own function that first sets LanguageClient_useFloatingHover=0 and then calls LanguageClient#textDocument_hover(), to achieve the exact same result, I think?

Unless of course, I'm mistaken, and that's not what @lorenzo is talking about.

I like the reopenHoverInSeparateWindow though, it allows to quickly swap to a separate window if you're in a hover, and realise you want to keep this documentation open while you continue working.

lorenzo · 2019-03-21T17:09:14Z

The idea is not to have your own function, as few people actually write vimL. I don’t, and have been a bum user for years.

I did mean to have 2 separate function to open the panel in different places. For example, you could keep one panel open while using a floating window for something else as you write a long function.

Moving a floating window to a panel does make sense to me as well, I can see the use case.

rhysd · 2019-03-21T20:07:35Z

Thank you for further explanation. I got the point.

Two options come in my mind for this.

Add argument to hover function

I feel creating multiple functions is a bit too much. How about adding an argument for specifying window type?

LanguageClient#textDocument_hover(): Auto (use floating window as much as possible
LanguageClient#textDocument_hover('preview'): Open in preview window
LanguageClient#textDocument_hover('float'): Open in floating window

Separate two mappings can be defined such as nnoremap K :<C-u>call LanguageClient#textDocument_hover()<CR> and nnoremap gK :<C-u>call LanguageClient#textDocument_hover('preview'). It may be useful to add another type such as 'vsplit' (opens new normal window by :vsplit).

Pros:
- Simple and stateless
Cons:
- User needs to define two mappings

Add special window-local mapping

Defines buffer local (floating window local) mapping p to reopen the window in separate window.

User opens floating window with K and moves cursor by entering K again. User enter p to open the document in separate window. It may be more useful to provide another local mapping such as q for closing the window.

Pros:
- It does not require new mapping
Cons:
- Operation is complicated. User needs to enter the floating window to reopen it in separate window

I prefer second one because I would almost always use floating window and opening it in separate window would be comparably rare case.

In terms of implementation, either (or both) is easy to implement.

asymmetric · 2019-03-22T15:25:36Z

I’m not sure the use-case of permanently opening the hover/tooltip is necessary enough to block this otherwise very useful PR. I’ve never found this behavior in other editors and personally never expected it.

Can I suggest we merge this and iterate over the “permanent hover” behavior in a separate PR?

autozimu · 2019-03-22T17:20:21Z

Agree with @asymmetric. Let's have the support first. Then we can cover the special use case.

autoload/LanguageClient.vim

doc/LanguageClient.txt

rhysd · 2019-03-23T04:15:38Z

@asymmetric @autozimu

OK, let me remove the part for now.

This reverts commit 18640c2. This reverts commit f40a1c3.

Addresses review comment autozimu#767 (comment)

Addresses review comments autozimu#767 (comment)

rhysd · 2019-03-23T04:31:08Z

@autozimu I addressed your review comments at 79e36ba and a785c80.

autozimu · 2019-03-26T04:13:15Z

Thanks!

YaLTeR · 2019-03-27T10:48:24Z

I just tried this and it works amazingly! This is so much better than what was possible previously, thanks a lot.

One thing I noticed: when the hover window is opened, Ctrl-E and Ctrl-Y don't move it with the buffer so it kind of becomes detached from the cursor. Not sure if this would be easy (or in scope) to fix though.

rhysd · 2019-03-27T12:29:21Z

Yeah, it's a bit difficult case. Currently floating window is closed automatically on CursorMoved, BufEnter and InsertEnter. But <C-e> and <C-y> is none of the cases. It just scrolls screen and position of cursor is maintained. I don't know the way to hook <C-e> and <C-y> actually..

See screenshots in: autozimu/LanguageClient-neovim#767 * roles/dotfiles/files/.vim/pack/bundle/opt/LanguageClient-neovim 5685989...12e65e7 (80): > Send messages for jdt files > Remove unused lazycell > cargo update > Update lsp-types and url > Bump jsonrpc-core > Declare support for LocationLinks > Update rust version in CI > Try to have idempotent install based on version checking > Update go sample > restore goto jdt content feature > Add LanguageClient_closeFloatingHover() (#865) > rangeFormatting does not properly transmit range > Treat ShowMessageRequest with no actions as ShowMessage > Better handling of text-edits at completion > (cargo-release) version 0.1.147 > Cleanup > signal usage of rustfmt using empty rustfmt.toml > add client-side support for "rust-analyzer.applySourceChange" > add CreateFile support in apply_WorkspaceEdit > Update lsp_types > Cargo update > Expand tilde in linux and macos logger paths > [#835] Set hover window as modifiable > Add missing space between severity, code, and message > Cleanup. > Add example scala and css project. > Support CodeActions > update jsonrpc-core > cargo update > Clearify language server is needed > Remove deoplete from Quick Start section. > Generate binary sha256 file on before deploy > support LocationLink responses for GotoDefinition > update lsp-types > update jsonrpc-core > Cargo update > cargo update > add new root marker for python projects: pyproject.toml > Remove word extraction from `textEdit`. > (cargo-release) version 0.1.146 > Workaround `text_edit` is present but `new_text` is empty. > Add examples about how to configure for Ruby (#799) > Remove unnecessary closures (#797) > Use self to avoid name repetition (#796) > Do not apply fzf workaround to enter insert mode when neovim >= 0.4. > Cross platform PowerShell install script (#794) > Refactor code to use references (#790) > (cargo-release) version 0.1.145 > clippy. > Add back x86_64-unknown-freebsd to CI. > (cargo-release) version 0.1.144 > Reimplement hover window with floating window for Neovim 0.4.0 or later (#767) > Add option to allow disable echo project root. > Fix filetype key typescript.tsx. > (cargo-release) version 0.1.143 > fix diagnostic sign issues (#769) > Support root detection for tsx files. > Use :normal! instead of :normal (#766) > (cargo-release) version 0.1.142 > fmt. > Cleanup. > Implement window/showMessageRequest (#755) > Bumpup lsp_typs from v0.54.0 to v0.56.0 > Mention integration with MUcomplete. > Fix errorneous endofline warning. > refactor: Delete unusing module > fix: Fix RawMessage enum path > refactor: vim::RawMessage enum move to types module > Bumpup jsonrpc-core from v9.0.0 to v10.1.0 > cargo update > (cargo-release) version 0.1.141 > Optimize update signs. > Properly cleanup signs and virtual texts at stop. > Update rustc in Docker. > Lint. > Restruct approach to get values from params and vim. > Update doc. > Add LanguageClient.txt target to help doc > Fix nits on grammar > Document where a relative g:LanguageClient_settingsPath is relative to.

Reimplement hover window with floating window for Neovim 0.4.0 or later

572f84a

rhysd changed the title ~~WIP: Reimplement hover window with floating window for Neovim 0.4.0 or later~~ Reimplement hover window with floating window for Neovim 0.4.0 or later Mar 14, 2019

Add test for hover window using floating window

db88b8e

rhysd force-pushed the float-window-for-hover branch from a1e77c2 to db88b8e Compare March 14, 2019 17:02

rhysd added 3 commits March 15, 2019 02:47

Close floating window when entering other window on WinEnter

1a7748f

Add test for closing floating window on WinEnter

d63389f

Fix checking enough space for floating window

5ccf450

rhysd force-pushed the float-window-for-hover branch from 3bf0254 to 5ccf450 Compare March 14, 2019 18:52

rhysd added 2 commits March 17, 2019 04:48

Use BufEnter instead of WinEnter

eb752f2

since WinEnter cannot close floating hover when the current buffer switches to another buffer within the same window.

Add test where entering another buffer closes floating hover

e240734

rhysd force-pushed the float-window-for-hover branch from 263cdc9 to e240734 Compare March 16, 2019 19:49

Follow the API change of nvim_open_win()

65ccc70

Note: the API was changed at neovim/neovim@96edbe7

rhysd added 5 commits March 21, 2019 19:40

Move cursor into floating hover when hover is already open

05afc80

Add health check for floating window

45f6d06

Describe the behavior of floating window support in document

04dce37

Add g:LanguageClient_useFloatingHover

0fbded1

Add test for moving the cursor into floating window

372939d

rhysd force-pushed the float-window-for-hover branch from 80d028e to 372939d Compare March 21, 2019 11:16

Add LanguageClient#openHoverInSeparateWindow()

f40a1c3

to reopen floating hover in separate preview window

rhysd added 2 commits March 21, 2019 22:07

Rename LanguageClient#openHoverInSeparateWindow to LanguageClient#reo…

18640c2

…penHoverInSeparateWindow

Give left margin only when line is not empty

6f4ce81

autozimu reviewed Mar 22, 2019

View reviewed changes

autoload/LanguageClient.vim Outdated Show resolved Hide resolved

autozimu reviewed Mar 22, 2019

View reviewed changes

doc/LanguageClient.txt Outdated Show resolved Hide resolved

rhysd added 3 commits March 23, 2019 13:17

Revert "Add LanguageClient#reopenHoverInSeparateWindow()"

4c14bd4

This reverts commit 18640c2. This reverts commit f40a1c3.

Prefer s:GetVar() to get()

79e36ba

Addresses review comment autozimu#767 (comment)

Move description of float window behavior to function doc section

a785c80

Addresses review comments autozimu#767 (comment)

autozimu merged commit 53ce2fe into autozimu:next Mar 26, 2019

autozimu mentioned this pull request Mar 27, 2019

Updates #35

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reimplement hover window with floating window for Neovim 0.4.0 or later #767

Reimplement hover window with floating window for Neovim 0.4.0 or later #767

rhysd commented Mar 12, 2019 •

edited

Loading

autozimu commented Mar 13, 2019

theHamsta commented Mar 13, 2019

rhysd commented Mar 14, 2019

rhysd commented Mar 16, 2019

autozimu commented Mar 19, 2019

rhysd commented Mar 20, 2019 •

edited

Loading

autozimu commented Mar 20, 2019 •

edited

Loading

rhysd commented Mar 21, 2019

lorenzo commented Mar 21, 2019

rhysd commented Mar 21, 2019

rhysd commented Mar 21, 2019 •

edited

Loading

lorenzo commented Mar 21, 2019

rhysd commented Mar 21, 2019

JeanMertz commented Mar 21, 2019

lorenzo commented Mar 21, 2019

rhysd commented Mar 21, 2019 •

edited

Loading

asymmetric commented Mar 22, 2019

autozimu commented Mar 22, 2019

rhysd commented Mar 23, 2019

rhysd commented Mar 23, 2019

autozimu commented Mar 26, 2019

YaLTeR commented Mar 27, 2019

rhysd commented Mar 27, 2019

Reimplement hover window with floating window for Neovim 0.4.0 or later #767

Reimplement hover window with floating window for Neovim 0.4.0 or later #767

Conversation

rhysd commented Mar 12, 2019 • edited Loading

autozimu commented Mar 13, 2019

theHamsta commented Mar 13, 2019

rhysd commented Mar 14, 2019

rhysd commented Mar 16, 2019

autozimu commented Mar 19, 2019

rhysd commented Mar 20, 2019 • edited Loading

autozimu commented Mar 20, 2019 • edited Loading

rhysd commented Mar 21, 2019

lorenzo commented Mar 21, 2019

rhysd commented Mar 21, 2019

rhysd commented Mar 21, 2019 • edited Loading

lorenzo commented Mar 21, 2019

rhysd commented Mar 21, 2019

JeanMertz commented Mar 21, 2019

lorenzo commented Mar 21, 2019

rhysd commented Mar 21, 2019 • edited Loading

Add argument to hover function

Add special window-local mapping

asymmetric commented Mar 22, 2019

autozimu commented Mar 22, 2019

rhysd commented Mar 23, 2019

rhysd commented Mar 23, 2019

autozimu commented Mar 26, 2019

YaLTeR commented Mar 27, 2019

rhysd commented Mar 27, 2019

rhysd commented Mar 12, 2019 •

edited

Loading

rhysd commented Mar 20, 2019 •

edited

Loading

autozimu commented Mar 20, 2019 •

edited

Loading

rhysd commented Mar 21, 2019 •

edited

Loading

rhysd commented Mar 21, 2019 •

edited

Loading