Skip to content

Add clearOutputs() method to ISharedCodeCell #330

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Darshan808
Copy link

References jupyterlab/jupyterlab#17632

Description

This PR introduces a clearOutputs() method to the ISharedCodeCell interface and its implementation in YCodeCell. This method provides a clean and direct way to remove all outputs from a code cell in the shared model.

This change is motivated by a bug in JupyterLab where uncoalesced stream outputs can persist in the cell output area. The root cause is that the frontend coalesces outputs, but the shared model retains multiple entries, making it tricky to clear all outputs using updateOutputs() when their lengths differ.

/**
* Clear all outputs from the cell.
*/
clearOutputs(): void;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing origin?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The origin parameter wasn’t exposed in the updateOutputs() method, so I followed the same pattern and didn’t expose it in clearOutputs() either.

updateOutputs(
start: number,
end: number,
outputs: Array<nbformat.IOutput>
): void;

I’m not entirely sure about the reasoning behind this, but if exposing it in the interface is something we should do, I’m happy to make that change.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do we need origin for in the first place? E.g. this one does not have origin:

setOutputs(outputs: Array<nbformat.IOutput>): void {
this.transact(() => {
this._youtputs.delete(0, this._youtputs.length);
const newOutputs = this.createOutputs(outputs);
this._youtputs.insert(0, newOutputs);
}, false);
}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But this one does:

removeStreamOutput(index: number, start: number, origin: any = null): void {
this.transact(
() => {
const output = this._youtputs.get(index);
const prevText = output.get('text') as Y.Text;
const length = prevText.length - start;
prevText.delete(start, length);
},
false,
origin
);
}

@krassowski krassowski added the enhancement New feature or request label Jun 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants