Display prompt [Information Message] to restart the server when the omnisharp configuration changes #2316
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2316 +/- ##
==========================================
+ Coverage 61.38% 61.51% +0.13%
==========================================
Files 85 86 +1
Lines 3835 3851 +16
Branches 552 554 +2
==========================================
+ Hits 2354 2369 +15
- Misses 1311 1312 +1
Partials 170 170
Continue to review full report at Codecov.
|
|
@DustinCampbell Are there any other options that we should consider here ? |
|
@colombod This test https://github.com/OmniSharp/omnisharp-vscode/pull/2316/files#diff-dd9d0618b9edb503c00be5f531debc27R78 is so cool. Thanks for the idea 🥇 |
|
Where is the code where we decide which options should cause us to prompt an OmniSharp restart? Right now, it looks like we'll prompt on any change to the 'omnisharp' or 'csharp' configurations. |
|
@DustinCampbell https://github.com/OmniSharp/omnisharp-vscode/pull/2316/files#diff-c4598b401326badbfaa08c9f303e66edR34. We check whether the value of these three options have changed. |
|
@akshita31; Thanks! I was having trouble spotting that code for some reason. Looks good to me! |
|
@DustinCampell the magic of rx! |
| } | ||
| }); | ||
|
|
||
| return () => disposable.dispose(); |
There was a problem hiding this comment.
This and the line below are beyond my rx-fu, can you explain them?
There was a problem hiding this comment.
publishBehavior is used to create a behavior observable which has the initial value as Option.Read(). refCount() is used so that the dispose function on the observable is called only when all the observers who susbcribed to this observable invoke unsubscribe. This test verifies this behavior https://github.com/OmniSharp/omnisharp-vscode/pull/2316/files#diff-dd9d0618b9edb503c00be5f531debc27R78
| let subscription = ConfigChangeObservable(optionObservable) | ||
| .subscribe(_ => { | ||
| let message = "OmniSharp configuration has changed. Would you like to relaunch the OmniSharp server with your changes?"; | ||
| ShowInformationMessage(vscode, message, { title: "Restart Now", command: 'o.restart' }); |
There was a problem hiding this comment.
I would have the button say "Restart OmniSharp"
| } | ||
|
|
||
| function hasChanged(oldOptions: Options, newOptions: Options): boolean { | ||
| if (oldOptions.path != newOptions.path || |
Added an option change observer that subscribes to an Observable and shows the information message when there is a change in the configuration the server cares about and then restarts the server when "Restart" is clicked. Added unit tests for each of the components involved.