Theia: currentEditor & activeEditor in EditorManager are not initialised when theia is loaded

Created on 22 Mar 2018  路  18Comments  路  Source: eclipse-theia/theia

EditorManager.currentEditor and EditorManager.activeEditor are both undefined when theia is loaded, even if there is an editor rendered in the main panel.

bug

Most helpful comment

@kittaakos When one refreshes the page, editors are restored by the shell but the state in the editor manager is not, either because the shell does not fire events or the editor manager misses them. It is bogus and leads to follow up errors, like triggering the command from the status bar cannot find an active editor in https://github.com/theia-ide/theia/pull/1512, although a user sees one.

All 18 comments

Do you have a stack trace of the error? Could you explain what did you do or wanted to do?

@kittaakos When one refreshes the page, editors are restored by the shell but the state in the editor manager is not, either because the shell does not fire events or the editor manager misses them. It is bogus and leads to follow up errors, like triggering the command from the status bar cannot find an active editor in https://github.com/theia-ide/theia/pull/1512, although a user sees one.

Hi @akosyakov , I did some investigation and I am not sure if there is an easy way to fix it.

From what I can tell from the code, the following describes how the EditorManager finds the current & active editor:

FocusTracker ---(emit activeChanged / currentChanged)---> ApplicationShell ---(emit activeChanged / currentChanged)---> EditorManager ---(update the active / current editor)

Therefore, with the current design, I personally do not think the code does the wrong thing, because the active & current editor is "focus-dependent", and when theia reloads, the user does not have chance to set focus to any widget before s/he sees the UI. So ... probably it makes sense for the editor manager to not have any knowledge of what the current & active editor is.

Maybe you would want to argue that, when theia is reloaded, the visible editor should be the current editor. I agree with you - but there could be multiple visible editors displaying at the same time - therefore we cannot just use the visibility to decide which one is the current.

One thing that @epatpol suggests is saving the current editor in the localStorage, and when theia is loaded, we use that layout string to restore EditorManager.currentEditor to the original state. It is a good idea, but I am not sure how to implement it, because the restoring logic is implemented in phosphor (see setLayoutData() function https://github.com/theia-ide/theia/blob/master/packages/core/src/browser/shell/application-shell.ts#L496), and we don't have access to those restoreLayout() functions.

Could you please give me some guidance & suggestion on this one? Thank you !

The active/current editor should be derived from the active/current widget. It is strange that the shell does not preserve this information as a part of the layout data. I am not sure why. @svenefftinge @spoenemann

@akosyakov IMO even if we have active/current widget info from the layout string, it is still possible that the current/active editor cannot be derived. Because the current/active widget is not necessarily an editor widget.

@akosyakov sorry please ignore my comment made half an hour ago. I am investigating this and find something interesting.

OK, the layout string does have information regarding which editors are visible and which ones are not, and we could iterate through all visible editor widgets from the application shell. But we got to determine which editor is the active editor in case that there are multiple visible editors in theia.

Could you @akosyakov please advise?

Thank you !

By definition of Phosphor, the _active_ widget is the one that has focus. If no widget has focus, there is no active widget. The same should apply to the concept of the active editor.

The _current_ widget is the last one that had focus.

@elaihau Could you try to refactor editor based status bar contributions as:

onStart(): void {
    this.updateStatusBar();
    this.editorManager.onCurrentEditorChanged(() => this.updateStatusBar());
}

protected updateStatusBar(): void {
   const current = this.editorManager.current;
   if (current) {
       // this.statusBar.setElement
   } else {
       // this.statusBar.removeElement
   }
}

It should ensure that on the start the status bar gets updated properly.

1586 Solves the general part of this, but still the problem remains when the last active widget is not an editor.

@spoenemann Thank you for your explanation of active vs current. Really helpful !

@akosyakov I am thinking of adding some code to the application-shell, so that it is able to arbitrarily determine which visible editor is the "current editor":
1) The first visible editor in the main panel
2) The first visible editor in the bottom panel
3) The first visible on the left panel
4) The first visible on the right panel

Is it something we should do, or it's an overkill?

could we do the same as JupyterLab? they store current for all widget containers. after reloading the app, the current state is same as before.

screen shot 2018-03-27 at 16 06 00

I am thinking of adding some code to the application-shell, so that it is able to arbitrarily determine which visible editor is the "current editor":

@elaihau There is always only one current widget ApplicationShell.currentWidget. There should not be another. The current editor is the last current widget which was an editor: EditorManager.currentEditor. It should not be changed.

I think with Miro's change and https://github.com/theia-ide/theia/issues/1570#issuecomment-376433213 the issue should be resolved.

could we do the same as JupyterLab?

@AlexTugarev in JLab they track widgets per a kind, we don't want to bring such complexity and force each widget contributor to track their widgets

@AlexTugarev

could we do the same as JupyterLab? they store current for all widget containers. after reloading the app, the current state is same as before.

In that context _current_ refers to which widget is visible in each tab bar. We do the same.

@elaihau Could you fix updating of the status bar in EditorContribution similar to what you did for the indentation? After that the issue could be closed.

As you could see from the attached image, we have a few buttons and labels in the status bar
screenshot from 2018-03-29 09-57-50

and only the "indentation config" relies on having a "current editor".

The cursor-location label (e.g., "Ln 2, Col 5"), and the language-type label (e.g., "Typescript") are editor-dependent however they are not clickable, so having them in the status bar does not break anything.

Therefore I guess we don't need to do anything more in the editor contribution ?

The cursor-location label (e.g., "Ln 2, Col 5"), and the language-type label (e.g., "Typescript") are editor-dependent however they are not clickable, so having them in the status bar does not break anything.

clickable or not is not important. The status bar data should match the active editor. Without changes in EditorContribution it still could be a case that they don't match.

Got it. I will change the editor contribution.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

Beetix picture Beetix  路  3Comments

vince-fugnitto picture vince-fugnitto  路  3Comments

jeanlucburot picture jeanlucburot  路  3Comments

vince-fugnitto picture vince-fugnitto  路  3Comments

vinokurig picture vinokurig  路  3Comments