Expand Selection selects too wide scope

[doofin]

Describe the bug

When use selection->Expand Selection,it sometimes selects the wrong range

Reproduction:
1.type single line some comment
2.put cursor before // or after //
3.expand selection will select wrong block

In addition,it's also problematic with expand selection on block comment /* */

Expected behavior

expand selection should only select the comment block when cursor is before before // or after //

Operating system

Linux

Editor/Extension

VS Code

Version of Metals

v0.11.1

Extra context or search terms

Expand ,Selection
probably relates to scala/vscode-scala-syntax#49

[tgodzik]

Thanks for reporting! This is related to the fact that the Scala parser omits comments, we should probably do a similar thing to what we do for keywords in https://github.com/scalameta/metals/blob/main/mtags/src/main/scala-2/scala/meta/internal/pc/Keywords.scala#L66

Check if we are in the comment and expand the selection for it otherwise do it normally.

[doofin]

@tgodzik Could you give me some advice on how to achieve this in more details ? and do we need to change Scala parsers in scalameta library?

[tgodzik]

A very similar issue was in https://github.com/scalameta/metals/pull/2822/files where we wanted to not show completions inside comments. We could use the same mechanism to check if we are in a comment and if we are we would probably need to use the Scalameta tokenizer to find the enclosing scope.

It would need to be implemented in:

For Scala 2:
https://github.com/scalameta/metals/blob/main/mtags/src/main/scala-2/scala/meta/internal/pc/SelectionRangeProvider.scala

For Scala 3:
https://github.com/scalameta/metals/blob/main/mtags/src/main/scala-3/scala/meta/internal/pc/SelectionRangeProvider.scala

Of course we don't need to do both at the same time, but the solution would be similar.

[doofin]

thanks! I will have a look

[doofin]

I see the problem is in https://github.com/scalameta/metals/blob/main/mtags/src/main/scala-3/scala/meta/internal/pc/SelectionRangeProvider.scala#L43 that

val path =
       Interactive.pathTo(driver.openedTrees(uri), pos)(using ctx)

where the trees returned by openedTreeswill prune all the coments. Does that mean we have to modify in InteractiveDriver inpackage dotty.tools` ? @tgodzik

In addition,seems that Keywords.scala#L66 doesn't exist for scala 3 yet . We can have a meeting about this if you would like to.

[tgodzik]

For Scala 3 we have https://github.com/scalameta/metals/blob/main/mtags/src/main/scala-3/scala/meta/internal/pc/completions/KeywordsCompletions.scala#L75 <- comments are handled there. We would probably need to also check if we are in a comment and if so the next range should be the range of that comment.

where the trees returned by openedTreeswill prune all the coments. Does that mean we have to modify in InteractiveDriver inpackage dotty.tools`

That's why we use the scalameta tokenizer for the task, which also handles tokens.

[tgodzik]

We merged the support for Scala 2, Scala 3 support should look very similar.

[doofin]

@tgodzik we actually merged support for Scala 3, now I'm looking at Scala 2. The scala meta code part seems similar, but I'm unable to find out how to get the cursor location as in https://github.com/scalameta/metals/blob/main/mtags/src/main/scala-3/scala/meta/internal/pc/SelectionRangeProvider.scala#L47

[tgodzik]

Ach, you're right! pos should be almost the same as span from Scala 3

[doofin]

thanks @tgodzik , what is the Scala 2 correspondence of

metals/mtags/src/main/scala-3/scala/meta/internal/pc/SelectionRangeProvider.scala

Line 49 in 2014643

Interactive.pathTo(driver.openedTrees(uri), pos)(using ctx)

?

[tgodzik]

That should be lastVisitedParentTrees