Skip to content

Commit

Permalink
fix: double-echo in phoenix
Browse files Browse the repository at this point in the history
  • Loading branch information
KernelDeimos committed Sep 5, 2024
1 parent a33d721 commit 6bdcae7
Show file tree
Hide file tree
Showing 4 changed files with 78 additions and 0 deletions.
40 changes: 40 additions & 0 deletions src/phoenix/doc/devlog.md
Original file line number Diff line number Diff line change
Expand Up @@ -1150,3 +1150,43 @@ This change can be made incrementally as follows:
- update the command to use the new implementation
- Once all commands are updated, the XDocumentPuterShell class will
be dormant and can safely be removed.

## 2024-09-04

### Terminal I/O

Prior to recent changes it was not possible to run phoenix shell inside
another instance of phoenix shell. The following had to be resolved for
this to work:
- Phoenix was waiting for a configuration object from the parent app,
under the assumption that this parent app is a terminal. Phoenix now
does not require this configuration object.
- Initial terminal size had to be requested for by Phoenix after some
initialization to avoid a race condition.
- Apps called by an application under a terminal were not able to control
`echo` behavior.

The new IO functionality follows the
[Selective Layer Implementation Pattern](https://puter.com/@ed/documentation/glossary.md##Selective-Layer-Implementation-Pattern)
with the following implemented:
- IOCTL: `TIOCGWINSZ`, sent via postMessage
- signal: `ioctl.set` event to simulate "SIGWINCH",
but this should be merged with existing code that
implements the concept of signals.
- ptt.termios: a high-level mimic of termios
(currently only echo control is implemented)

XDocumentPTT now implements `TIOCGWINSZ` (named after the POSIX equivalent)
which requests the window size. This function calls `postMessage` on the
AppConnection with the following
[type-tagged object](../../../doc/api/type-tagged.md):
`{ $: 'ioctl.request', requestCode: 104 }`.

The window size information is still provided
by the badly-named `ioctl.set` event, which should later be changed to
a similar convention, like `{ $: 'signal', code: 28 }`.

The termios implementation is a high-level mimic and does not actually
use the underlying IOCTL implementation. Instead it sends a type-tagged
object that looks like `{ $: 'chtermios', termios: { echo: false } }`,
where `echo` can be `true` or `false`.
27 changes: 27 additions & 0 deletions src/phoenix/src/pty/XDocumentPTT.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,18 @@ export class XDocumentPTT {
};
}

this.termios = new Proxy({}, {
set (target, k, v) {
terminalConnection.postMessage({
$: 'chtermios',
termios: {
[k]: v,
}
});
return Reflect.set(target, k, v);
}
});

this.ioctl_listeners = {};

this.readableStream = new ReadableStream({
Expand Down Expand Up @@ -91,4 +103,19 @@ export class XDocumentPTT {
listener(evt);
}
}

once (name, listener) {
const wrapper = evt => {
listener(evt);
this.off(name, wrapper);
};
this.on(name, wrapper);
}

off (name, listener) {
if ( ! this.ioctl_listeners.hasOwnProperty(name) ) return;
this.ioctl_listeners[name] = this.ioctl_listeners[name].filter(
l => l !== listener
);
}
}
1 change: 1 addition & 0 deletions src/phoenix/src/puter-shell/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ export const launchPuterShell = async (ctx) => {

// NEXT
ptt.TIOCGWINSZ();
ptt.termios.echo = false;

const fire = (text) => {
// Define fire-like colors (ANSI 256-color codes)
Expand Down
10 changes: 10 additions & 0 deletions src/phoenix/src/puter-shell/providers/PuterAppCommandProvider.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,15 @@ export class PuterAppCommandProvider {
if (message.$ === 'stdout') {
ctx.externs.out.write(decoder.decode(message.data));
}
if (message.$ === 'chtermios') {
if ( message.termios.echo !== undefined ) {
if ( message.termios.echo ) {
ctx.externs.echo.on();
} else {
ctx.externs.echo.off();
}
}
}
});

// Repeatedly copy data from stdin to the child, while it's running.
Expand All @@ -108,6 +117,7 @@ export class PuterAppCommandProvider {
setTimeout(next_data, 0);
}

// TODO: propagate sigint to the app
return Promise.race([ app_close_promise, sigint_promise ]);
}
};
Expand Down

0 comments on commit 6bdcae7

Please sign in to comment.