fix: initialize terminal with correct size (#10369)

* Fit once during creation

This does not fix any bugs (that I know of) but we only need to fit once
when the terminal is created, not every time we reconnect.  Granted,
currently we do not support reconnecting without refreshing anyway so it
does not really matter, but this just seems more correct.

Plus now we will not have to pass the fit addon around.

* Pass size when connecting web socket URL

I think this will solve an issue where screen does does not correctly
handle an immediate resize.  It seems to ignore the resize, but even if
you send it again nothing changes, seemingly thinking it is already at
that size?

* Use new struct for decoding reconnecting pty requests

Decoding a JSON message does not touch omitted (or null) fields so once
a message with a resize comes in, every single message from that point
will cause a resize.

I am not sure if this is an actual problem in practice but at the very
least it seems unintentional.
This commit is contained in:
Asher
2023-10-23 23:42:39 -04:00
committed by GitHub
parent 1286904de8
commit 4af8446f48
3 changed files with 21 additions and 16 deletions
+1 -1
View File
@@ -196,8 +196,8 @@ func (s *ptyState) waitForStateOrContext(ctx context.Context, state State) (Stat
// until EOF or an error writing to ptty or reading from conn.
func readConnLoop(ctx context.Context, conn net.Conn, ptty pty.PTYCmd, metrics *prometheus.CounterVec, logger slog.Logger) {
decoder := json.NewDecoder(conn)
var req codersdk.ReconnectingPTYRequest
for {
var req codersdk.ReconnectingPTYRequest
err := decoder.Decode(&req)
if xerrors.Is(err, io.EOF) {
return
+16 -15
View File
@@ -54,7 +54,6 @@ const TerminalPage: FC = () => {
const [terminalState, setTerminalState] = useState<
"connected" | "disconnected" | "initializing"
>("initializing");
const [fitAddon, setFitAddon] = useState<FitAddon | null>(null);
const [searchParams] = useSearchParams();
// The reconnection token is a unique token that identifies
// a terminal session. It's generated by the client to reduce
@@ -125,7 +124,6 @@ const TerminalPage: FC = () => {
terminal.loadAddon(new CanvasAddon());
}
const fitAddon = new FitAddon();
setFitAddon(fitAddon);
terminal.loadAddon(fitAddon);
terminal.loadAddon(new Unicode11Addon());
terminal.unicode.activeVersion = "11";
@@ -134,13 +132,21 @@ const TerminalPage: FC = () => {
handleWebLinkRef.current(uri);
}),
);
setTerminal(terminal);
terminal.open(xtermRef.current);
const listener = () => {
// This will trigger a resize event on the terminal.
fitAddon.fit();
};
// We have to fit twice here. It's unknown why, but the first fit will
// overflow slightly in some scenarios. Applying a second fit resolves this.
fitAddon.fit();
fitAddon.fit();
// This will trigger a resize event on the terminal.
const listener = () => fitAddon.fit();
window.addEventListener("resize", listener);
// Terminal is correctly sized and is ready to be used.
setTerminal(terminal);
return () => {
window.removeEventListener("resize", listener);
terminal.dispose();
@@ -165,16 +171,10 @@ const TerminalPage: FC = () => {
// Hook up the terminal through a web socket.
useEffect(() => {
if (!terminal || !fitAddon) {
if (!terminal) {
return;
}
// We have to fit twice here. It's unknown why, but
// the first fit will overflow slightly in some
// scenarios. Applying a second fit resolves this.
fitAddon.fit();
fitAddon.fit();
// The terminal should be cleared on each reconnect
// because all data is re-rendered from the backend.
terminal.clear();
@@ -229,6 +229,8 @@ const TerminalPage: FC = () => {
reconnectionToken,
workspaceAgent.id,
command,
terminal.rows,
terminal.cols,
)
.then((url) => {
if (disposed) {
@@ -289,7 +291,6 @@ const TerminalPage: FC = () => {
};
}, [
command,
fitAddon,
proxy.preferredPathAppURL,
reconnectionToken,
terminal,
+4
View File
@@ -5,11 +5,15 @@ export const terminalWebsocketUrl = async (
reconnect: string,
agentId: string,
command: string | undefined,
height: number,
width: number,
): Promise<string> => {
const query = new URLSearchParams({ reconnect });
if (command) {
query.set("command", command);
}
query.set("height", height.toString());
query.set("width", width.toString());
const url = new URL(baseUrl || `${location.protocol}//${location.host}`);
url.protocol = url.protocol === "https:" ? "wss:" : "ws:";