Add a clipboard write path and keyboard log navigation - #201
Merged
Merged
Conversation
Nothing could be read out of the console. The log view is the only place a message, a command echo, or a failing value ever appears, and none of it could be selected, so a developer who saw an error in a device build had to retype it or screenshot the game window. copy-last and copy-log put the newest line, or the last N, on the system clipboard and answer in the console like every other command. A write is a request a platform can decline - tvOS has no clipboard, and a browser may refuse without a user gesture - so the write is read back and compared, and a copy that did not happen is reported rather than silently doing nothing. time rejoined its arguments with single spaces, so a quoted argument arrived as several: 'time set-variable greet "two words"' was rejected before the timed command ran. It now dispatches the parsed arguments it already has instead of a flattened string, and the log window read is shared with trace rather than duplicated.
The log could not be moved at all from the keyboard. The command line owns panel focus for as long as the console is open, so Page Up, Page Down, Home, and End all arrived at the command field instead of the log view, leaving the wheel and a 10px dragger as the only ways to reach an error in a full buffer. Page Up and Page Down now page the log by one viewport, and the command modifier with Home or End reaches its oldest and newest lines. Home and End without it stay with the command line, where they move the caret as a text field should. Both the command and control flags are accepted, for the reason the paste path already checks both. A page places the scroller and pins nothing, and that is load-bearing. The follower detaches on a value below the pin it already holds, so a page reads as the developer's own scroll; pinning to the new position instead would make the position equal the pin, the tail would never detach, and the next pass would scroll the view straight back to the end. LogTailFollowerTests now pins that rule directly, since the terminal depends on it. This host's editor lays out no UI Toolkit panel, so a log view has no geometry here and its scroller has nothing to scroll. TerminalUILogScrollTests skips with that reason rather than reporting a red the host produced; the decision it drives is covered engine-independently in LogScrollKeysTests.
A panel left half-built by an earlier test can report a scroller range for a frame or two and lose it again. Guarding on having seen a range once let a test start on that flicker and assert into geometry that had gone, which read as a red the change did not cause. The guard now requires the layout to survive several consecutive frames, which is the difference between an environment that can answer the suite and one that cannot.
The review's two blocking findings were measured rather than believed, and both were refuted: restoring master's 'time' and running it still substitutes a $name and still pushes the same single history entry, because substitution happens when the outer line is parsed and the history funnel is the same one either way. Those two behaviours are now pinned by tests, so the claim cannot be re-made on the next review. The real bug it found was the one neither of us had modelled. A page sets the scroller and relies on the follower seeing a value below its pin - but running a command calls Attach, which clears that pin, and the pin lands a frame later. A page taken in that window has nothing to be below, so the next pass reads it as output and snaps the view straight back to the end. LogTailFollower gains an explicit Detach(position) for the one move Observe cannot infer, and TerminalUI calls it. A heuristic inside Observe was tried first and broke three existing contracts - a view nobody scrolled must still follow - which is why the caller's knowledge is the signal and not a guess. Also: the clipboard refusal message is no longer given for a log that holds no text, which is a different failure from a platform declining a copy; Read() is no longer dead code, since the paste path now goes through it; and the modifier comment describes what the flags actually are rather than what a platform usually sends. Three of the seven PlayMode tests passed with the feature deleted - a negative test that never parks the view first cannot fail. Each now proves the log moved before asserting that a key did not move it, and two copy tests that asserted a clipboard value they never set now assert the clipboard is unchanged.
…reeze The second review executed the follower and found a freeze this branch would have shipped. A page writes the scroller and then records a detach unconditionally, so a key that moved nothing - Page Down or Ctrl+End on a view already at its end - detached a view sitting exactly on the end. The next frame's layout grows the end past it, and from there nothing moved the view again: the log stopped scrolling, silently, with recovery only by running a command. The trigger was the ordinary one, catching up on a log that is still growing, where the last press of the sweep is the press that did it. Detach now takes the extent and declines when the scroll landed on the end, which puts the rule where the tolerance lives instead of restating it at the call site. Pinned by a test that fails against the unconditional version, and by a PlayMode test for the gesture itself. Also from that round: whether the copied lines held text is answered per line rather than on the joined string, because two empty lines still join to one newline and were reported as a successful copy of a single character; the Ctrl+End test no longer pages twice and now logs after the key so it can distinguish "returned to the tail" from "never left it"; the history assertion is one entry rather than at most one; the three new runtime metas carry the MonoImporter their neighbours use instead of the one Unity would rewrite. And the features are documented, which the previous commit shipped without: README gains "Copying out of the console" and "Reading the log with the keyboard" next to the paste section, and the changelog now says what a page reaching the end does and that time substitutes a variable once rather than twice.
This was referenced Sep 29, 2026
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit a272b1a. Configure here.
The comment stated it twice: once as a behaviour kept, once as a behaviour changed. A reader met two statements of one rule and could not tell which was current. One block now says both halves - $name is still substituted, and a stored value that is itself $name is no longer expanded a second time.
Owner
Author
|
DISCLOSURE: LLM-GENERATED TEXT Correct, and it shipped broken. Fixed in
It survived two review rounds because the test helper called Terminal.Log(TerminalLogType.Input, line);
_shell.RunCommand(line);That alone turned 7 of the suite green-red. The fix is Swept the class, not the symptom. Two consumers confirmed already correct, and left alone: Recorded as a skill so it does not recur: |
Cursor Bugbot found it on the PR, and it was right: EnterCommand writes the typed line into the log as TerminalLogType.Input before the handler runs, so copy-last copied the word "copy-last" instead of the message the developer was reaching for. The second review had already found this and it was dropped. The test suite could not see it because the file's Run helper called shell.RunCommand directly, which never writes the echo, so the whole suite exercised a path the product never takes. Run now mirrors the console, and that alone turned 7 of 19 tests red before any production change. OutputLength walks back over the trailing command echoes and bounds the copy window. Only the newest end is walked: an echo further back is a command the developer ran, and a transcript wants those as much as the output. Swept rather than patched. trace read a fixed logCount - 2, which has the same defect twice - it printed a command line whenever the command above it said nothing, and it skipped the newest message when dispatched programmatically, where there is no echo at all. It uses the same rule now, and both changes are in the changelog. CommandPaletteUI.CollectOutput and TerminalUI.RefreshLogs were checked and are already correct: the palette filters a whole window that cannot contain another submit's echo, and the view renders echoes because they belong on screen. The asymmetry is deliberate and is now written down. New skill .llm/skills/log-echo-contract/ carries the contract, the two valid filter shapes, and the test-helper trap that hid all of it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
DISCLOSURE: LLM-GENERATED TEXT
Why:
Nothing could be read out of the console or moved through with the keyboard. The log view is the only place an error ever appears, and it had no clipboard write path and no key that reached it (#194, #195).
What:
copy-lastandcopy-log [n]; a write the platform declines is reported, not silent.timedropping argument quoting, so a quoted argument survives it (The time command drops argument quoting, so it cannot time any command with a quoted argument #196).How we know:
EditMode 639/639. PlayMode matches master: the same 5 failures, measured on unmodified master.
The log-scroll suite skips on this host (no rendered Game view) and names the reason.
Note
Medium Risk
Touches built-in command behavior, clipboard I/O, and terminal input routing; regressions would show as wrong copied text, broken trace, or scroll/follow fighting new log output.
Overview
Adds
copy-lastandcopy-log [n]so console output can land on the system clipboard (messages only, no stack traces), withTerminalClipboardcentralizing read/write and reporting when a platform refuses the write. Log reads for copy andtracenow useOutputLengthto skip trailing command echoes instead of fixed “second newest” indices, which fixes copying your own command name and wrongtracetargets (including programmatic dispatch).While the command line keeps focus, Page Up/Down and Ctrl/Cmd+Home/End scroll the log via
LogScrollKeysandTryScrollLog, detaching tail-follow so new output does not yank a parked view;LogTailFollower.Detachtreats a page that lands on the end as following again.timeruns the timed command with the already-parsedCommandArgtail so quoted arguments survive. Docs (README, CHANGELOG) and alog-echo-contractagent skill document the echo contract and test helpers that echo before dispatch.Reviewed by Cursor Bugbot for commit fafbc15. Bugbot is set up for automated code reviews on this repo. Configure here.