Is visualCodeAction working?

weigangd commented 3 years ago

Describe the bug

I am trying to use the "extract function assist" function from rust-analyzer: https://rust-analyzer.github.io/thisweek/2021/02/08/changelog-63.html#new-features . I found the visualCodeAction but I can't make it work. Whenever I select code and use directly :call LanguageClient#textDocument_visualCodeAction() nothing happens. Also when I select code and use the context_menu :call LanguageClient_contextMenu() and the CodeAction also nothing happens. Codeactions without visual selection work fine.

Environment

neovim/vim version : NVIM v0.4.4
This plugin version : plugs['LanguageClient-neovim'].commit = 'a42594c9c320b1283e9b9058b85a8097d8325fed'
This plugin's binary version : languageclient 0.1.161
Language server link and version: rust-analyzer 7435b9e : https://github.com/rust-analyzer/rust-analyzer/releases/tag/2021-02-15

Additional context

"Extract function assist" works in nvim-lsp, so it shouldn't be a problem of rust-analyzer.

martskins commented 3 years ago

It's working for me with that version of rust-analyzer.

Peek 2021-02-23 19-38

Can you try setting the log level to DEBUG and the log after trying this?

weigangd commented 3 years ago

Thank you for your answer. Thanks to you GIF i noticed that it works when I use the character visual mode v but when I use the line visual mode V then nothing happens. I think the important logging lines are:

121:37:50 DEBUG reader-None src/rpcclient.rs:207 <= None {"id": 1, "jsonrpc": "2.0", "method": "textDocument/codeAction", "params": {"bufnr": 1, "filename": "/Users/david/projects
/rust/hello/src/main.rs", "character": 0, "handle": true, "languageId": "rust", "line": 3, "range": {"end": {"character": 2147483646, "line": 6}, "start": {"character": 0, "line"
: 3}}}}                                                                                                                                                                           
21:37:50 INFO unnamed src/language_server_protocol.rs:1318 text_document_code_action; params=Object({"bufnr": Number(1), "character": Number(0), "filename": String("/Users/david/
projects/rust/hello/src/main.rs"), "handle": Bool(true), "languageId": String("rust"), "line": Number(3), "range": Object({"end": Object({"character": Number(2147483646), "line":
 Number(6)}), "start": Object({"character": Number(0), "line": Number(3)})})})                                                                                                    
21:37:50 INFO unnamed src/language_server_protocol.rs:1993 text_document_did_change; params=Object({"bufnr": Number(1), "character": Number(0), "filename": String("/Users/david/p
rojects/rust/hello/src/main.rs"), "handle": Bool(true), "languageId": String("rust"), "line": Number(3), "range": Object({"end": Object({"character": Number(2147483646), "line": 
Number(6)}), "start": Object({"character": Number(0), "line": Number(3)})})})                                                                                                     
21:37:50 DEBUG writer-None src/rpcclient.rs:254 => None {"jsonrpc":"2.0","method":"LSP#text","params":["/Users/david/projects/rust/hello/src/main.rs"],"id":21}                   
21:37:50 DEBUG reader-None src/rpcclient.rs:207 <= None {"id": 21, "jsonrpc": "2.0", "result": ["fn main() {", "    println!(\"Hello, world!\");", "    let foo = Foo::Bar(0);", "
    match foo {", "        Foo::Bar(_) => {}", "        Foo::Baz { a, b } => {}", "    }", "}", "", "struct Foo2 {", "    a: i32,", "}", "", "impl Foo2 {", "    /// Get a mutable
 reference to the foo2's a.", "    fn a_mut(&mut self) -> &mut i32 {", "        &mut self.a", "    }", "", "    /// Get a reference to the foo2's a.", "    fn a(&self) -> &i32 {"
, "        &self.a", "    }", "}", "", "enum Foo {", "    Bar(i32),", "    Baz { a: String, b: i32 },", "}", ""]}                                                                 
21:37:50 DEBUG writer-Some("rust") src/rpcclient.rs:254 => Some("rust") {"jsonrpc":"2.0","method":"textDocument/codeAction","params":{"context":{"diagnostics":[]},"range":{"end":
{"character":2147483646,"line":6},"start":{"character":0,"line":3}},"textDocument":{"uri":"file:///Users/david/projects/rust/hello/src/main.rs"}},"id":2}                         
21:37:50 DEBUG reader-None src/rpcclient.rs:207 <= None {"method": "languageClient/handleCursorMoved", "jsonrpc": "2.0", "params": {"bufnr": 1, "viewport": {"end": 29, "start": 0
}, "languageId": "rust", "buftype": "", "position": {"character": 0, "line": 3}, "filename": "/Users/david/projects/rust/hello/src/main.rs"}}                                     
21:37:50 INFO unnamed src/language_server_protocol.rs:2873 handle_cursor_moved; params=Object({"bufnr": Number(1), "buftype": String(""), "filename": String("/Users/david/project
s/rust/hello/src/main.rs"), "languageId": String("rust"), "position": Object({"character": Number(0), "line": Number(3)}), "viewport": Object({"end": Number(29), "start": Number(
0)})}) force_redraw=false                                                                                                                                                         
21:37:50 DEBUG unnamed src/language_client.rs:108 state.last_cursor_line: 6 ==> 3

