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

Store compilers with dependencies *and* scalacOptions. #425

Merged
merged 1 commit into from
Nov 29, 2020

Conversation

ckipp01
Copy link
Member

@ckipp01 ckipp01 commented Nov 28, 2020

The main reason behind this change is that in Metals when a user adds or
removes a $scalac magic import, it has no affect unless they close
Metals and restart it. This leads to people assuming that the $scalac
imports aren't working in Metals.

This is because when we do a looking into the ctx compilers we are only
taking into consideration the dependencies instead of the scalacOptions.
This slight changes just makes the key (Dependencies, scalacOptions)
instead of Dependencies so that when a user changes the $scalac
import, it's taken into consideration when doing the lookup or providing
a new compiler.

Address the following comment in #397

I think this doesn't work in worksheets as far as I tried, not sure exactly why.

Old Behavior (appears that it isn't working)

2020-11-28 13 40 20

New Behavior

2020-11-28 13 38 25

The main reason behind this change is that in Metals when a user adds or
removes a `$scalac`  magic import, it has no affect unless they close
Metals and restart it. This leads to people assuming that the `$scalac`
imports aren't working in Metals.

This is because when we do a looking into the ctx compilers we are only
taking into consideration the dependencies instead of the scalacOptions.
This slight changes just makes the key `(Dependencies, scalacOptions)`
instead of `Dependencies` so that when a user changes the `$scalac`
import, it's taken into consideration when doing the lookup or providing
a new compiler.
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.

LGTM! Good catch!

@ckipp01 ckipp01 merged commit f5c223b into scalameta:master Nov 29, 2020
@ckipp01 ckipp01 deleted the scalacWorksheets branch November 29, 2020 10:45
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.

2 participants