Implement hover subcommand for #292

[Nashenas88]

Implement hover subcommand for #292

[Nashenas88]

I tested this manually and could only get the following response regardless of (line,col):

> hover test_data/hover/src/main.rs 1 1
2: HoverSuccess(
    Hover {
        contents: [],
        range: None
    }
)

Is this not implemented or is my current implementation missing something?

[nrc]

It is implemented and ought to work - you'll need to give exactly the right location for a ref (this can be surprisingly difficult, and I think both row and col are 0-indexed, but maybe they are 1-indexed?). Its also possible this is broken in the current nightly if you are using today's (I thought it wouldn't be, but I'm not sure if a PR landed in time or not).

[nrc]

The code looks good - I'm somewhat reluctant to merge it if it doesn't work, but I don't see where it could be going wrong.

[Nashenas88]

I would agree to not merge it until it's working. I submitted the PR just to make sure that a) it didn't forgotten amongst everything else I'm working on and b) to get other eyes to see if I was doing something obviously wrong.

Also, it seems that the tests are failing before this commit as well. I checked out the previous commit and ran the tests. It's always crashing trying to run the second test.

[nrc]

This should work now, will need rebasing on top of #300

[Nashenas88]

Testing this with cargo run test_data/hover after doing the latest rebase (issue still there):

> def test_data/hover/src/main.rs 15 14
2: Locations(
    [
        Location {
            uri: "file:///Users/paul/programming/rls2/test_data/hover/src/main.rs",
            range: Range {
                start: Position {
                    line: 9,
                    character: 7
                },
                end: Position {
                    line: 9,
                    character: 7
                }
            }
        }
    ]
)
> hover test_data/hover/src/main.rs 15 14
3: HoverSuccess(
    Hover {
        contents: [],
        range: None
    }
)
>

I've checked lines 15 and 16 columns 0-18 and they all return the same thing. 15,14 is the only combination that returns a result for def.

Edit: This is on OS X 10.11.6 (El Capitan), rust latest nightly with rls, rust-analysis and rust-src components installed.

[nrc]

@Nashenas88 so just to clarify: are you saying that def only works at a single position in the whole file? (I would expect it to work at any position within a ref, so at least a few adjacent positions, and the file looks like it should have multiple refs in it).

And hover doesn't work at all - even where def does work? (And this is based on top of current master with a recent nightly?)

[Nashenas88]

@nrc I intended to write that (15,14) is the first spot on that one line that def works (writing anything on little sleep isn't always the best idea...).

It does work in multiple other spots in the file. In all cases hover always returns the blank HoverSuccess message.

I've just tried it again with the nightly from tonight, but haven't attempted a new rebase of rls tonight. It has the same results. It was rebased on current master and nightly when I wrote the previous comment, and it had the changes from #300 in the rebase.

I was busy tonight so I couldn't investigate additionally, but I'll try looking into it late tomorrow night or sometime Saturday.

[nrc]

OK, that sounds good in some respects and weird in others - I don't see how we could pass the hover test, but not have this work.

Let me know if you get stuck investigating and I can also take a look.

[Nashenas88]

I rabbit-holed too much on Saturday trying to get gdb to work with the test. Sunday was busy due to Mother's day. This morning I dug a little more, and I'm still confused. Before serialization they're both sending exactly the same piece of data. I need to debug at the point where the server is receiving the call to see what the difference is. It could also be how I'm parsing the return somehow... I won't have time to look at this again until after work.

[Nashenas88]

I haven't had more than 30 minutes at a time to look at this, but I've found one difference so far. In src/actions/mod.rs:fn hover the let ty =... line in the thread::spawn closure doesn't get the same value. It's got a value in the test, but is empty in the manual cli run. Something about the analysis is different because the span input and params are exactly the same in the test run and in the manual cli run.

@nrc Is there something else I'm missing when calling the cli version? I'm executing it like so: cargo run /Users/paul/programming/rls2/test_data/hover. I've also tried just use a relative path but get the same results.

[nrc]

So it sounds like you might be querying before the build and analysis is finished - do you see the 'build done' notification?

You'll need -- after cargo run for the arg to be passed to the RLS rather than to Cargo.

[Nashenas88]

So, interestingly, running with or without the -- doesn't make a difference, cargo was appending it to the binary args in both cases. I also did not know to wait for the diagnosticsEnd message. What had been throwing me off is that the def command doesn't need to wait for it. It returns a seemingly normal result all the time, making me think it was already capable of processing messages. The other issue I had is that the argument doesn't seem to be used. It uses your current directory, so running in the rls src dir was actually waiting for rls to analyze itself. I cd'ed into test_data/hover, ran ../../target/debug/rls . and didn't have to wait long at all for diagnosticEnd to appear. Both def and hover work fine. This is what I'm seeing now:

3: HoverSuccess(
    Hover {
        contents: [
            LanguageString(
                LanguageString {
                    language: "rust",
                    value: "&str"
                }
            )
        ],
        range: None
    }
)

I'll rebase against latest, retest and then I think this should be good to go. Sorry for the long, overly complicated delay.

[Nashenas88]

@nrc This is ready to go now.

[nrc]

Thanks for sticking with it! Looks good