-
Notifications
You must be signed in to change notification settings - Fork 74
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
Add option to show implicit methods and classes #447
Add option to show implicit methods and classes #447
Conversation
fca6cd2
to
19da2d8
Compare
19da2d8
to
8db7740
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good 👍
I added an option to toggle the option on and off, which might be useful to users. I will need to do a small change in the server too. |
@@ -656,6 +656,14 @@ function launchMetals( | |||
}); | |||
}); | |||
|
|||
registerCommand("metals.toggle-implicit-conversions-and-classes", () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm trying to think if this will be used by any other editors that may use metals-languageclient
. Do you think we should add this there, or is this probably pretty VS Cod specfic?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think currently it's pretty much VS Code specific, it could be useful in cases that we show decorations, but emacs does not used the language client
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For hovers, it will just start working with the next hover.
2738ad0
to
270cdf3
Compare
const config = workspace.getConfiguration("metals"); | ||
const configProperty = config.inspect<boolean>(setting); | ||
const currentValues = configProperty?.workspaceValue ?? false; | ||
config.update(setting, !currentValues, ConfigurationTarget.Workspace); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gabro I set it to use workspace by default, I think it makes sense for toggle. What do you think?
Follow up after scalameta/metals#2232