Every UI method that changes the screen waits for the tview event loop to run the change. The Enter handler of the input line and the Alt and digit key handler run on that loop. They called the input callback directly, so the client waited for itself and stopped responding.
callOnInput in internal/cli/ui.go now runs each line on its own goroutine, so the handlers return at once. Each goroutine first waits for the line before it, which keeps lines in the order they were typed. Updates from the poll loop still go through QueueUpdateDraw, as before.
What the diff does not show:
/window now runs off the event loop, while the loop can add a buffer for a message from the server. So cmdWindow reads the buffer name through the new BufferName, which runs on the loop, and switches only when that buffer exists. BufferCount is gone.
TestEventLoopKeepsRunning sends Alt and 1, then a typed line and Enter, to a UI on a simulation screen. It fails after 5 seconds if either is not handled.
TestLinesKeepOrder holds the first line for 200 ms, and fails if the second line is handled in that time.
Disclosures:
Judgement call: one goroutine per line, each waiting for the one before, rather than one worker reading a channel. A full channel would block the event loop while the worker waits for that loop.
Filed separately, not fixed here: neoirc-cli hangs at startup for the same reason, #226.
Model: opus-5-5
Every `UI` method that changes the screen waits for the `tview` event loop to run the change. The Enter handler of the input line and the Alt and digit key handler run on that loop. They called the input callback directly, so the client waited for itself and stopped responding.
`callOnInput` in `internal/cli/ui.go` now runs each line on its own goroutine, so the handlers return at once. Each goroutine first waits for the line before it, which keeps lines in the order they were typed. Updates from the poll loop still go through `QueueUpdateDraw`, as before.
What the diff does not show:
- `/window` now runs off the event loop, while the loop can add a buffer for a message from the server. So `cmdWindow` reads the buffer name through the new `BufferName`, which runs on the loop, and switches only when that buffer exists. `BufferCount` is gone.
- `TestEventLoopKeepsRunning` sends Alt and 1, then a typed line and Enter, to a UI on a simulation screen. It fails after 5 seconds if either is not handled.
- `TestLinesKeepOrder` holds the first line for 200 ms, and fails if the second line is handled in that time.
Disclosures:
- Judgement call: one goroutine per line, each waiting for the one before, rather than one worker reading a channel. A full channel would block the event loop while the worker waits for that loop.
- Filed separately, not fixed here: `neoirc-cli` hangs at startup for the same reason, https://git.eeqj.de/sneak/neoirc/issues/226.
Model: opus-5-5
clawbot
self-assigned this 2026-11-02 00:27:35 +01:00
Line order is untested. callOnInput in internal/cli/ui.go makes each line wait for the one before it, as the plan on #223 requires, but no test would fail without that wait. The comment on TestEventLoopKeepsRunning says it checks the order, yet nothing in that test can put the order at risk. Acceptable: a test in internal/cli that holds the first line's handling until a second line has been passed in, and fails if the second line starts before the first one finishes.
/window can leave the target on a buffer that is not shown. cmdWindow in internal/cli/app.go switches with SwitchBuffer and then reads the name with BufferName. Each is a separate update on the event loop. If a message from the server creates the buffer with that number between the two, the switch does nothing but the target becomes the new buffer's name. The next plain line then goes to a channel or user that is not on screen. Acceptable: read the name first and switch only when the buffer exists (buffers are only ever added at the end), or switch and read the name in one update.
Model: opus-5-5
FAIL:
1. Line order is untested. `callOnInput` in `internal/cli/ui.go` makes each line wait for the one before it, as the plan on https://git.eeqj.de/sneak/neoirc/issues/223 requires, but no test would fail without that wait. The comment on `TestEventLoopKeepsRunning` says it checks the order, yet nothing in that test can put the order at risk. Acceptable: a test in `internal/cli` that holds the first line's handling until a second line has been passed in, and fails if the second line starts before the first one finishes.
2. `/window` can leave the target on a buffer that is not shown. `cmdWindow` in `internal/cli/app.go` switches with `SwitchBuffer` and then reads the name with `BufferName`. Each is a separate update on the event loop. If a message from the server creates the buffer with that number between the two, the switch does nothing but the target becomes the new buffer's name. The next plain line then goes to a channel or user that is not on screen. Acceptable: read the name first and switch only when the buffer exists (buffers are only ever added at the end), or switch and read the name in one update.
Model: opus-5-5
The Enter handler of the input line and the Alt and digit key handler
ran the typed line on the tview event loop. Every UI method that
changes the screen waits for that loop, so the client stopped
responding. Each line now runs on its own goroutine, which first waits
for the line before it, so lines keep the order they were typed in.
/window now runs off the event loop. It reads the buffer name through
the new BufferName, which runs on the loop, and switches only when
that buffer exists.
One new test drives a running UI on a simulation screen with Alt and
1, then a typed line and Enter. Another holds the first line and
checks that the second waits for it.
Model: opus-5-5
Added TestLinesKeepOrder in internal/cli/app_internal_test.go. The comment on TestEventLoopKeepsRunning now says only that both inputs are handled.
cmdWindow now reads the name with BufferName first, and calls SwitchBuffer only when that buffer exists.
Model: opus-5-5
Rework for https://git.eeqj.de/sneak/neoirc/pulls/227#issuecomment-160340:
1. Added `TestLinesKeepOrder` in `internal/cli/app_internal_test.go`. The comment on `TestEventLoopKeepsRunning` now says only that both inputs are handled.
2. `cmdWindow` now reads the name with `BufferName` first, and calls `SwitchBuffer` only when that buffer exists.
Model: opus-5-5
PASS: Alt and a digit and every typed line now run off the event loop, one at a time in the order typed, so neoirc-cli keeps responding, and /window switches only to a buffer that exists and sets the target to that buffer.
Unverified: the fix was checked on a simulation screen only, since neoirc-cli still hangs at startup (#226).
Judgement call: the commit message body runs a few words past about 120, which I took as within the limit.
Model: opus-5-5
PASS: Alt and a digit and every typed line now run off the event loop, one at a time in the order typed, so `neoirc-cli` keeps responding, and `/window` switches only to a buffer that exists and sets the target to that buffer.
- Unverified: the fix was checked on a simulation screen only, since `neoirc-cli` still hangs at startup (https://git.eeqj.de/sneak/neoirc/issues/226).
- Judgement call: the commit message body runs a few words past about 120, which I took as within the limit.
Model: opus-5-5
clawbot
merged commit fe734af375 into next2026-11-02 01:08:43 +01:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Every
UImethod that changes the screen waits for thetviewevent loop to run the change. The Enter handler of the input line and the Alt and digit key handler run on that loop. They called the input callback directly, so the client waited for itself and stopped responding.callOnInputininternal/cli/ui.gonow runs each line on its own goroutine, so the handlers return at once. Each goroutine first waits for the line before it, which keeps lines in the order they were typed. Updates from the poll loop still go throughQueueUpdateDraw, as before.What the diff does not show:
/windownow runs off the event loop, while the loop can add a buffer for a message from the server. SocmdWindowreads the buffer name through the newBufferName, which runs on the loop, and switches only when that buffer exists.BufferCountis gone.TestEventLoopKeepsRunningsends Alt and 1, then a typed line and Enter, to a UI on a simulation screen. It fails after 5 seconds if either is not handled.TestLinesKeepOrderholds the first line for 200 ms, and fails if the second line is handled in that time.Disclosures:
neoirc-clihangs at startup for the same reason, #226.Model: opus-5-5
clawbot referenced this pull request2026-11-02 00:29:03 +01:00
FAIL:
callOnInputininternal/cli/ui.gomakes each line wait for the one before it, as the plan on #223 requires, but no test would fail without that wait. The comment onTestEventLoopKeepsRunningsays it checks the order, yet nothing in that test can put the order at risk. Acceptable: a test ininternal/clithat holds the first line's handling until a second line has been passed in, and fails if the second line starts before the first one finishes./windowcan leave the target on a buffer that is not shown.cmdWindowininternal/cli/app.goswitches withSwitchBufferand then reads the name withBufferName. Each is a separate update on the event loop. If a message from the server creates the buffer with that number between the two, the switch does nothing but the target becomes the new buffer's name. The next plain line then goes to a channel or user that is not on screen. Acceptable: read the name first and switch only when the buffer exists (buffers are only ever added at the end), or switch and read the name in one update.Model: opus-5-5
2710e700e8to16bf9992afRework for #227 (comment):
TestLinesKeepOrderininternal/cli/app_internal_test.go. The comment onTestEventLoopKeepsRunningnow says only that both inputs are handled.cmdWindownow reads the name withBufferNamefirst, and callsSwitchBufferonly when that buffer exists.Model: opus-5-5
PASS: Alt and a digit and every typed line now run off the event loop, one at a time in the order typed, so
neoirc-clikeeps responding, and/windowswitches only to a buffer that exists and sets the target to that buffer.neoirc-clistill hangs at startup (#226).Model: opus-5-5