Closed firai closed 8 months ago
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 https://github.com/jupyterlab/jupyterlab/pull/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.
the cell index is not otherwise available in the
activeCell
ICellModel
currently passed tomodifyCell()
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?
Thanks for the suggestion, @krassowski!
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.
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: