Remove the middleware addon component from all language servers - #23898
Conversation
|
|
||
| protected shouldCreateHidingMiddleware(jupyterDependencyManager: IJupyterExtensionDependencyManager): boolean { | ||
| return jupyterDependencyManager && jupyterDependencyManager.isJupyterExtensionInstalled; | ||
| return ( |
There was a problem hiding this comment.
I'm assuming this was a oversight.
Based on the comments of HidingMiddlewareAddon, we should only hide completions and the like only when using Pylance & not with Jedi
/**
* This class is used to hide all intellisense requests for notebook cells.
*/
There was a problem hiding this comment.
Rich Chiodo (@rchiodo) would be the best person to ask (I think?) about how HidingMiddlewareAddon is expected to be used. He's back on Monday.
As of #20831, it appears that shouldCreateHidingMiddleware always returns false for Pylance (see NodeLanguageClientMiddleware). Given the derived class, I don't think the LanguageServerType.Node check is necessary.
I'm not sure what LanguageServerType.Microsoft represents. Is it a legacy option from the MPLS days?
There was a problem hiding this comment.
LanguageServerType.Microsoft is from MPLS
There was a problem hiding this comment.
Sounds like we don't ever need the middleware then.
There was a problem hiding this comment.
I've removed the code that adds the middleware,
Had to force push as my previous commits weren't signed
Don Jayamanne (DonJayamanne)
added
skip package*.json
b969f60 to
8ea7f18
Compare
| } | ||
|
|
||
| // eslint-disable-next-line class-methods-use-this | ||
| protected shouldCreateHidingMiddleware(_: IJupyterExtensionDependencyManager): boolean { |
There was a problem hiding this comment.
Looks like none of the language servers need this anymore.
Hence removing this completely
Don Jayamanne (DonJayamanne)
changed the title
8ea7f18 to
8186f4f
Compare
|
If I'm remembering correctly this was necessary when Pylance did it's own concatenation of notebook cells (it was necessary to skip intellisense for notebook cells so we wouldn't get dupes?) I believe it's not used/necessary anymore too. |
4 participants
Fixes #23897