definition does not exclude :: in strings

[renkun-ken]

In the following code

xml_find_all(x, "descendant::expr")

when I hold Command button in VSCode, it sends a definition request. When I move my cursor to "descendant::expr", the request results in an error:

[Error - 2:31:59 PM] Request textDocument/definition failed.
  Message: {"message":"there is no package called ‘descendant’","call":{},"package":"descendant","lib.loc":{}}

  Code: -32603 

It seems that hover definition does not exclude :: in string literals.

[randy3k]

Ya, you are right. we would need to check the current scope at

languageserver/R/utils.R

Lines 45 to 53 in c6d3edc

	check_scope <- function(uri, document, position) {
	if (is_rmarkdown(uri)) {
	line <- position$line
	!identical(sum(vapply(
	document$content[1:(line + 1)], startsWith, integer(1), "```")) %% 2, 0)
	} else {
	TRUE
	}
	}

[andycraig]

I'm using the version of languageserver on CRAN. When I hold Ctrl and hover over print in the below code, it works as expected (I get the hover definition, and if I click it opens a file with the definition):

xml_find_all(x, "base::print")

When I do the same for

xml_find_all(x, "descendant::expr")

I get basically the same error:

[Error - 18:02:34] Request textDocument/definition failed.
  Message: {"message":"there is no package called ‘descendant’","call":{}}

  Code: -32603 

I don't have the package descendant installed, so I think that's the problem. (I tried installing it, but it doesn't seem to be on CRAN.) @renkun-ken Do you have it installed?

Moving the mouse over expr without Ctrl held down, I don't get any hover popup but I also don't get an error showing. The code for hover_reply includes a tryCatch that is probably catching an identical error. I think I removed the equivalent tryCatch in the definition code, so I've introduced an inconsistency there. @randy3k If you want, I could do a PR to put it back in (also for the symbol reply functions).

[renkun-ken]

@andycraig, thanks for your testing. My point is that I'm not sure if it makes sense to provide definition of foo::bar in string literals, since the code I provide here is really an XPath expression rather than R code, or it could be some C++ code, or anything.

[andycraig]

@renkun-ken Ah, beg your pardon. No wonder descendant isn’t on CRAN! Re-reading the issue I now see what you mean.

[renkun-ken]

Thanks! I test it and it works well now.

[randy3k]

Actually it doesn’t work for multi line strings. But they are not common plus the search would be costly. This is good enough for now.

[renkun-ken]

Multi-line strings can be handled by parsed xml doc, i.e., xpath query if the cursor is in the range of a string literal, but it requires that the document is syntactically correct to be parseable. It think it's good enough too.