Don't render Markdown when attempting to navigate above first cell

[firai]

Fixes #52. Disable Markdown rendering when the following keys are pressed in the first cell (if it is a Markdown cell), in order to keep CM focused:

• Ctrl+k
• k

[github-actions]

👈 Launch a Binder on branch firai/jupyterlab-vim/dont-render-first-md-cell

[firai]

This PR currently makes activateCellVim() and onActiveCellChanged() pass the activeCellIndex from the INotebookTracker into modifyCell(), in addition to the current argument/parameter of activeCell. As far as I can find from the JL API docs, the cell index is not otherwise available in the activeCell ICellModel currently passed to modifyCell().

The disadvantage of doing this is that we still don't have a way to identify the last cell, which we may want for future special last cell handling (say, if we wanted to auto-return to vim mode after coming back up from the footer after jupyterlab/jupyterlab#14796, or block vim mode from going to the footer altogether). I'm not sure if there are any needs for having the numerical cell index.

In lieu of passing the numerical cell index, it would be possible to make the caller pass a string or enum to identify first and last cells, with the calling function handling the identification. A dummy/other value would most likely be passed for any other cell location in these cases. It would also be possible to have both if we make the calling function change the passed cell index to -1 for the last cell, but that's, well, hacky.

[krassowski]

the cell index is not otherwise available in the activeCell ICellModel currently passed to modifyCell()

This is correct.

The disadvantage of doing this is that we still don't have a way to identify the last cell

(this can be taken from .widgets.length)

we make the calling function change the passed cell index to -1 for the last cell

-1 is already used to indicate no cells in notebook, see here (though this is implementation detail).

In lieu of passing the numerical cell index, it would be possible to make the caller pass a string or enum to identify first and last cells, with the calling function handling the identification.

Edge case: console. In console mode there is one cell that can be focused, the input cell at the bottom. There are cells above it which cannot be navigated to. Does it make the input cell the first and the last cell? While console is not currently vim enabled (I think?), it could be in the future.

I think the most future proof approach would be to make modifyCell receive two arguments:

• activeCell (as it does currently)
• context: ICellContext - this will allow to add/remove optional arguments in the future without worrying about argument index in calls; it could be defined as:

interface ICellContext {
  index?: number;
  cellCount?: number; // or `total`
  // in the future, if needed, we could add:
  // type: 'notebook' | 'console'
}

What do you think?

[firai]

Thanks for the suggestion, @krassowski!

After attempting to implementing this, _modifyEdgeNavigation() seems to be broken after modifyEditor() (i.e. vim is still enabled) for the first cell on JL reload and I can't navigate to the cell below with j, but everything works after jumping to another cell and back. This bug doesn't seem to exist at commit 0e00f0ffff0510ba4eb1655969b5528acfc135f0 (before I implement the interface). I see the following browser console error, but it doesn't seem to be related, as I get the same error with the commit before the interface. Do you have any suggestions on how to debug this?

P.s. I tried console logging the state of activeCellContext at each call, but the items exist at all logs (although the length is sometimes -1 during the process of loading). I don't see why _modifyEdgeNavigation() would break.

Completely scratching my head on this one, and I'm starting to think this might be related to #15 (comment) (but reverse).

modifyCell() and _modifyEdgeNavigation() get called several times when JL is loaded, but when this bug happens, there's somehow an extra modifyEditor() initiated by onActiveEditorChanged. Seems to be some sort of race condition, because it will very occasionally work. I don't know why changing to an interface causes the bug to happen more consistently/frequently. Maybe the extra code time is affecting the race condition.

EDIT: Consolidated my investigations on the bug that I thought was caused by the latest commit, but now does not appear to be.

The bug discussed in the collapsed sections, where modifyCell() does not get called for the first cell when JL is loaded with a notebook address (until focus switches to another cell), is caused a file editor in a background tab, which initiates an additional modifyEditor() (as a result of onActiveEditorChanged) on the first notebook cell. I thought this was caused by the change to using an interface, but it turns out I was using different conditions when I tested the two commits. Not a bug (probably) caused by this PR.

I think we should be otherwise good to go, pending code review.

[krassowski]

Thank you @firai, tested locally to confirm that this fixes the issue.