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

improvement: if no binary version in jar path try using build target info #6698

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

kasiaMarek
Copy link
Contributor

For jars if no scala version included in the path, fallback to using scala version from build target before defaulting to "2.13".

resolves: #6693

@tgodzik
Copy link
Contributor

tgodzik commented Aug 21, 2024

We could also try to check if there tasty files, but this is already a good improvement

@kasiaMarek
Copy link
Contributor Author

kasiaMarek commented Aug 22, 2024

We could also try to check if there tasty files

That's a better way to do this (though tasty files won't be in sources, so we have to map source jars to dep jars).

@kasiaMarek kasiaMarek marked this pull request as ready for review August 23, 2024 07:48
Copy link
Contributor

@tgodzik tgodzik left a comment

Choose a reason for hiding this comment

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

Aside from the naming comment LGTM!

@@ -434,10 +434,14 @@ final class BuildTargets private (
* @return path to the source jar for that jar
*/
private def sourceJarPathFallback(
sourceJarPath: AbsolutePath
sourceJarPath: AbsolutePath,
Copy link
Contributor

Choose a reason for hiding this comment

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

We would probably need to rename the parameters and the method

@tOverney
Copy link

tOverney commented Nov 5, 2024

@tgodzik @kasiaMarek what's holding up these changes?
Fixing the source jar navigation in those cases would be the biggest improvement on metals for me :)

@kasiaMarek
Copy link
Contributor Author

what's holding up these changes?

I'm afraid, I am. I need to address Tomek's review comment and it can be merged after that.

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.

No code navigation when source (and dependency) jar reside in local repo
3 participants