Skip to content

Use untpd.Tree instead of tpd.Tree for SelectionRangeProvider #22702

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

rochala
Copy link
Contributor

@rochala rochala commented Mar 4, 2025

Thanks to @NPCRUS @Yummy-Yums for taking a part in a spree. Sorry for taking that long to create a PR, but I was unavailable.

Fixes #22566

@rochala
Copy link
Contributor Author

rochala commented Mar 4, 2025

And this is PR that would fix this issue if we didn't migrate to untpd trees: #22703

@rochala rochala requested a review from kasiaMarek March 4, 2025 08:17
def toSelectionRange(srcPos: SourcePosition) =
val selectionRange = new SelectionRange()
selectionRange.setRange(srcPos.toLsp)
selectionRange

val treeSelectionRange = toSelectionRange(tree.sourcePos)

def getArgsSpan(args: List[Tree]): Option[SourcePosition] =
args match
case Seq(param) => Some(param.sourcePos)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this needed? Won't it repeat the same selection? Can you add a test with a single param?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rochala do you need help finishing this one?

@som-snytt
Copy link
Contributor

"Apropos of nothing", this is an example where I don't know where to fetch from, because of scala3 vs dotty.

[email protected]:rochala/dotty.git

It would be nice if github accommodated either specification (on cloned repos).

@som-snytt
Copy link
Contributor

som-snytt commented Mar 6, 2025

I don't have context for the previous comment if we didn't migrate to untpd trees, but I will learn about it soon, perhaps.

Edit: I'm not sure if this PR means the current PR or the other PR! That is how impoverished my context is.

@kasiaMarek
Copy link
Member

I don't have context for the previous comment if we didn't migrate to untpd trees

In this PR (current PR) @rochala switches from using typed trees to untyped ones for selection provider. This both fixes the issue with parentheses (#22566) and generally makes more sense, since untyped trees (vs typed ones) closely resemble the structure of the source code. My understanding is that if we didn't make that switch the other PR (#22703) contains an alternative solution. @rochala, does this mean that this PR supersedes #22703?

@rochala
Copy link
Contributor Author

rochala commented Mar 7, 2025

No, they are 2 different bugfixes.

@NPCRUS
Copy link
Contributor

NPCRUS commented Apr 30, 2025

ping

@rochala rochala force-pushed the 22566-selection-range-omits-closing-paren branch from 0dc2e28 to af12fac Compare May 5, 2025 16:39
@rochala rochala requested a review from kasiaMarek May 5, 2025 16:40
Copy link
Member

@kasiaMarek kasiaMarek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@tgodzik tgodzik merged commit 107a2f1 into scala:main May 6, 2025
29 checks passed
tgodzik pushed a commit to scala/scala3-lts that referenced this pull request May 12, 2025
…22702)

Thanks to @NPCRUS @Yummy-Yums for taking a part in a spree. Sorry for
taking that long to create a PR, but I was unavailable.

Fixes scala#22566

---------

Co-authored-by: NPCRUS <[email protected]>
Co-authored-by: Yummy-Yums <[email protected]>
tgodzik added a commit to scala/scala3-lts that referenced this pull request May 12, 2025
…22702)

Thanks to @NPCRUS @Yummy-Yums for taking a part in a spree. Sorry for
taking that long to create a PR, but I was unavailable.

Fixes scala#22566

---------

Co-authored-by: NPCRUS <[email protected]>
Co-authored-by: Yummy-Yums <[email protected]>
[Cherry-picked 107a2f1][modified]
odersky pushed a commit to dotty-staging/dotty that referenced this pull request May 12, 2025
…22702)

Thanks to @NPCRUS @Yummy-Yums for taking a part in a spree. Sorry for
taking that long to create a PR, but I was unavailable.

Fixes scala#22566

---------

Co-authored-by: NPCRUS <[email protected]>
Co-authored-by: Yummy-Yums <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Selection range omits closing paren
5 participants