Skip to content
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

bugfix: Fix issues with auto imports not showing up #4956

Merged
merged 1 commit into from Feb 10, 2023

Conversation

tgodzik
Copy link
Contributor

@tgodzik tgodzik commented Feb 9, 2023

Previously, we merging different auto imports into one, we would us uri to get the changes from the single action. That uri might have been encoded/decode which might be different from what there is in a map of changes.

Now, instead we just merge all the changes to avoid any issues with url decoding or encoding. Those changes cannot come from different files, so this will not be an issue.

Fixes #4906

@@ -40,8 +38,9 @@ class ImportMissingSymbol(compilers: Compilers, buildTargets: BuildTargets)
codeAction
.getEdit()
.getChanges()
.getOrDefault(uri, Collections.emptyList)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is another one :/

Changes would have file:///C:/Users/Tomasz/Documents/Work/CodeActionImportIssue/subprojecta/src/main/scala/SomeClassA.scala

while uri was "file:///c%3A/Users/Tomasz/Documents/Work/CodeActionImportIssue/subprojecta/src/main/scala/SomeClassA.scala

Copy link
Member

@ckipp01 ckipp01 left a comment

Choose a reason for hiding this comment

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

Makes sense. Btw there was a new circe release that is screwing with the CompletionSbtLibSuite. We should probably update the version test to rely on completions for 0.13.x or something instead of the current 0.14.x.

Previously, we merging different auto imports into one, we would us uri to get the changes from the single action. That uri might have been encoded/decode which might be different from what there is in a map of changes.

Now, instead we just merge all the changes to avoid any issues with url decoding or encoding. Those changes cannot come from different files, so this will not be an issue.

Fixes scalameta#4906
@tgodzik tgodzik reopened this Feb 10, 2023
@tgodzik
Copy link
Contributor Author

tgodzik commented Feb 10, 2023

I had a bit of a weird issue with this PR, but it's rebased and running now.

@tgodzik tgodzik requested a review from ckipp01 February 10, 2023 08:11
Copy link
Member

@ckipp01 ckipp01 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 4cf33c3 into scalameta:main Feb 10, 2023
@tgodzik tgodzik deleted the fix-windows-escaping branch February 10, 2023 12:25
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.

Unable to import though code action
2 participants