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

Carriage return perf #6304

Merged
merged 4 commits into from May 4, 2019
Merged
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
145 changes: 60 additions & 85 deletions packages/outputarea/src/model.ts
Expand Up @@ -12,8 +12,6 @@ import { nbformat } from '@jupyterlab/coreutils';
import {
IObservableList,
ObservableList,
IObservableValue,
ObservableValue,
IModelDB
} from '@jupyterlab/observables';

Expand Down Expand Up @@ -153,19 +151,6 @@ export class OutputAreaModel implements IOutputAreaModel {
});
}
this.list.changed.connect(this._onListChanged, this);

// If we are given a IModelDB, keep an up-to-date
// serialized copy of the OutputAreaModel in it.
if (options.modelDB) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we also delete the modelDB option in the options interface?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure, I guess that's a breaking change, but one that I am pretty sure was used nowhere but in the context of Google RTC.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you note it above in the breaking changes section?

Copy link
Contributor

Choose a reason for hiding this comment

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

oh, you already did. Thanks!

this._modelDB = options.modelDB;
this._serialized = this._modelDB.createValue('outputs');
if (this._serialized.get()) {
this.fromJSON(this._serialized.get() as nbformat.IOutput[]);
} else {
this._serialized.set(this.toJSON());
}
this._serialized.changed.connect(this._onSerializedChanged, this);
}
}

/**
Expand Down Expand Up @@ -252,7 +237,7 @@ export class OutputAreaModel implements IOutputAreaModel {
*/
set(index: number, value: nbformat.IOutput): void {
// Normalize stream data.
this._normalize(value);
Private.normalize(value);
let item = this._createItem({ value, trusted: this._trusted });
this.list.set(index, item);
}
Expand Down Expand Up @@ -318,7 +303,7 @@ export class OutputAreaModel implements IOutputAreaModel {
let trusted = this._trusted;

// Normalize the value.
this._normalize(value);
Private.normalize(value);

// Consolidate outputs if they are stream outputs of the same kind.
if (
Expand All @@ -330,8 +315,8 @@ export class OutputAreaModel implements IOutputAreaModel {
// text to the current item and replace the previous item.
// This also replaces the metadata of the last item.
this._lastStream += value.text as string;
this._lastStream = Private.removeOverwrittenChars(this._lastStream);
value.text = this._lastStream;
this._removeOverwrittenChars(value);
let item = this._createItem({ value, trusted });
let index = this.length - 1;
let prev = this.list.get(index);
Expand All @@ -341,7 +326,7 @@ export class OutputAreaModel implements IOutputAreaModel {
}

if (nbformat.isStream(value)) {
this._removeOverwrittenChars(value);
value.text = Private.removeOverwrittenChars(value.text as string);
}

// Create the new item.
Expand All @@ -360,53 +345,15 @@ export class OutputAreaModel implements IOutputAreaModel {
}

/**
* Normalize an output.
* A flag that is set when we want to clear the output area
* *after* the next addition to it.
*/
private _normalize(value: nbformat.IOutput): void {
if (nbformat.isStream(value)) {
if (Array.isArray(value.text)) {
value.text = (value.text as string[]).join('\n');
}
}
}

/**
* Remove characters that are overridden by backspace characters.
*/
private _fixBackspace(txt: string): string {
let tmp = txt;
do {
txt = tmp;
// Cancel out anything-but-newline followed by backspace
tmp = txt.replace(/[^\n]\x08/gm, '');
} while (tmp.length < txt.length);
return txt;
}
protected clearNext = false;

/**
* Remove chunks that should be overridden by the effect of
* carriage return characters.
* An observable list containing the output models
* for this output area.
*/
private _fixCarriageReturn(txt: string): string {
txt = txt.replace(/\r+\n/gm, '\n'); // \r followed by \n --> newline
while (txt.search(/\r[^$]/g) > -1) {
const base = txt.match(/^(.*)\r+/m)[1];
let insert = txt.match(/\r+(.*)$/m)[1];
insert = insert + base.slice(insert.length, base.length);
txt = txt.replace(/\r+.*$/m, '\r').replace(/^.*\r/m, insert);
}
return txt;
}

/*
* Remove characters overridden by backspaces and carriage returns
*/
private _removeOverwrittenChars(value: nbformat.IOutput): void {
let tmp = value.text as string;
value.text = this._fixCarriageReturn(this._fixBackspace(tmp));
}

protected clearNext = false;
protected list: IObservableList<IOutputModel> = null;

/**
Expand All @@ -426,29 +373,9 @@ export class OutputAreaModel implements IOutputAreaModel {
sender: IObservableList<IOutputModel>,
args: IObservableList.IChangedArgs<IOutputModel>
) {
if (this._serialized && !this._changeGuard) {
this._changeGuard = true;
this._serialized.set(this.toJSON());
this._changeGuard = false;
}
this._changed.emit(args);
}

/**
* If the serialized version of the outputs have changed due to a remote
* action, then update the model accordingly.
*/
private _onSerializedChanged(
sender: IObservableValue,
args: ObservableValue.IChangedArgs
) {
if (!this._changeGuard) {
this._changeGuard = true;
this.fromJSON(args.newValue as nbformat.IOutput[]);
this._changeGuard = false;
}
}

/**
* Handle a change to an item.
*/
Expand All @@ -462,9 +389,6 @@ export class OutputAreaModel implements IOutputAreaModel {
private _isDisposed = false;
private _stateChanged = new Signal<IOutputAreaModel, void>(this);
private _changed = new Signal<this, IOutputAreaModel.ChangedArgs>(this);
private _modelDB: IModelDB = null;
private _serialized: IObservableValue = null;
private _changeGuard = false;
}

/**
Expand All @@ -488,3 +412,54 @@ export namespace OutputAreaModel {
*/
export const defaultContentFactory = new ContentFactory();
}

/**
* A namespace for module-private functionality.
*/
namespace Private {
/**
* Normalize an output.
*/
export function normalize(value: nbformat.IOutput): void {
if (nbformat.isStream(value)) {
if (Array.isArray(value.text)) {
value.text = (value.text as string[]).join('\n');
}
}
}

/**
* Remove characters that are overridden by backspace characters.
*/
function fixBackspace(txt: string): string {
let tmp = txt;
do {
txt = tmp;
// Cancel out anything-but-newline followed by backspace
tmp = txt.replace(/[^\n]\x08/gm, '');
} while (tmp.length < txt.length);
return txt;
}

/**
* Remove chunks that should be overridden by the effect of
* carriage return characters.
*/
function fixCarriageReturn(txt: string): string {
txt = txt.replace(/\r+\n/gm, '\n'); // \r followed by \n --> newline
while (txt.search(/\r[^$]/g) > -1) {
const base = txt.match(/^(.*)\r+/m)[1];
let insert = txt.match(/\r+(.*)$/m)[1];
insert = insert + base.slice(insert.length, base.length);
txt = txt.replace(/\r+.*$/m, '\r').replace(/^.*\r/m, insert);
}
return txt;
}

/*
* Remove characters overridden by backspaces and carriage returns
*/
export function removeOverwrittenChars(text: string): string {
return fixCarriageReturn(fixBackspace(text));
}
}