Edit: Format log

weigangd commented 3 years ago

The '"end": {"character": 2147483646,...' seems suspicious

martskins commented 3 years ago

Oh I see. Yeah, that seems to be what vim returns when you call getpos in visual line mode.

From :help getpos()

        Note that for '< and '> Visual mode matters: when it is "V"
        (visual line mode) the column of '< is zero and the column of
        '> is a large number.

'> is precisely the end bit you are seeing there. I'm not sure if there is a better, non-hacky way to get the bounds of the visual selection, but I'll see if I can find something. If anyone else is interested in finding a solution for this, the relevant piece of code is here: https://github.com/autozimu/LanguageClient-neovim/blob/a42594c9c320b1283e9b9058b85a8097d8325fed/autoload/LSP.vim#L76

weigangd commented 3 years ago

Inspired by: http://vim.1045645.n5.nabble.com/Bug-with-getpos-td1163979.html I found out one can use col("'>") instead of getpos("'>"). Also get_visual_selection() looks a lot like it was copied from https://stackoverflow.com/a/6271254 and a lot of it is not neccessary. I fixed it for me locally with the following code:

function! s:get_visual_selection() abort     
    let line_start = line("'<")                               
    let column_start = col("'<")                      
    let line_end = line("'>")                    
    let column_end = col("'>")     

    "no selection -> [LC] invalid value: integer `-1`, expected u64     
    return {                                                            
      \ 'start': {                                    
        \ 'line': line_start - 1,                 
        \ 'character': column_start - 1,     
        \ },                                 
      \ 'end': {                        
        \ 'line': line_end - 1,      
        \ 'character': column_end - 1,      
        \ }                                
      \ }           
endfunction

This works fine for me but there are two more things I would like to mention:

getpos() and col() return the byte index and not the screen column position like virtcol(). I haven't found a way to correctly handle for example german umlauts in code like "äöü".to_string()
As can be seen in @martskins GIF the extracted function is called "$0fun_name" instead of only "fun_name" like in https://rust-analyzer.github.io/thisweek/2021/02/08/changelog-63.html#new-features . I don't know if this is something which should be handled in the client or if I should ask the rust-analyzer people

martskins commented 3 years ago

Oh nice one! If you want to open a PR for that I'd be happy to merge.

About the $0fun_name thing. I assumed it was an issue with my snippets config so I didn't pay too much attention to it, but if it happens to you too might be worth investigating.

weigangd commented 3 years ago

Created the PR.

The $0fun_name thing happens to me too and I don't really know anything about or use snippets. So if you have time to investigate it I would be glad. It happened with nvim-lsp, too.

weigangd commented 3 years ago

Regarding the $0 thing: I wrote the an issue for the rust-analyzer team ( https://github.com/rust-analyzer/rust-analyzer/issues/7793 ). It seems to be a task for them. In my eyes this issue here can be closed except if you want to do something about the non-ascii visual selection.

martskins commented 3 years ago

Is it fair to say this has been solved? Still not released but hopefully will see the main branch soonish.

weigangd commented 3 years ago

Yes this ticket can be closed. 👍

autozimu / LanguageClient-neovim