-
Notifications
You must be signed in to change notification settings - Fork 323
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
fix: do not call doctor.check()
on each build target compilation end
#3768
Conversation
metals/src/main/scala/scala/meta/internal/metals/MetalsLanguageServer.scala
Show resolved
Hide resolved
metals/src/main/scala/scala/meta/internal/metals/doctor/Doctor.scala
Outdated
Show resolved
Hide resolved
metals/src/main/scala/scala/meta/internal/metals/MetalsLanguageServer.scala
Outdated
Show resolved
Hide resolved
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.
Actually, creating a new client's provider option should solve all of the above-mentioned suggestions. doctorVisibilityProvider
?
b9173c0
to
2dfe3fd
Compare
metals/src/main/scala/scala/meta/internal/metals/InitializationOptions.scala
Show resolved
Hide resolved
```typescript | ||
interface InitializationOptions: { | ||
compilerOptions: { | ||
completionCommand?: string; | ||
isCompletionItemDetailEnabled?: boolean; | ||
isCompletionItemDocumentationEnabled?: boolean; | ||
isCompletionItemResolve?: boolean; | ||
isHoverDocumentationEnabled?: boolean; | ||
isSignatureHelpDocumentationEnabled?: boolean; | ||
overrideDefFormat?: "ascii" | "unicode"; | ||
parameterHintsCommand?: string; | ||
snippetAutoIndent?: boolean; | ||
} | ||
"copyWorksheetOutputProvider": boolean, | ||
"debuggingProvider": boolean, | ||
"decorationProvider": boolean, | ||
"didFocusProvider": boolean, | ||
"disableColorOutput" boolean, | ||
"doctorProvider": "json" | "html", | ||
"executeClientCommandProvider": boolean, | ||
"globSyntax": "vscode" | "uri" | ||
"icons": "octicons" | "vscode" | "unicode", | ||
"inlineDecorationProvider": boolean, | ||
"inputBoxProvider": boolean, | ||
"isExitOnShutdown" : boolean, | ||
"isHttpEnabled": boolean, | ||
"commandInHtmlFormat": "vscode" | "sublime", | ||
"openFilesOnRenameProvider": boolean, | ||
"openNewWindowProvider": boolean, | ||
"quickPickProvider": boolean, | ||
"renameFileThreshold": number, | ||
"slowTaskProvider": boolean, | ||
"statusBarProvider": "on" | "off" | "show-message" | "log-message", | ||
"treeViewProvider": boolean | ||
debuggingProvider?: boolean; | ||
decorationProvider?: boolean; | ||
inlineDecorationProvider?: boolean; | ||
didFocusProvider?: boolean; | ||
doctorProvider?: "json" | "html"; | ||
executeClientCommandProvider?: boolean; | ||
globSyntax?: "vscode" | "uri"; | ||
icons?: "vscode" | "octicons" | "atom" | "unicode"; | ||
inputBoxProvider?: boolean; | ||
isVirtualDocumentSupported?: boolean; | ||
isExitOnShutdown?: boolean; | ||
isHttpEnabled?: boolean; | ||
openFilesOnRenameProvider?: boolean; | ||
quickPickProvider?: boolean; | ||
renameFileThreshold?: number; | ||
slowTaskProvider?: boolean; | ||
statusBarProvider?: "on" | "off" | "log-message" | "show-message"; | ||
treeViewProvider?: boolean; | ||
testExplorerProvider?: boolean; | ||
openNewWindowProvider?: boolean; | ||
copyWorksheetOutputProvider?: boolean; | ||
commandInHtmlFormat?: "vscode" | "sublime"; | ||
doctorVisibilityProvider?: boolean; |
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 copypasted typescript interface from metals-language-client
- I think it's easier to maintain it this way. It's also more accurate because it indicates that these properties are optional and don't have to be set.
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.
Overall looks good.
I have only one comment about Subject
@dos65 I removed Subject. I also encourage you to do reviews in metals-vscode and 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.
👍
closes #3755
needs scalameta/metals-vscode#930
Because
Compilations.scala
usesBatchedFunction
to batch multiple requests it's very tempting to add there the callback that will execute doctor refresh. But I didn't want to couple doctor and compilations so I createdSubject
, should I just use() => doctor.executeRefreshDoctor
as aCompilations's
constructor argument?