Set active editor on startup #6152
Conversation
akosyakov
requested review from
AlexTugarev,
azatsarynnyy and
vince-fugnitto
| @@ -43,6 +43,7 @@ export class TextEditorServiceImpl implements TextEditorService { | |||
| constructor(@inject(EditorManager) private editorManager: EditorManager) { | |||
| editorManager.onCurrentEditorChanged(this.onEditorChanged); | |||
| editorManager.onCreated(w => this.onEditorCreated(w)); | |||
| editorManager.all.forEach(w => this.onEditorCreated(w)); | |||
There was a problem hiding this comment.
@tolusha thanks for tackling this! 🙏
in general this should work, but we could actually clean up the editorManager.onCurrentEditorChanged(this.onEditorChanged) here, no?
WDYT of renaming onEditorCreated to connectEditor or something like that, adjusting onCurrentEditorChanged handler, and now calling connectEditor for editorManager.currentEditor once?
There was a problem hiding this comment.
maybe a bit off topic as this is not related to the issue in scope of this PR, but before I forget to mention it: I think we should remove this.editors map which holds references to MonacoEditor instances without a real reason.
e.g. getActiveEditor uses editorManager an so could listTextEditors be implemented as well.
There was a problem hiding this comment.
@AlexTugarev
I've done some refactoring as you mentioned.
I am not sure if changes regarding connectEditor make sense.
There was a problem hiding this comment.
ok with me. let's test with many opened editors to see how it works with many events being fired.
| @@ -38,25 +38,19 @@ export class TextEditorServiceImpl implements TextEditorService { | |||
| onTextEditorAdd: Event<MonacoEditor> = this.onTextEditorAddEmitter.event; | |||
| onTextEditorRemove: Event<MonacoEditor> = this.onTextEditorRemoveEmitter.event; | |||
|
|
|||
| private editors = new Map<string, MonacoEditor>(); | |||
There was a problem hiding this comment.
always nice to remove duplicate state! 😊
|
@AlexTugarev please finish the review |
|
I am going to squash commits and merge... |
Signed-off-by: Anatoliy Bazko <abazko@redhat.com>
tolusha
force-pushed
the
ab/fixActiveEditor
branch
from
6691758
to
6a753bf
Compare
Signed-off-by: Anatoliy Bazko abazko@redhat.com
Reference issue
#6101
What it does
When Thiea is started the editors which had been created were not taking into account. Which leads to setting
window.activeTextEditorto undefined and impossibility to add a new python configuration [2][1] https://github.com/theia-ide/theia/blob/ab/fixActiveEditor/packages/plugin/src/theia.d.ts#L2985
[2] https://github.com/Microsoft/vscode-python/blob/master/src/client/debugger/extension/configuration/launch.json/updaterService.ts#L26
How to test
pluginsfolderlaunch.jsonand try to add any python configurationlaunch.json[1] https://github.com/microsoft/vscode-python/releases/download/2019.5.18875/ms-python-release.vsix
Review checklist
Reminder for reviewers