Skip to content

Commit

Permalink
Split standard input line history per session (notebook) (#13944)
Browse files Browse the repository at this point in the history 
* implemented split line history by kernel session in `Stdin` widget

* added `splitStdinHistoryBySession` preference to editorConfig

* finished implementation of `splitStdinHistoryBySession` preference

* Rename to `inputHistoryScope` and move to `INotebookConfig`

* Pass the setting to notebook config

* Use `Map` instead of object as suggested in review

---------

Co-authored-by: krassowski <5832902+krassowski@users.noreply.github.com>
  • Loading branch information
@telamonian @krassowski
telamonian and krassowski committed Mar 25, 2023
1 parent a3a0e3e commit 6d31bbb
Show file tree
Hide file tree
Showing 5 changed files with 89 additions and 22 deletions.
8 changes: 7 additions & 1 deletion packages/cells/src/widget.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -725,6 +725,11 @@ export namespace Cell {
*/
maxNumberOutputs?: number;

/**
* Whether to split stdin line history by kernel session or keep globally accessible.
*/
inputHistoryScope?: 'global' | 'session';

/**
* Whether this cell is a placeholder for future rendering.
*/
Expand Down Expand Up @@ -986,7 +991,8 @@ export class CodeCell extends Cell<ICodeCellModel> {
contentFactory: contentFactory,
maxNumberOutputs: this.maxNumberOutputs,
translator: this.translator,
promptOverlay: true
promptOverlay: true,
inputHistoryScope: options.inputHistoryScope
}));
output.addClass(CELL_OUTPUT_AREA_CLASS);
output.toggleScrolling.connect(() => {
Expand Down
7 changes: 7 additions & 0 deletions packages/notebook-extension/schema/tracker.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -688,6 +688,13 @@
"type": "boolean",
"default": false
},
"inputHistoryScope": {
"type": "string",
"default": "global",
"enum": ["global", "session"],
"title": "Input History Scope",
"description": "Whether the line history for standard input (e.g. the ipdb prompt) should kept separately for different kernel sessions (`session`) or combined (`global`)."
},
"kernelShutdown": {
"title": "Shut down kernel",
"description": "Whether to shut down or not the kernel when closing a notebook.",
Expand Down
3 changes: 3 additions & 0 deletions packages/notebook-extension/src/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1783,6 +1783,9 @@ function activateNotebookHandler(
defaultCell: settings.get('defaultCell').composite as nbformat.CellType,
recordTiming: settings.get('recordTiming').composite as boolean,
overscanCount: settings.get('overscanCount').composite as number,
inputHistoryScope: settings.get('inputHistoryScope').composite as
| 'global'
| 'session',
maxNumberOutputs: settings.get('maxNumberOutputs').composite as number,
showEditorForReadOnlyMarkdown: settings.get(
'showEditorForReadOnlyMarkdown'
Expand Down
7 changes: 7 additions & 0 deletions packages/notebook/src/widget.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -653,6 +653,7 @@ export class StaticNotebook extends WindowedList {
const options: CodeCell.IOptions = {
contentFactory,
editorConfig,
inputHistoryScope: this.notebookConfig.inputHistoryScope,
maxNumberOutputs: this.notebookConfig.maxNumberOutputs,
model,
placeholder: this._notebookConfig.windowingMode !== 'none',
Expand Down Expand Up @@ -1049,6 +1050,11 @@ export namespace StaticNotebook {
*/
maxNumberOutputs: number;

/**
* Whether to split stdin line history by kernel session or keep globally accessible.
*/
inputHistoryScope: 'global' | 'session';

/**
* Number of cells to render in addition to those
* visible in the viewport.
Expand Down Expand Up @@ -1119,6 +1125,7 @@ export namespace StaticNotebook {
scrollPastEnd: true,
defaultCell: 'code',
recordTiming: false,
inputHistoryScope: 'global',
maxNumberOutputs: 50,
showEditorForReadOnlyMarkdown: true,
disableDocumentWideUndoRedo: true,
Expand Down
86 changes: 65 additions & 21 deletions packages/outputarea/src/widget.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ export class OutputArea extends Widget {
this.rendermime = options.rendermime;
this._maxNumberOutputs = options.maxNumberOutputs ?? Infinity;
this._translator = options.translator ?? nullTranslator;
this._inputHistoryScope = options.inputHistoryScope ?? 'global';

const model = (this.model = options.model);
for (
Expand Down Expand Up @@ -439,7 +440,8 @@ export class OutputArea extends Widget {
prompt: stdinPrompt,
password,
future,
translator: this._translator
translator: this._translator,
inputHistoryScope: this._inputHistoryScope
});
input.addClass(OUTPUT_AREA_OUTPUT_CLASS);
panel.addWidget(input);
Expand Down Expand Up @@ -727,6 +729,7 @@ export class OutputArea extends Widget {
namespace: UUID.uuid4()
});
private _translator: ITranslator;
private _inputHistoryScope: 'global' | 'session' = 'global';
}

export class SimplifiedOutputArea extends OutputArea {
Expand Down Expand Up @@ -789,6 +792,11 @@ export namespace OutputArea {
* Translator
*/
readonly translator?: ITranslator;

/**
* Whether to split stdin line history by kernel session or keep globally accessible.
*/
inputHistoryScope?: 'global' | 'session';
}

/**
Expand Down Expand Up @@ -942,41 +950,52 @@ export interface IStdin extends Widget {
* The default stdin widget.
*/
export class Stdin extends Widget implements IStdin {
private static _history: string[] = [];
private static _history: Map<string, string[]> = new Map();

private static _historyIx(ix: number): number | undefined {
const len = Stdin._history.length;
private static _historyIx(key: string, ix: number): number | undefined {
const history = Stdin._history.get(key);
if (!history) {
return undefined;
}
const len = history.length;
// wrap nonpositive ix to nonnegative ix
if (ix <= 0) {
return len + ix;
}
}

private static _historyAt(ix: number): string | undefined {
const len = Stdin._history.length;
const ixpos = Stdin._historyIx(ix);
private static _historyAt(key: string, ix: number): string | undefined {
const history = Stdin._history.get(key);
if (!history) {
return undefined;
}
const len = history.length;
const ixpos = Stdin._historyIx(key, ix);

if (ixpos !== undefined && ixpos < len) {
return Stdin._history[ixpos];
return history[ixpos];
}
// return undefined if ix is out of bounds
}

private static _historyPush(line: string): void {
Stdin._history.push(line);
if (Stdin._history.length > 1000) {
private static _historyPush(key: string, line: string): void {
const history = Stdin._history.get(key)!;
history.push(line);
if (history.length > 1000) {
// truncate line history if it's too long
Stdin._history.shift();
history.shift();
}
}

private static _historySearch(
key: string,
pat: string,
ix: number,
reverse = true
): number | undefined {
const len = Stdin._history.length;
const ixpos = Stdin._historyIx(ix);
const history = Stdin._history.get(key)!;
const len = history.length;
const ixpos = Stdin._historyIx(key, ix);
const substrFound = (x: string) => x.search(pat) !== -1;

if (ixpos === undefined) {
Expand All @@ -989,7 +1008,7 @@ export class Stdin extends Widget implements IStdin {
return;
}

const ixFound = (Stdin._history.slice(0, ixpos) as any).findLastIndex(
const ixFound = (history.slice(0, ixpos) as any).findLastIndex(
substrFound
);
if (ixFound !== -1) {
Expand All @@ -1002,7 +1021,7 @@ export class Stdin extends Widget implements IStdin {
return;
}

const ixFound = Stdin._history.slice(ixpos + 1).findIndex(substrFound);
const ixFound = history.slice(ixpos + 1).findIndex(substrFound);
if (ixFound !== -1) {
// wrap ix to negative and adjust for slice
return ixFound - len + ixpos + 1;
Expand All @@ -1020,6 +1039,10 @@ export class Stdin extends Widget implements IStdin {
this.addClass(STDIN_CLASS);
this._future = options.future;
this._historyIndex = 0;
this._historyKey =
options.inputHistoryScope === 'session'
? options.parent_header.session
: '';
this._historyPat = '';
this._parentHeader = options.parent_header;
this._password = options.password;
Expand All @@ -1031,6 +1054,11 @@ export class Stdin extends Widget implements IStdin {
this._input.placeholder = this._trans.__(
'↑↓ for history. Search history with c-↑/c-↓'
);

// initialize line history
if (!Stdin._history.has(this._historyKey)) {
Stdin._history.set(this._historyKey, []);
}
}

/**
Expand Down Expand Up @@ -1068,7 +1096,7 @@ export class Stdin extends Widget implements IStdin {
this._value += '········';
} else {
this._value += input.value;
Stdin._historyPush(input.value);
Stdin._historyPush(this._historyKey, input.value);
}
this._promise.resolve(void 0);
} else if (event.key === 'Escape') {
Expand All @@ -1086,13 +1114,17 @@ export class Stdin extends Widget implements IStdin {

const reverse = event.key === 'ArrowUp';
const searchHistoryIx = Stdin._historySearch(
this._historyKey,
this._historyPat,
this._historyIndex,
reverse
);

if (searchHistoryIx !== undefined) {
const historyLine = Stdin._historyAt(searchHistoryIx);
const historyLine = Stdin._historyAt(
this._historyKey,
searchHistoryIx
);
if (historyLine !== undefined) {
if (this._historyIndex === 0) {
this._valueCache = input.value;
Expand All @@ -1105,7 +1137,10 @@ export class Stdin extends Widget implements IStdin {
} else if (event.key === 'ArrowUp') {
this.resetSearch();

const historyLine = Stdin._historyAt(this._historyIndex - 1);
const historyLine = Stdin._historyAt(
this._historyKey,
this._historyIndex - 1
);
if (historyLine) {
if (this._historyIndex === 0) {
this._valueCache = input.value;
Expand All @@ -1122,7 +1157,10 @@ export class Stdin extends Widget implements IStdin {
input.value = this._valueCache;
++this._historyIndex;
} else {
const historyLine = Stdin._historyAt(this._historyIndex + 1);
const historyLine = Stdin._historyAt(
this._historyKey,
this._historyIndex + 1
);
if (historyLine) {
input.value = historyLine;
++this._historyIndex;
Expand Down Expand Up @@ -1153,11 +1191,12 @@ export class Stdin extends Widget implements IStdin {

private _future: Kernel.IShellFuture;
private _historyIndex: number;
private _historyKey: string;
private _historyPat: string;
private _input: HTMLInputElement;
private _parentHeader: KernelMessage.IInputReplyMsg['parent_header'];
private _password: boolean;
private _promise = new PromiseDelegate<void>();
private _historyPat: string;
private _trans: TranslationBundle;
private _value: string;
private _valueCache: string;
Expand Down Expand Up @@ -1192,6 +1231,11 @@ export namespace Stdin {
* Translator
*/
readonly translator?: ITranslator;

/**
* Whether to split stdin line history by kernel session or keep globally accessible.
*/
inputHistoryScope?: 'global' | 'session';
}
}

Expand Down

0 comments on commit 6d31bbb

Please sign in to comment.