Skip to content

Commit 6bdcae7

Browse files
committed
fix: double-echo in phoenix
1 parent a33d721 commit 6bdcae7

File tree

4 files changed

+78
-0
lines changed

4 files changed

+78
-0
lines changed

src/phoenix/doc/devlog.md

+40
Original file line numberDiff line numberDiff line change
@@ -1150,3 +1150,43 @@ This change can be made incrementally as follows:
11501150
- update the command to use the new implementation
11511151
- Once all commands are updated, the XDocumentPuterShell class will
11521152
be dormant and can safely be removed.
1153+
1154+
## 2024-09-04
1155+
1156+
### Terminal I/O
1157+
1158+
Prior to recent changes it was not possible to run phoenix shell inside
1159+
another instance of phoenix shell. The following had to be resolved for
1160+
this to work:
1161+
- Phoenix was waiting for a configuration object from the parent app,
1162+
under the assumption that this parent app is a terminal. Phoenix now
1163+
does not require this configuration object.
1164+
- Initial terminal size had to be requested for by Phoenix after some
1165+
initialization to avoid a race condition.
1166+
- Apps called by an application under a terminal were not able to control
1167+
`echo` behavior.
1168+
1169+
The new IO functionality follows the
1170+
[Selective Layer Implementation Pattern](https://puter.com/@ed/documentation/glossary.md##Selective-Layer-Implementation-Pattern)
1171+
with the following implemented:
1172+
- IOCTL: `TIOCGWINSZ`, sent via postMessage
1173+
- signal: `ioctl.set` event to simulate "SIGWINCH",
1174+
but this should be merged with existing code that
1175+
implements the concept of signals.
1176+
- ptt.termios: a high-level mimic of termios
1177+
(currently only echo control is implemented)
1178+
1179+
XDocumentPTT now implements `TIOCGWINSZ` (named after the POSIX equivalent)
1180+
which requests the window size. This function calls `postMessage` on the
1181+
AppConnection with the following
1182+
[type-tagged object](../../../doc/api/type-tagged.md):
1183+
`{ $: 'ioctl.request', requestCode: 104 }`.
1184+
1185+
The window size information is still provided
1186+
by the badly-named `ioctl.set` event, which should later be changed to
1187+
a similar convention, like `{ $: 'signal', code: 28 }`.
1188+
1189+
The termios implementation is a high-level mimic and does not actually
1190+
use the underlying IOCTL implementation. Instead it sends a type-tagged
1191+
object that looks like `{ $: 'chtermios', termios: { echo: false } }`,
1192+
where `echo` can be `true` or `false`.

src/phoenix/src/pty/XDocumentPTT.js

+27
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,18 @@ export class XDocumentPTT {
4141
};
4242
}
4343

44+
this.termios = new Proxy({}, {
45+
set (target, k, v) {
46+
terminalConnection.postMessage({
47+
$: 'chtermios',
48+
termios: {
49+
[k]: v,
50+
}
51+
});
52+
return Reflect.set(target, k, v);
53+
}
54+
});
55+
4456
this.ioctl_listeners = {};
4557

4658
this.readableStream = new ReadableStream({
@@ -91,4 +103,19 @@ export class XDocumentPTT {
91103
listener(evt);
92104
}
93105
}
106+
107+
once (name, listener) {
108+
const wrapper = evt => {
109+
listener(evt);
110+
this.off(name, wrapper);
111+
};
112+
this.on(name, wrapper);
113+
}
114+
115+
off (name, listener) {
116+
if ( ! this.ioctl_listeners.hasOwnProperty(name) ) return;
117+
this.ioctl_listeners[name] = this.ioctl_listeners[name].filter(
118+
l => l !== listener
119+
);
120+
}
94121
}

src/phoenix/src/puter-shell/main.js

+1
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,7 @@ export const launchPuterShell = async (ctx) => {
138138

139139
// NEXT
140140
ptt.TIOCGWINSZ();
141+
ptt.termios.echo = false;
141142

142143
const fire = (text) => {
143144
// Define fire-like colors (ANSI 256-color codes)

src/phoenix/src/puter-shell/providers/PuterAppCommandProvider.js

+10
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,15 @@ export class PuterAppCommandProvider {
8888
if (message.$ === 'stdout') {
8989
ctx.externs.out.write(decoder.decode(message.data));
9090
}
91+
if (message.$ === 'chtermios') {
92+
if ( message.termios.echo !== undefined ) {
93+
if ( message.termios.echo ) {
94+
ctx.externs.echo.on();
95+
} else {
96+
ctx.externs.echo.off();
97+
}
98+
}
99+
}
91100
});
92101

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

120+
// TODO: propagate sigint to the app
111121
return Promise.race([ app_close_promise, sigint_promise ]);
112122
}
113123
};

0 commit comments

Comments
 (0)