Skip to content

Commit

Permalink
Backport PR #12376 on branch 3.3.x (Add parent header to input reply …
Browse files Browse the repository at this point in the history
…kernel message) (#12406)

* Backport PR #12376: Add parent header to input reply kernel message

* Make new argument optional

* Fix linter

Co-authored-by: David Brochart <david.brochart@gmail.com>
Co-authored-by: Frédéric Collonval <fcollonval@users.noreply.github.com>
  • Loading branch information
3 people committed Apr 14, 2022
1 parent 1856413 commit 298e86b
Show file tree
Hide file tree
Showing 6 changed files with 124 additions and 12 deletions.
21 changes: 17 additions & 4 deletions packages/outputarea/src/widget.ts
Original file line number Diff line number Diff line change
Expand Up @@ -366,6 +366,7 @@ export class OutputArea extends Widget {
panel.addWidget(prompt);

const input = factory.createStdin({
parent_header: msg.header,
prompt: stdinPrompt,
password,
future
Expand Down Expand Up @@ -857,6 +858,7 @@ export class Stdin extends Widget implements IStdin {
this._input = this.node.getElementsByTagName('input')[0];
this._input.focus();
this._future = options.future;
this._parent_header = options.parent_header;
this._value = options.prompt + ' ';
}

Expand All @@ -882,10 +884,13 @@ export class Stdin extends Widget implements IStdin {
if (event.type === 'keydown') {
if ((event as KeyboardEvent).keyCode === 13) {
// Enter
this._future.sendInputReply({
status: 'ok',
value: input.value
});
this._future.sendInputReply(
{
status: 'ok',
value: input.value
},
this._parent_header
);
if (input.type === 'password') {
this._value += Array(input.value.length + 1).join('·');
} else {
Expand Down Expand Up @@ -918,6 +923,9 @@ export class Stdin extends Widget implements IStdin {
this._input.removeEventListener('keydown', this);
}

private _parent_header:
| KernelMessage.IInputReplyMsg['parent_header']
| undefined;
private _future: Kernel.IShellFuture;
private _input: HTMLInputElement;
private _value: string;
Expand All @@ -943,6 +951,11 @@ export namespace Stdin {
* The kernel future associated with the request.
*/
future: Kernel.IShellFuture;

/**
* The header of the input_request message.
*/
parent_header?: KernelMessage.IInputReplyMsg['parent_header'];
}
}

Expand Down
23 changes: 23 additions & 0 deletions packages/outputarea/test/widget.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,29 @@ describe('outputarea/widget', () => {

describe('#createStdin()', () => {
it('should create a stdin widget', async () => {
const manager = new KernelManager();
const kernel = await manager.startNew();
const factory = new OutputArea.ContentFactory();
const future = kernel.requestExecute({ code: CODE });
const options = {
parent_header: {
date: '',
msg_id: '',
msg_type: 'input_request' as const,
session: '',
username: '',
version: ''
},
prompt: 'hello',
password: false,
future
};
expect(factory.createStdin(options)).toBeInstanceOf(Widget);
await kernel.shutdown();
kernel.dispose();
});

it('should create a stdin widget without parent header', async () => {
const manager = new KernelManager();
const kernel = await manager.startNew();
const factory = new OutputArea.ContentFactory();
Expand Down
8 changes: 7 additions & 1 deletion packages/services/src/kernel/default.ts
Original file line number Diff line number Diff line change
Expand Up @@ -816,14 +816,20 @@ export class KernelConnection implements Kernel.IKernelConnection {
* #### Notes
* See [Messaging in Jupyter](https://jupyter-client.readthedocs.io/en/latest/messaging.html#messages-on-the-stdin-router-dealer-sockets).
*/
sendInputReply(content: KernelMessage.IInputReplyMsg['content']): void {
sendInputReply(
content: KernelMessage.IInputReplyMsg['content'],
parent_header?: KernelMessage.IInputReplyMsg['parent_header']
): void {
const msg = KernelMessage.createMessage({
msgType: 'input_reply',
channel: 'stdin',
username: this._username,
session: this._clientId,
content
});
if (parent_header) {
msg.parent_header = parent_header;
}

this._sendMessage(msg);
this._anyMessage.emit({ msg, direction: 'send' });
Expand Down
7 changes: 5 additions & 2 deletions packages/services/src/kernel/future.ts
Original file line number Diff line number Diff line change
Expand Up @@ -153,8 +153,11 @@ export abstract class KernelFutureHandler<
/**
* Send an `input_reply` message.
*/
sendInputReply(content: KernelMessage.IInputReplyMsg['content']): void {
this._kernel.sendInputReply(content);
sendInputReply(
content: KernelMessage.IInputReplyMsg['content'],
parent_header?: KernelMessage.IInputReplyMsg['parent_header']
): void {
this._kernel.sendInputReply(content, parent_header);
}

/**
Expand Down
12 changes: 10 additions & 2 deletions packages/services/src/kernel/kernel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -363,11 +363,16 @@ export interface IKernelConnection extends IObservableDisposable {
* Send an `input_reply` message.
*
* @param content - The content of the reply.
* @param parent_header - The parent message header.
*
* #### Notes
* See [Messaging in Jupyter](https://jupyter-client.readthedocs.io/en/latest/messaging.html#messages-on-the-stdin-router-dealer-sockets).
* In v7.0.0, the `parent_header` argument will become mandatory.
*/
sendInputReply(content: KernelMessage.IInputReplyMsg['content']): void;
sendInputReply(
content: KernelMessage.IInputReplyMsg['content'],
parent_header?: KernelMessage.IInputReplyMsg['parent_header']
): void;

/**
* Create a new comm.
Expand Down Expand Up @@ -775,7 +780,10 @@ export interface IFuture<
/**
* Send an `input_reply` message.
*/
sendInputReply(content: KernelMessage.IInputReplyMsg['content']): void;
sendInputReply(
content: KernelMessage.IInputReplyMsg['content'],
parent_header?: KernelMessage.IInputReplyMsg['parent_header']
): void;
}

export interface IShellFuture<
Expand Down
65 changes: 62 additions & 3 deletions packages/services/test/kernel/ikernel.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,35 @@ describe('Kernel.IKernel', () => {

describe('#pendingInput', () => {
it('should be a signal following input request', async () => {
let called = false;
defaultKernel.pendingInput.connect((sender, args) => {
if (!called) {
called = true;
defaultKernel.sendInputReply(
{ status: 'ok', value: 'foo' },
{
date: '',
msg_id: '',
msg_type: 'input_request',
session: '',
username: '',
version: ''
}
);
}
});
const code = `input("Input something")`;
await defaultKernel.requestExecute(
{
code: code,
allow_stdin: true
},
true
).done;
expect(called).toBe(true);
});

it('should be a signal following input request without parent header', async () => {
let called = false;
defaultKernel.pendingInput.connect((sender, args) => {
if (!called) {
Expand Down Expand Up @@ -307,7 +336,17 @@ describe('Kernel.IKernel', () => {
expect(direction).toBe('send');
}
});
kernel.sendInputReply({ status: 'ok', value: 'foo' });
kernel.sendInputReply(
{ status: 'ok', value: 'foo' },
{
date: '',
msg_id: '',
msg_type: 'input_request',
session: '',
username: '',
version: ''
}
);
await emission;
});
});
Expand Down Expand Up @@ -868,7 +907,17 @@ describe('Kernel.IKernel', () => {
expect(msg.header.msg_type).toBe('input_reply');
done.resolve(undefined);
});
kernel.sendInputReply({ status: 'ok', value: 'test' });
kernel.sendInputReply(
{ status: 'ok', value: 'test' },
{
date: '',
msg_id: '',
msg_type: 'input_request',
session: '',
username: '',
version: ''
}
);
await done.promise;
await tester.shutdown();
tester.dispose();
Expand All @@ -885,7 +934,17 @@ describe('Kernel.IKernel', () => {
tester.sendStatus(UUID.uuid4(), 'dead');
await dead;
expect(() => {
kernel.sendInputReply({ status: 'ok', value: 'test' });
kernel.sendInputReply(
{ status: 'ok', value: 'test' },
{
date: '',
msg_id: '',
msg_type: 'input_request',
session: '',
username: '',
version: ''
}
);
}).toThrowError(/Kernel is dead/);
tester.dispose();
});
Expand Down

0 comments on commit 298e86b

Please sign in to comment.