Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not add an empty cell when an entire Markdown cell gets extracted into a virtual document #15870

Open
krassowski opened this issue Feb 25, 2024 · 0 comments

Comments

@krassowski
Copy link
Member

Description

JupyterLab 4 added support for Markdown documents by using extractors mechanisms; it seems that markdown cells are erroneously included as empty cells in the resulting Python document.

It seems that the logic in TextForeignCodeExtractor and in the extractForeignCode() or appendCodeBlock() need to be modified so that when an entire cell gets extracted no cell would be added (rather than adding an empty code cell, padded by two empty lines on each side).

Reproduce

See jupyterlab-lsp issues:

Expected behavior

When an entire cell is extracted do not add new lines before and after it.

This could be solved by special casing the logic in:

appendCodeBlock(
block: Document.ICodeBlockOptions,
editorShift: CodeEditor.IPosition = { line: 0, column: 0 },
virtualShift?: CodeEditor.IPosition
): void {
let cellCode = block.value;
let ceEditor = block.ceEditor;
if (this.isDisposed) {
console.warn('Cannot append code block: document disposed');
return;
}
let sourceCellLines = cellCode.split('\n');
let { lines, foreignDocumentsMap } = this.prepareCodeBlock(
block,
editorShift
);
for (let i = 0; i < lines.length; i++) {
this.virtualLines.set(this.lastVirtualLine + i, {
skipInspect: [],
editor: ceEditor,
// TODO this is incorrect, wont work if something was extracted
sourceLine: this.lastSourceLine + i
});
}
for (let i = 0; i < sourceCellLines.length; i++) {
this.sourceLines.set(this.lastSourceLine + i, {
editorLine: i,
editorShift: {
line: editorShift.line - (virtualShift?.line || 0),
column: i === 0 ? editorShift.column - (virtualShift?.column || 0) : 0
},
// TODO: move those to a new abstraction layer (DocumentBlock class)
editor: ceEditor,
foreignDocumentsMap,
// TODO this is incorrect, wont work if something was extracted
virtualLine: this.lastVirtualLine + i
});
}
this.lastVirtualLine += lines.length;
// one empty line is necessary to separate code blocks, next 'n' lines are to silence linters;
// the final cell does not get the additional lines (thanks to the use of join, see below)
this.lineBlocks.push(lines.join('\n') + '\n');
// adding the virtual lines for the blank lines
for (let i = 0; i < this.blankLinesBetweenCells; i++) {
this.virtualLines.set(this.lastVirtualLine + i, {
skipInspect: [this.idPath],
editor: ceEditor,
sourceLine: null
});
}
this.lastVirtualLine += this.blankLinesBetweenCells;
this.lastSourceLine += sourceCellLines.length;
}

For example if lines is just one empty line we could skip adding this.blankLinesBetweenCells. This need to be conditional on foreignDocumentsMap being non-empty so that it does not lead to false negatives (e.g. undetected empty code cells),

Context

  • JupyterLab version: 4.1.2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants