Weird initialization order and event registration

[SPiCaRiA]

Please Review Before Posting!

• I checked that the bug does not happen in the CodeMirror Vim demo. If it does, please report it there and not here.
• I'm reasonably sure that this bug is indeed about the Vimrc file support and not a general Vim in Obsidian issue. If it's a general Vim issue, report it here.

Describe the bug:

In my PR, I simply update vimStatusBar element when:

1. The updateVimEvents method is called, indicating the vim mode change caused by file switching
2. The logVimModeChange method is called, indicating the vim-mode-change cm event just happened

However, I get the vimStatusBar is null error from the branch 1, updateVimEvents calls, when the plug-in loads. Therefore, I doubted something was wrong during the loading stage.

To Reproduce:

Just switch to my PR and run it on your Obsidian with the developer tool opened.

Problem Tracing

As suggested by the title of this issue, I think the initialization and event registrations are weird. If I'm understanding correctly (I'm sure not all, so plz let me know what you think!):

1. In the onload method, register the initialization logic on the active-leaf-change event.

• Since the this.initialized and this.codeMirrorVimObject.loadedVimrc flags block all the following codes after the first run, I guess we might want to use workspace.onLayoutReady instead.
• Isn't reading .vimrc part of the initialization? I'm not sure why the relevant codes are separated from the this.initialized method, any reasons? Or if adding an watch on any setting change for the vimrc file path is planned, I think it would totally make sense to have it in a separated function as callback.

async onload() {
  await this.loadSettings();
  this.addSettingTab(new SettingsTab(this.app, this));

  console.log("loading Vimrc plugin");

  this.app.workspace.on("active-leaf-change", async () => {
    if (!this.initialized)
      await this.initialize();
    if (this.codeMirrorVimObject.loadedVimrc)
      return;
    let fileName = this.settings.vimrcFileName;
    if (!fileName || fileName.trim().length === 0) {
      fileName = DEFAULT_SETTINGS.vimrcFileName;
      console.log(
        "Configured Vimrc file name is illegal, falling-back to default"
      );
    }
    let vimrcContent = "";
    try {
      vimrcContent = await this.app.vault.adapter.read(fileName);
    } catch (e) {
      console.log(
        "Error loading vimrc file",
        fileName,
        "from the vault root",
        e.message
      );
    }
    this.readVimInit(vimrcContent);
  });
}

2. Inside the this.initialize method, we registered event callbacks for the active-leaf-change and file-open events.

• At this stage, the first file-open event (for Obsidian's intial load) has not been fired yet. Therefore, the first fire will have the callbacks registered here, which includes the this.updateVimEvents method call (which calls my this.updateVimStatusBar method subsequently)
• However, we are still inside the this.initialize method and haven't gone to this.readVimInit yet (which creates the this.vimStatusBar element). At this time, the this.vimStatusBar element has not been created! Therefore, the this.updateVimEvents callback execution during the first file-open event will not be able to have a this.vimStatusBar element, and I believe this is what leads to the vimStatusBar is null bug.

async initialize() {
  if (this.initialized)
    return;

  this.codeMirrorVimObject = (window as any).CodeMirrorAdapter?.Vim;

  this.registerYankEvents(activeWindow);
  this.app.workspace.on("window-open", (workspaceWindow, w) => {
    this.registerYankEvents(w);
  })

  // Two events cos
  // this don't trigger on loading/reloading obsidian with note opened
  this.app.workspace.on("active-leaf-change", async () => {
    this.updateSelectionEvent();

    this.updateVimEvents();
  });
  // and this don't trigger on opening same file in new pane
  this.app.workspace.on("file-open", async () => {
    this.updateSelectionEvent();

    this.updateVimEvents();
  });

  this.initialized = true;
}

Solution

A quick solution that makes my PR work is to wrap the vim status bar update logics with a if (this.vimStatusBar !== null) check. However IMHO, it might worth refactoring the initialization stage a bit, which involves several questions that I'm not sure about:

1. Do we want to call this.updateVimEvents on the initial load of Obsidian?
2. If the answer to previous question is yes, can we hoist the this.readVimInit call to somewhere before, so that this.vimStatusBar is created when this.updateVimEvents callback is executed?

Thanks!