From e92e9778261fe58dc90f11abd5b40371e0c5e79a Mon Sep 17 00:00:00 2001 From: opencode Date: Tue, 29 Sep 2026 20:52:14 +0000 Subject: [PATCH 1/3] Add a search over the log view find keeps only the log lines holding the text and reports how many matched; find on its own and F3/Shift+F3 step through the matches, and clear-filter shows every line again. A log buffer is the newest 256 lines and nothing else, so finding one error meant reading past the other 255. A search never matches a command echo. Both surfaces echo the typed line into the log as Input before the handler runs, so the search's own echo held the query and every search found itself - a query appearing nowhere reported a match. Excluded by type, not by the trailing-echo walk the copy commands use: that would make the count depend on whether anything had been logged since. Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 1 + README.md | 23 +- .../Backend/BuiltinCommands.cs | 51 ++ Runtime/CommandTerminal/UI/LogFilter.cs | 240 ++++++ Runtime/CommandTerminal/UI/LogFilter.cs.meta | 11 + Runtime/CommandTerminal/UI/LogFindKeys.cs | 32 + .../CommandTerminal/UI/LogFindKeys.cs.meta | 11 + Runtime/CommandTerminal/UI/TerminalUI.cs | 367 ++++++++- Tests/Editor/LogFilterTests.cs | 433 +++++++++++ Tests/Editor/LogFilterTests.cs.meta | 11 + Tests/Editor/LogFindKeysTests.cs | 45 ++ Tests/Editor/LogFindKeysTests.cs.meta | 11 + Tests/Runtime/TerminalUILogFilterTests.cs | 717 ++++++++++++++++++ .../Runtime/TerminalUILogFilterTests.cs.meta | 11 + 14 files changed, 1960 insertions(+), 4 deletions(-) create mode 100644 Runtime/CommandTerminal/UI/LogFilter.cs create mode 100644 Runtime/CommandTerminal/UI/LogFilter.cs.meta create mode 100644 Runtime/CommandTerminal/UI/LogFindKeys.cs create mode 100644 Runtime/CommandTerminal/UI/LogFindKeys.cs.meta create mode 100644 Tests/Editor/LogFilterTests.cs create mode 100644 Tests/Editor/LogFilterTests.cs.meta create mode 100644 Tests/Editor/LogFindKeysTests.cs create mode 100644 Tests/Editor/LogFindKeysTests.cs.meta create mode 100644 Tests/Runtime/TerminalUILogFilterTests.cs create mode 100644 Tests/Runtime/TerminalUILogFilterTests.cs.meta diff --git a/CHANGELOG.md b/CHANGELOG.md index 2dcc8ea..fc204a3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Added +- `find ` searches the log view: it keeps only the lines holding the text, jumps to the first match, and reports how many lines matched out of how many the search ranged over. `find` with no argument steps to the next match and wraps at the end, F3 and Shift+F3 do the same from the keyboard, and `clear-filter` shows every line again. Until now a log too long to read had no way in but dragging a 10px scrollbar: with the default 256-entry buffer, finding one error meant paging through every line that was not it, and the log view is the only place a message ever appears. The count is the answer to "did my search hit", which a view showing some lines cannot give. A search that matched nothing reports that and stays set, so a typo does not silently put the whole log back on screen. A query is the arguments joined back into one string, so `find "two words"` searches for two words, and an empty or whitespace-only query is refused so it cannot drop the search already in place. The search runs over the text as the log shows it, so what is on screen is what can be found, and it ignores case. It does not find commands you ran: both console surfaces echo the typed line into the log as an `Input` entry before the handler runs, and a search that matched its own echo reported a hit for every query, including one that appears nowhere. `clear-console` does not drop the search - the view goes empty until the next `clear-filter` - and the query is recorded only as the `find` line in the log, so it is not recoverable once that line rotates out of the buffer. - `copy-last` and `copy-log [n]` put the console log on the system clipboard: the newest line, or the last N, joined with newlines. Until now nothing could be read out of the console at all - 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 `Command 'give' threw ArgumentException` in a device build had to retype it or screenshot the game window. Both surfaces echo the line you typed into the log before the command runs, so a copy skips the echoes at the newest end: `copy-last` returns the line you were reading, not the word `copy-last`. An echo further back stays, because a transcript of a session wants the commands in it as much as the output. A copied line is the line the log shows, so the stack trace is not included (`trace` is how you read one) and copying 256 lines does not carry 256 traces. A clipboard write is a request a platform can decline - tvOS has none, and a browser may refuse without a user gesture - so the write is read back and compared, and a copy that did not happen says so instead of silently doing nothing. An empty log, a log holding only commands, a count that is not a number, and a count below one are each reported rather than ignored. - The log scrolls with the keyboard while the command line holds focus. Page Up and Page Down page it by one viewport, and Ctrl+Home (Cmd+Home on macOS) and Ctrl+End reach its oldest and newest lines, so a developer looking for one error in a full 256-entry buffer no longer has to drag a 10px scrollbar. Home and End without the modifier stay with the command line, where they move the caret as a text field should, and every other key is untouched. Paging back detaches the tail exactly as scrolling back does, so new output no longer pulls the view to the end while it is being read; Ctrl+End is the way back to following. A page that reaches the end leaves the log following rather than parked a line short of an end that is still growing. - `CommandLog.CopyTo(LogItem[] destination)` copies the log's visible window, oldest first, into a caller-owned array and returns how many entries it wrote. Reading `CommandLog.Logs` counts and then indexes as two separate reads, so a log written from another thread between them can be missed or, if the buffer was cleared or shrunk, throw. One `CopyTo` call is one consistent view. A destination shorter than the window truncates to its oldest entries, so size it for the largest buffer the session configures. diff --git a/README.md b/README.md index f204bf4..121ba48 100644 --- a/README.md +++ b/README.md @@ -479,10 +479,31 @@ The command line holds panel focus for as long as the console is open, so the te - **Page Up / Page Down** page the log by one viewport. - **Ctrl+Home** and **Ctrl+End** (Cmd on macOS) jump to its oldest and newest lines. -Home and End without the modifier stay with the command line, where they move the caret as a text field should. Every other key is untouched - typing, history recall, completion, and closing work exactly as they did. +Home and End without the modifier stay with the command line, where they move the caret as a text field should. Every other key is untouched - typing, history recall, completion, and closing work exactly as they did - except F3 and Shift+F3, which step a log search while one is set (see [Searching the log](#searching-the-log)). Paging back detaches the tail exactly as scrolling back does, so new output does not pull the view to the end while you are reading. Ctrl+End is the way back to following. A log that has not been laid out - a headless editor, or a view with no rendered Game view - has nothing to page through and stays where it is. +## Searching the log + +The log holds the newest 256 lines by default and nothing else, so reaching one of them means reading past the rest. Two commands narrow the view and one steps through it: + +- `find NullRef` shows only the lines holding the text, and jumps to the first one. +- `find` on its own steps to the next match, wrapping at the end; **F3** and **Shift+F3** do the same from the keyboard. +- `clear-filter` shows every line again. + +Each reports what it did - `Showing 20 of 240 log lines.`, or `Match 3 of 80.` - so a search that hit nothing says so instead of leaving you looking at an empty log, and the search stays set so a typo does not silently put everything back. A query is the arguments joined back into one string, so `find "two words"` searches for two words. An empty query is refused and leaves the search you had alone. + +The search is over the text as the log shows it, so what you can see is what you can find, and it ignores case: `nullref` reaches `NullReferenceException`. + +Four limits worth knowing before you rely on it: + +- It does not find the commands you ran. Both surfaces echo the line you typed into the log, and a search that matched its own echo would report a hit for every query, including one that appears nowhere. Use `copy-log` for a transcript with your commands in it. +- The query is only recorded as the `find` line in the log. Once that line rotates out of the buffer - or you run `clear-console` - nothing on screen says the view is filtered, and the query is gone. `clear-filter` is how you put the log back. +- `clear-console` does not drop the search. The view goes empty until the next `clear-filter` or a new `find`. +- F3 and Shift+F3 need the command line to hold focus, as the paging keys do, and the jump to a match needs a log view that has been laid out. A view with no rendered Game view filters but does not scroll. + +Binding a hotkey to `f3` fires that binding as well while a search is set; the terminal reads the key independently of your bindings. + ## Typing wins over a character binding A binding that presses a character key is left to the field being typed into. While the command line or the palette search bar has focus, pressing the key types the character and does not run the binding - so with the defaults `` ` `` (toggle) and `` #` `` (full), those characters are typeable and the console key no longer closes an open console. diff --git a/Runtime/CommandTerminal/Backend/BuiltinCommands.cs b/Runtime/CommandTerminal/Backend/BuiltinCommands.cs index 98b0196..6217a74 100644 --- a/Runtime/CommandTerminal/Backend/BuiltinCommands.cs +++ b/Runtime/CommandTerminal/Backend/BuiltinCommands.cs @@ -374,6 +374,57 @@ public static void CommandCopyLog(CommandArg[] args) CopyLogLines(int.MaxValue); } + /* + `find` is the answer to a log too long to read: it hides every line + that does not hold the text, and takes the developer to the first + one. The count it reports is the answer to "did my search hit", + which a view showing some lines cannot give. + + The query is the arguments as one string, so a quoted argument + reaches the search whole - the same rule `log` follows, and the + reason `find "two words"` searches for two words. + */ + [RegisterCommand( + isDefault: true, + Name = "find", + Help = "Show only the log lines that contain the text, then the next match on a repeat" + )] + public static void CommandFind(CommandArg[] args) + { + TerminalUI terminal = TerminalUI.Instance; + if (terminal == null) + { + Terminal.Log(TerminalLogType.Warning, "No Terminal UI found."); + return; + } + + if (args.Length == 0) + { + terminal.StepLogFilter(forward: true); + return; + } + + terminal.SetLogFilter(JoinArguments(args)); + } + + [RegisterCommand( + isDefault: true, + Name = "clear-filter", + Help = "Show every log line again, dropping a find", + MaxArgCount = 0 + )] + public static void CommandClearFilter(CommandArg[] args) + { + TerminalUI terminal = TerminalUI.Instance; + if (terminal == null) + { + Terminal.Log(TerminalLogType.Warning, "No Terminal UI found."); + return; + } + + terminal.ClearLogFilter(); + } + [RegisterCommand( isDefault: true, Name = "help", diff --git a/Runtime/CommandTerminal/UI/LogFilter.cs b/Runtime/CommandTerminal/UI/LogFilter.cs new file mode 100644 index 0000000..2993a59 --- /dev/null +++ b/Runtime/CommandTerminal/UI/LogFilter.cs @@ -0,0 +1,240 @@ +namespace WallstopStudios.DxCommandTerminal.UI +{ + using System; + using Backend; + + /* + A search over the log view: which lines stay on screen, how many of + them there are, and which one the developer is reading. + + The log holds the newest N lines and nothing else, so the only way to + reach one of them is to read past the rest. A filter hides what does + not match, which is the thing a developer paging a full buffer by hand + is already doing, one viewport at a time. + + Matching runs against the line as the log shows it. The text is + escaped on the way into the buffer, so `LogItem.message` is what the + developer is looking at, and a search that cannot find what is on + screen is not a search. Ordinal and case-insensitive: `nullref` should + reach `NullReferenceException`, and no culture rule decides whether two + characters are the same letter on this machine. + + The position is the match's place in the kept list, not an index into + the buffer. The ring rotates under the developer - every new line + moves it - so a buffer index would name a different line after the next + log entry, and a search that jumped somewhere else on its own would be + worse than having no position at all. + */ + internal sealed class LogFilter + { + public string Query { get; private set; } + + public bool IsActive => !string.IsNullOrEmpty(Query); + + /* Kept lines, and the lines the window held, from the same pass. */ + public int MatchCount { get; private set; } + + public int TotalCount { get; private set; } + + /* One-based, so the number a developer reads is the number reported. */ + public int? CurrentMatch { get; private set; } + + /* + Whether a line survives the search. The message is never null - + `LogItem` is the only way to make one and it coalesces - and the + query is never empty, because a filter that keeps everything while + reporting that it filtered is not a filter. + + Command echoes never match, and that is the whole reason this is + not `message.Contains(query)` alone. Both console surfaces write + the typed line into the log as `Input` BEFORE the handler runs, so + the search's own echo - `find the hit` - is in the window this + reads and holds the query in its own text. Every search then found + itself: the count was inflated by at least one, and a query that + appears nowhere still reported a match, which is the one answer a + search must never give. See the log-echo-contract skill. + + The exclusion is by type rather than by position. Excluding only + the trailing echo - the shape the copy commands use - would make + the count depend on whether anything was logged since: the same + search would report a different number on the frame the developer + ran their next command, and a line would appear in the view + without the developer having done anything. By type it is stable + for as long as the search is set. + + The limit this buys is stated rather than hidden: a search cannot + find a command the developer ran, only what the program said about + it. The developer knows what they typed, and the output is what + they are looking for. + */ + public static bool Matches(LogItem item, string query) + { + return IsSearchable(item) + && !string.IsNullOrEmpty(query) + && item.message.Contains(query, StringComparison.OrdinalIgnoreCase); + } + + /* + Whether a line the search already excluded would have matched, had + it been a candidate. The count the developer reads counts the lines + the search ranged over, and an echo is not one of them; this is how + the denominator is derived without a second string comparison per + line, and it is not the rule - a caller that wants to know if a + line matches asks `Matches`, which applies the exclusion itself. + */ + private static bool IsSearchable(LogItem item) + { + return item.type != TerminalLogType.Input; + } + + /* + Replaces the query and forgets the position: the new query has a + different set of matches, and the old position names one of them + that may not be there. + + A query of nothing but whitespace is refused rather than trimmed to + nothing. Every line contains a space, so honoring it would report + a count of the whole log and hide nothing, and clearing instead + would drop a search the developer can see they set. The caller + reports the refusal and leaves the previous search alone. + */ + public bool SetQuery(string query) + { + if (string.IsNullOrWhiteSpace(query)) + { + return false; + } + + Query = query.Trim(); + CurrentMatch = null; + MatchCount = 0; + TotalCount = 0; + return true; + } + + public void Clear() + { + Query = null; + CurrentMatch = null; + MatchCount = 0; + TotalCount = 0; + } + + /* + Copies the matching lines into the caller's array, oldest first, + and returns how many it wrote. The destination is the view's own + reused array, which grows with the window and never shrinks, so a + search costs one pass over the window. The window itself is left + alone: the log still holds every line when the search is cleared, + and the next pass reads it again anyway. + + The position is clamped here rather than in the step, because the + ring rotates between two steps. A match the developer was reading + can be gone, and a position past the end of a shorter list would + name a child that does not exist. A query set since the last pass + has no position yet, and lands on the first match. + */ + public int Apply(LogItem[] window, int count, LogItem[] destination) + { + if (window == null || destination == null) + { + return 0; + } + + /* + Bounded by the window, not trusted from the caller. The view + sizes the destination to the window so the two never disagree, + but a caller that got it wrong should get a truncated search + rather than an index past the end of its own array. + */ + int searchableEnd = Math.Min(count, window.Length); + string query = Query; + + /* + Two counters, one pass. `searchable` is the denominator the + developer reads, and it is every line the search could have + matched: an echo is excluded from the matches, so counting it + here would report "20 of 61" beside a rule that says 61 lines + were never candidates. + + The loop runs to the end of the window even once the + destination is full, because a truncated `kept` with a complete + `searchable` is the honest pair: the view drew what fitted, and + the total still says how many lines there were. Stopping early + would report the two from different sets. + */ + int kept = 0; + int searchable = 0; + for (int i = 0; i < searchableEnd; ++i) + { + LogItem item = window[i]; + if (!IsSearchable(item)) + { + continue; + } + + ++searchable; + if (kept < destination.Length && Matches(item, query)) + { + destination[kept] = item; + ++kept; + } + } + + MatchCount = kept; + TotalCount = searchable; + if (kept < 1) + { + CurrentMatch = null; + } + else + { + CurrentMatch = Math.Min(CurrentMatch.GetValueOrDefault(1), kept); + } + + return kept; + } + + /* False means the window holds no match to step to. */ + public bool StepForward() + { + return Step(1); + } + + public bool StepBackward() + { + return Step(-1); + } + + /* + The next match, wrapping at both ends. The matches are a set, not + a document, and a search that stopped at the last one would need the + developer to already know how many there were to come back to the + first. + + A step taken before a position exists lands on the first match in + the requested direction, so the first step of a new search is not a + jump past the hit the developer is reading. + */ + private bool Step(int delta) + { + if (MatchCount <= 0) + { + return false; + } + + int next = CurrentMatch.GetValueOrDefault() + delta; + if (next < 1) + { + next = MatchCount; + } + else if (MatchCount < next) + { + next = 1; + } + + CurrentMatch = next; + return true; + } + } +} diff --git a/Runtime/CommandTerminal/UI/LogFilter.cs.meta b/Runtime/CommandTerminal/UI/LogFilter.cs.meta new file mode 100644 index 0000000..4622edb --- /dev/null +++ b/Runtime/CommandTerminal/UI/LogFilter.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 87716b16d2b94e27b8349b4e35bad73a +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Runtime/CommandTerminal/UI/LogFindKeys.cs b/Runtime/CommandTerminal/UI/LogFindKeys.cs new file mode 100644 index 0000000..8dd2027 --- /dev/null +++ b/Runtime/CommandTerminal/UI/LogFindKeys.cs @@ -0,0 +1,32 @@ +namespace WallstopStudios.DxCommandTerminal.UI +{ + using UnityEngine; + + /* + The keys that step a search, which is the one part of finding that a + command cannot do for you. + + F3 is the key every tool that searches text has bound to "the next + one", and Shift+F3 to the one before it. The command line holds panel + focus for as long as the console is open, so the key arrives at the + command field and the terminal routes it, exactly as it routes Page Up + to the log. + + Nothing else is claimed. A key with no search set is left alone, which + keeps it the developer's: F3 does nothing in a text field anywhere + else either, and a message about a search nobody set would be noise on + a key nobody pressed to search. + */ + internal static class LogFindKeys + { + public static bool IsStepForward(KeyCode keyCode, bool shiftKey) + { + return KeyCode.F3 == keyCode && !shiftKey; + } + + public static bool IsStepBackward(KeyCode keyCode, bool shiftKey) + { + return KeyCode.F3 == keyCode && shiftKey; + } + } +} diff --git a/Runtime/CommandTerminal/UI/LogFindKeys.cs.meta b/Runtime/CommandTerminal/UI/LogFindKeys.cs.meta new file mode 100644 index 0000000..a721a17 --- /dev/null +++ b/Runtime/CommandTerminal/UI/LogFindKeys.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 84d6eb73c0784def85ecc961e6f395c2 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Runtime/CommandTerminal/UI/TerminalUI.cs b/Runtime/CommandTerminal/UI/TerminalUI.cs index 412687b..12852ff 100644 --- a/Runtime/CommandTerminal/UI/TerminalUI.cs +++ b/Runtime/CommandTerminal/UI/TerminalUI.cs @@ -27,6 +27,20 @@ not reallocate on every capacity change around it. */ private const int MinimumLogWindowSize = 16; + /* + Log refreshes a queued search jump waits for the view to lay out + the line it is aiming at. The layout runs after the pass that + creates the line, so the first attempt has nothing to scroll to and + the second one does; a view that never lays out at all - a headless + editor, a window collapsed to nothing - is answered by dropping the + request rather than retrying it every pass forever. + + Counted in refreshes rather than frames, because those are the only + ones that can act on it: a closed terminal runs no log refresh at + all, and a frame budget would never be spent while it stayed shut. + */ + private const int FindScrollFrameBudget = 4; + public static TerminalUI Instance { get; private set; } // Cache log callback to reduce allocations @@ -236,12 +250,42 @@ window height is still settling toward its target. private LogTailFollower _logTail; private long? _lastSeenBufferVersion; + /* + The search the developer set over the log view, and where in it + they are. Per terminal, not per session: it is a view of one + screen, and a second terminal sharing the log buffer is its own + view with its own reason to be reading a different part of it. + */ + private readonly LogFilter _logFilter = new(); + + /* + A search asked for one of its matches to be brought into view, and + the log refreshes left to wait for that line to have a layout. + + The match is held as its ordinal in the kept list rather than as the + child index it currently is, and the index is derived at apply time. + The ring rotates under a search that is left standing - every new + line moves it - so a cached index names a different match a frame + later, and a search that jumped to the wrong line is worse than one + that jumped to none. + */ + private int? _pendingFindScroll; + + private int _findScrollPasses; + /* The log window the frame reads, sized to the buffer's capacity and reused. RefreshLogs copies into it once so the count it lays out and the lines it renders are the same read. */ private LogItem[] _logWindow = Array.Empty(); + + /* + The lines of that window a search keeps, in the same reused form. + Separate from the window so clearing a search redraws every line + without the buffer being read a second time. + */ + private LogItem[] _filteredLogWindow = Array.Empty(); private bool _paletteHeldSurface; private bool _needsInitialRefresh; private string _lastKnownCommandText; @@ -1807,6 +1851,14 @@ internal void TeardownUI() _textInput = null; _stateButtonContainer = null; _lastCodeSyncedValue = null; + + /* + The jump was aimed at a line in the tree that just went away. + A rebuild draws the filtered log from scratch, so keeping the + request would scroll the fresh view to whatever line took that + index. + */ + DropFindScroll(); } /* @@ -1938,6 +1990,132 @@ internal void ApplyHint(int index) _needsFocus = true; } + /* + `find`: the log view keeps only the lines that hold the text, and + the first of them is brought into view. + + The counts are read here rather than from the view's next pass, + because the command answers now and a count read a frame later + would describe a log that has moved on - and because a frame is + long enough for the log to gain a line the answer does not know + about. + + The text is not repeated in the answer. The command echo directly + above it in the log already shows exactly what was searched for, + and a line that repeated it would match the search that produced + it: the next `find` of the same text would then report one more + match than the developer can count on screen, every time they ran + it. + */ + internal void SetLogFilter(string query) + { + if (Terminal.Buffer == null) + { + /* + No session means no log to search, and the facade getter + legitimately reads null before the session exists and after + a play-session reset. Reported rather than treated as "no + matches", which would be an answer about a log that is not + there. + */ + Terminal.Log(TerminalLogType.Warning, "There is no log to search yet."); + return; + } + + if (!_logFilter.SetQuery(query)) + { + /* + The search already in place is left alone, and so is any + jump it had queued: a refused query is not a request to + change what is on screen. + */ + Terminal.Log( + TerminalLogType.Warning, + "Nothing to search for. clear-filter shows every log line again." + ); + return; + } + + int matches = ReadRenderedLogWindow(Terminal.Buffer, out _); + if (matches < 1) + { + /* + The search stays set. Dropping it here would mean a typo + silently put the whole log back on screen, and the developer + would have to type the search again to see that it had hit + nothing. + */ + DropFindScroll(); + Terminal.Log( + TerminalLogType.Warning, + "No log line matches the search. clear-filter shows every line again." + ); + return; + } + + QueueFindScroll(); + + /* + "of" counts the lines the search could have matched, not the + ones it did - so the total and the match count are not + interchangeable words here, and "matching log lines" on the + total would say the opposite of what the number means. + */ + Terminal.Log($"Showing {matches} of {_logFilter.TotalCount} log lines."); + } + + /* + `find` with no argument, and F3: the next match, or the one before + it, wrapping. + + The window is re-read first because the ring rotates under a search + that is left standing: the matches the developer is stepping + through are the ones the log holds now, not the ones it held when + they typed the search. + */ + internal void StepLogFilter(bool forward) + { + if (!_logFilter.IsActive) + { + Terminal.Log(TerminalLogType.Warning, "No search is set. find sets one."); + return; + } + + if (Terminal.Buffer == null) + { + /* + The counts a step reports are the last ones read, and with + no log there is nothing to step through - stepping anyway + would answer "Match 7 of 20" for a log that holds nothing. + */ + Terminal.Log(TerminalLogType.Warning, "There is no log to search yet."); + return; + } + + ReadRenderedLogWindow(Terminal.Buffer, out _); + if (!(forward ? _logFilter.StepForward() : _logFilter.StepBackward())) + { + Terminal.Log(TerminalLogType.Warning, "No log line matches the search."); + return; + } + + QueueFindScroll(); + Terminal.Log($"Match {_logFilter.CurrentMatch} of {_logFilter.MatchCount}."); + } + + internal void ClearLogFilter() + { + if (!_logFilter.IsActive) + { + Terminal.Log("No search is set."); + return; + } + + _logFilter.Clear(); + DropFindScroll(); + Terminal.Log("Search cleared. The log shows every line again."); + } + /* A recalled line is a new value, so the caret belongs at its end. FocusInput only writes a caret on a fresh focus, and the field was @@ -2450,6 +2628,12 @@ newlines and all. } if (context.TryScrollLog(evt)) + { + KeyEvents.Consume(context._commandInput, evt); + return; + } + + if (context.TryStepLogFilter(evt)) { KeyEvents.Consume(context._commandInput, evt); } @@ -2714,9 +2898,10 @@ guarded local instead of re-deriving nullability per access. read under it separately could describe two different moments. The window is sized to the buffer's capacity, so it holds every entry a read can return, and it is written once and - reused. + reused. A search narrows that read to the lines it keeps, and + the count the view lays out is the count it draws. */ - int logCount = ReadLogWindow(buffer); + int logCount = ReadRenderedLogWindow(buffer, out LogItem[] rendered); if (content.childCount != logCount) { dirty = true; @@ -2744,7 +2929,7 @@ read under it separately could describe two different moments. for (int i = 0; i < logCount && i < childCount; ++i) { VisualElement item = content[i]; - LogItem logItem = _logWindow[i]; + LogItem logItem = rendered[i]; switch (item) { case TextField logText: @@ -2775,6 +2960,7 @@ read under it separately could describe two different moments. } _needsScrollToEnd |= ObserveLogTail(newLogs); + ApplyPendingFindScroll(content, logCount); return; static void SetupLogText(VisualElement logText, LogItem log) @@ -2822,6 +3008,158 @@ private int ReadLogWindow(CommandLog buffer) return buffer.CopyTo(_logWindow); } + /* + The lines the log view draws this pass: the buffer's window, or the + ones of it a search keeps. Hands back the array holding them and + how many there are, so the caller reads the count and the lines + from one place - a count from a filtered read and lines from the + unfiltered window would draw lines the count never promised. + + No search means no second pass and no second array: the window is + already the answer, which is why the filtered array only ever grows + to the window's size. + */ + private int ReadRenderedLogWindow(CommandLog buffer, out LogItem[] rendered) + { + if (buffer == null) + { + rendered = Array.Empty(); + return 0; + } + + int windowCount = ReadLogWindow(buffer); + if (!_logFilter.IsActive) + { + rendered = _logWindow; + return windowCount; + } + + if (_filteredLogWindow.Length < _logWindow.Length) + { + _filteredLogWindow = new LogItem[_logWindow.Length]; + } + + rendered = _filteredLogWindow; + return _logFilter.Apply(_logWindow, windowCount, _filteredLogWindow); + } + + /* + Records which match the next refreshes have to bring into view. + + The ordinal, not a child index: the ring rotates under a search + that is left standing, and the kept list shifts with it, so an index + taken now names a different match a frame later. + + The scroll-to-end a command run asks for is dropped here rather than + left to be re-armed. `EnterCommand` attaches the tail and sets the + flag, and `ObserveLogTail` puts it back on every pass the follower + is still following - so a jump that waited for a layout would be + undone by a scroll to the end on the pass in between, and the + developer would see the view go to the end and snap back. + */ + private void QueueFindScroll() + { + int? match = _logFilter.CurrentMatch; + if (!match.HasValue) + { + return; + } + + _pendingFindScroll = match; + _findScrollPasses = FindScrollFrameBudget; + _needsScrollToEnd = false; + } + + /* + Forgets a queued jump, and the budget that was waiting for it. Every + site that ends a search's intent to scroll calls this, so a request + cannot outlive the decision that made it. + */ + private void DropFindScroll() + { + _pendingFindScroll = null; + _findScrollPasses = 0; + } + + /* + The match a search asked for, once the view can say where it is. + + A child's position is a layout result, and the layout runs after the + pass that created the child: a search made this frame is aiming at + children this frame has only just added, so the first pass has + nothing to scroll to and the next one does. The request is dropped + rather than retried forever, because a view that never lays out at + all - a headless editor with no rendered view, a window collapsed + to nothing - would otherwise carry a request nothing can act on, + and a view that starts laying out much later would scroll to + whatever line then held that index. + + The index is read from the ordinal here, not from the request: the + kept list can have shifted between the request and this pass, so a + cached index would name a different match than the one asked for. + + `content` is the reconciled container and `logCount` how many + children it holds, so the index is in range by construction - the + check is the clamp, and a match the search no longer has ends the + request rather than scrolling somewhere arbitrary. + */ + private void ApplyPendingFindScroll(VisualElement content, int logCount) + { + int? pending = _pendingFindScroll; + if (!pending.HasValue) + { + return; + } + + int index = pending.GetValueOrDefault() - 1; + if (index < 0 || logCount <= index) + { + DropFindScroll(); + return; + } + + VisualElement match = content[index]; + if (0f < match.layout.height) + { + DropFindScroll(); + ScrollToLogLine(match); + return; + } + + if (0 < _findScrollPasses) + { + --_findScrollPasses; + return; + } + + DropFindScroll(); + } + + /* + Puts a line at the top of the log view, which is where a find puts + its hit, and detaches the tail: a developer who searched is reading + the line they found, and the output arriving next is not what they + asked for. Following again is Ctrl+End, or a search that lands on + the end. + + The offset is the line's own position in the content, which is the + same space the scroller's value is in, and it is clamped to the + extent the scroller holds now - a content that grew after the + layout this read would otherwise leave the view past its end. + */ + private void ScrollToLogLine(VisualElement line) + { + Scroller scroller = _logScrollView?.verticalScroller; + if (scroller == null) + { + return; + } + + scroller.value = Math.Max(0f, Math.Min(line.layout.yMin, scroller.highValue)); + _logTail.Detach(scroller.value, scroller.highValue); + _needsScrollToEnd = false; + } + /* New output scrolls into view only while the log view is at its own end. A developer who scrolls up reads at their own pace, and @@ -2918,6 +3256,29 @@ an end that is still growing. return true; } + /* + The same routing for a key that steps a search, and the same two + answers: a key with no search set is not the log's to answer and is + left alone, and a key the log answers is consumed so it does not + also reach the field it arrived through. + */ + private bool TryStepLogFilter(KeyDownEvent evt) + { + bool forward = LogFindKeys.IsStepForward(evt.keyCode, evt.shiftKey); + if (!forward && !LogFindKeys.IsStepBackward(evt.keyCode, evt.shiftKey)) + { + return false; + } + + if (!_logFilter.IsActive) + { + return false; + } + + StepLogFilter(forward); + return true; + } + private void ScrollToEnd() { Scroller scroller = _logScrollView?.verticalScroller; diff --git a/Tests/Editor/LogFilterTests.cs b/Tests/Editor/LogFilterTests.cs new file mode 100644 index 0000000..89ff2e4 --- /dev/null +++ b/Tests/Editor/LogFilterTests.cs @@ -0,0 +1,433 @@ +namespace WallstopStudios.DxCommandTerminal.Tests.Runtime +{ + using Backend; + using NUnit.Framework; + using UI; + + /* + The decision a search makes: which lines of the log view survive, how + many there are, and which one the developer is on. The wiring - the + command, the key, and the children the view draws - is + TerminalUILogFilterTests; this file is the decision. + + Every test applies a window and reads the result back out of the + destination array, the way the view does, so a match count and the + lines it counted cannot disagree. + */ + public sealed class LogFilterTests + { + private static LogItem[] Window(params string[] messages) + { + LogItem[] window = new LogItem[messages.Length]; + for (int i = 0; i < messages.Length; ++i) + { + window[i] = new LogItem(TerminalLogType.Message, messages[i], string.Empty); + } + + return window; + } + + [Test] + public void AQueryKeepsTheLinesThatHoldItInLogOrder() + { + LogFilter filter = new(); + Assert.That(filter.SetQuery("NullReference"), Is.True); + + LogItem[] window = Window( + "starting", + "NullReferenceException: value", + "middle", + "also NullReference here" + ); + LogItem[] destination = new LogItem[window.Length]; + + int kept = filter.Apply(window, window.Length, destination); + + Assert.That(kept, Is.EqualTo(2), "Only the two matching lines are drawn"); + Assert.That(destination[0].message, Is.EqualTo("NullReferenceException: value")); + Assert.That( + destination[1].message, + Is.EqualTo("also NullReference here"), + "The oldest match is drawn first, so the view reads top to bottom like the log" + ); + Assert.That(filter.MatchCount, Is.EqualTo(2)); + Assert.That( + filter.TotalCount, + Is.EqualTo(4), + "The count a developer compares against is the whole log, not the matches" + ); + } + + [Test] + public void MatchingIgnoresCaseAndFollowsNoCulture() + { + /* + A developer types the shortest thing they remember, and the + exception they are looking for is spelled with capitals. A + culture-sensitive comparison would also decide that two + characters are the same letter, which is not a question a log + search should be asking. + */ + LogFilter filter = new(); + Assert.That(filter.SetQuery("nullreference"), Is.True); + + LogItem[] window = Window("NullReferenceException: value"); + int kept = filter.Apply(window, window.Length, new LogItem[window.Length]); + + Assert.That(kept, Is.EqualTo(1), "A lowercase search reaches the capitalized line"); + Assert.That( + LogFilter.Matches( + new LogItem(TerminalLogType.Message, "straße", string.Empty), + "STRASSE" + ), + Is.False, + "No culture rule folds a sharp s into a double s" + ); + } + + [Test] + public void AQueryMatchesTheLineTheLogShows() + { + /* + The text is escaped on the way into the buffer, so the message + is already what the developer is reading. A search that matched + something else would find lines they cannot see, which is the + one thing a search must not do. + */ + LogItem escaped = new(TerminalLogType.Message, @"name is Admin‮exe", string.Empty); + + Assert.That( + LogFilter.Matches(escaped, "‮"), + Is.True, + "The escape the view draws is in the text the search reads" + ); + Assert.That( + LogFilter.Matches(escaped, "Admin"), + Is.True, + "The visible part of the name still matches" + ); + } + + [Test] + public void ACommandEchoIsNotAMatch() + { + /* + The defect this pins. Both console surfaces echo the typed line + into the log as `Input` before the handler runs, so the search's + own echo - `find the hit` - sits in the window holding the + query in its own text. Matching it made every search find + itself: the count was always one too high, and a query that + appears nowhere still reported a match. + */ + LogItem echo = new(TerminalLogType.Input, "find the hit", string.Empty); + + Assert.That( + LogFilter.Matches(echo, "the hit"), + Is.False, + "A search cannot find the command that performed it" + ); + Assert.That( + LogFilter.Matches( + new LogItem(TerminalLogType.Message, "find the hit", string.Empty), + "the hit" + ), + Is.True, + "Output holding the same words is a match; only the echo type is excluded" + ); + } + + [Test] + public void AQueryThatOnlyTheEchoHoldsReportsNoMatch() + { + /* + The user-visible shape of the same defect, and the answer a + search has to be able to give. Without the exclusion this window + matched its own echo and reported a hit for a string the log + holds nowhere else. + */ + LogFilter filter = new(); + Assert.That(filter.SetQuery("the hit"), Is.True); + + LogItem[] window = + { + new LogItem(TerminalLogType.Message, "routine frame", string.Empty), + new LogItem(TerminalLogType.Input, "find the hit", string.Empty), + }; + int kept = filter.Apply(window, window.Length, new LogItem[window.Length]); + + Assert.That(kept, Is.EqualTo(0), "A query no output holds found nothing"); + Assert.That(filter.MatchCount, Is.EqualTo(0)); + } + + [Test] + public void TheEchoExclusionIsByTypeSoTheCountDoesNotMoveUnderTheDeveloper() + { + /* + Excluding only the trailing echo - the shape the copy commands + use - would make the count depend on whether anything was logged + since. The same search would report a different number on the + frame the developer ran their next command, and a line would + appear in the view without them doing anything. Excluded by + type, the count holds however much else the log has taken since. + */ + LogFilter filter = new(); + Assert.That(filter.SetQuery("hit"), Is.True); + + LogItem[] before = + { + new LogItem(TerminalLogType.Message, "hit one", string.Empty), + new LogItem(TerminalLogType.Message, "miss", string.Empty), + }; + int first = filter.Apply(before, before.Length, new LogItem[before.Length]); + + /* + The echo goes at the newest end, where a positional exclusion + would drop it, and output carrying the same text goes behind it. + A by-position rule and a by-type rule disagree here and only + here: positionally the search ranges over two lines, by type + over four. + */ + LogItem[] after = + { + new LogItem(TerminalLogType.Message, "hit one", string.Empty), + new LogItem(TerminalLogType.Message, "miss", string.Empty), + new LogItem(TerminalLogType.Message, "hit two", string.Empty), + new LogItem(TerminalLogType.Input, "find hit", string.Empty), + }; + int second = filter.Apply(after, after.Length, new LogItem[after.Length]); + + Assert.That( + second, + Is.EqualTo(first + 1), + "A new match and a new echo are two different things to the count" + ); + Assert.That( + filter.TotalCount, + Is.EqualTo(3), + "The echoed command is not one of the lines the search ranged over" + ); + } + + [Test] + public void AQueryOfNothingButWhitespaceIsRefused() + { + LogFilter filter = new(); + Assert.That(filter.SetQuery("boom"), Is.True, "A search is set to be lost"); + + Assert.That( + filter.SetQuery(" "), + Is.False, + "Every line contains a space, so this is not a search" + ); + Assert.That(filter.SetQuery(string.Empty), Is.False); + Assert.That(filter.SetQuery(null), Is.False); + Assert.That( + filter.Query, + Is.EqualTo("boom"), + "A refused query leaves the search the developer set alone" + ); + } + + [Test] + public void AQueryIsTrimmed() + { + LogFilter filter = new(); + Assert.That(filter.SetQuery(" boom "), Is.True); + + Assert.That( + filter.Query, + Is.EqualTo("boom"), + "A pasted query with a trailing space still finds the line" + ); + } + + [Test] + public void ANewQueryLandsOnTheFirstMatch() + { + LogFilter filter = new(); + Assert.That(filter.SetQuery("hit"), Is.True); + + LogItem[] window = Window("hit one", "miss", "hit two"); + filter.Apply(window, window.Length, new LogItem[window.Length]); + + Assert.That( + filter.CurrentMatch, + Is.EqualTo(1), + "The first match is where a search starts" + ); + } + + [Test] + public void SteppingWrapsAtBothEnds() + { + LogFilter filter = new(); + Assert.That(filter.SetQuery("hit"), Is.True); + + LogItem[] window = Window("hit one", "hit two", "miss", "hit three"); + int kept = filter.Apply(window, window.Length, new LogItem[window.Length]); + Assert.That(kept, Is.EqualTo(3)); + + Assert.That(filter.StepForward(), Is.True); + Assert.That(filter.CurrentMatch, Is.EqualTo(2)); + Assert.That(filter.StepForward(), Is.True); + Assert.That(filter.CurrentMatch, Is.EqualTo(3), "The last match, not past it"); + Assert.That(filter.StepForward(), Is.True); + Assert.That( + filter.CurrentMatch, + Is.EqualTo(1), + "Forward from the last match wraps to the first" + ); + Assert.That(filter.StepBackward(), Is.True); + Assert.That( + filter.CurrentMatch, + Is.EqualTo(3), + "Backward from the first match wraps to the last" + ); + } + + [Test] + public void SteppingWithNoMatchesReportsIt() + { + LogFilter filter = new(); + Assert.That(filter.SetQuery("hit"), Is.True); + + LogItem[] window = Window("miss", "miss"); + filter.Apply(window, window.Length, new LogItem[window.Length]); + + Assert.That(filter.MatchCount, Is.EqualTo(0)); + Assert.That(filter.StepForward(), Is.False, "There is nothing to step to"); + Assert.That(filter.StepBackward(), Is.False); + } + + [Test] + public void ARotatingBufferDoesNotLeaveThePositionPastTheEnd() + { + /* + The position is a place in the kept list, and the list changes + under a search that is left standing: every new line rotates + the ring, and a match can be pushed out of the window entirely. + A position past the end would name a child the view is not + drawing, and the search would jump nowhere. + */ + LogFilter filter = new(); + Assert.That(filter.SetQuery("hit"), Is.True); + + LogItem[] window = Window("hit one", "hit two", "hit three"); + int kept = window.Length; + filter.Apply(window, kept, new LogItem[kept]); + filter.StepForward(); + filter.StepForward(); + Assert.That(filter.CurrentMatch, Is.EqualTo(3)); + + LogItem[] rotated = Window("miss", "hit two"); + kept = filter.Apply(rotated, rotated.Length, new LogItem[rotated.Length]); + + Assert.That(kept, Is.EqualTo(1)); + Assert.That( + filter.CurrentMatch, + Is.EqualTo(1), + "Clamped to a position the current window has" + ); + } + + [Test] + public void ADestinationSmallerThanTheMatchesTruncates() + { + /* + The view sizes its array to the buffer's capacity, so this is + the shape that never happens in the product. It is pinned + because the alternative is a write past the end of the + caller's array, and a caller that sized its own array wrong + should get fewer lines rather than a corrupted one. + */ + LogFilter filter = new(); + Assert.That(filter.SetQuery("hit"), Is.True); + + LogItem[] window = Window("hit one", "hit two", "hit three"); + int kept = filter.Apply(window, window.Length, new LogItem[1]); + + Assert.That(kept, Is.EqualTo(1)); + Assert.That( + filter.MatchCount, + Is.EqualTo(1), + "The count is what was drawn, not what matched" + ); + Assert.That(filter.TotalCount, Is.EqualTo(3)); + } + + [Test] + public void ClearingDropsTheQueryAndTheCounts() + { + LogFilter filter = new(); + Assert.That(filter.SetQuery("hit"), Is.True); + + LogItem[] window = Window("hit one", "miss"); + filter.Apply(window, window.Length, new LogItem[window.Length]); + + filter.Clear(); + + Assert.That(filter.IsActive, Is.False); + Assert.That(filter.Query, Is.Null); + Assert.That(filter.MatchCount, Is.EqualTo(0)); + Assert.That(filter.TotalCount, Is.EqualTo(0)); + Assert.That( + filter.CurrentMatch, + Is.Null, + "A cleared search has no position to resume from" + ); + Assert.That(filter.StepForward(), Is.False); + } + + [Test] + public void AnUnfilteredFilterKeepsTheWholeWindow() + { + /* + Not a property of a fresh object: what the view relies on is + that an inactive filter is never handed to `Apply` at all, and + that the counts it reports are zero rather than left over, so a + stale "Match 3 of 20" cannot survive into a search that was + never set. + */ + LogFilter filter = new(); + + Assert.That( + filter.IsActive, + Is.False, + "A terminal with no search draws the window itself, never through this" + ); + Assert.That(filter.MatchCount, Is.EqualTo(0)); + Assert.That(filter.TotalCount, Is.EqualTo(0)); + Assert.That(filter.CurrentMatch, Is.Null); + Assert.That( + filter.StepForward(), + Is.False, + "There is nothing to step through, and saying so is what stops a stale count" + ); + } + + [Test] + public void ApplyRunsToTheEndOfTheWindowEvenWhenTheDestinationIsFull() + { + /* + The two numbers a search reports come from one pass over the + window. A loop that stopped when the destination filled - the + obvious way to write this - would count a truncated set of + matches beside a complete set of candidates, and the report + would describe a window the search never finished reading. + */ + LogFilter filter = new(); + Assert.That(filter.SetQuery("hit"), Is.True); + + LogItem[] window = Window("hit one", "hit two", "hit three", "miss", "hit four"); + int kept = filter.Apply(window, window.Length, new LogItem[2]); + + Assert.That(kept, Is.EqualTo(2), "The destination held what it could"); + Assert.That(filter.MatchCount, Is.EqualTo(2), "The count is what was drawn"); + Assert.That( + filter.TotalCount, + Is.EqualTo(window.Length), + "The denominator still covers every line the search could have matched" + ); + } + } +} diff --git a/Tests/Editor/LogFilterTests.cs.meta b/Tests/Editor/LogFilterTests.cs.meta new file mode 100644 index 0000000..11e6539 --- /dev/null +++ b/Tests/Editor/LogFilterTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 1b81e56f66da40ec89c6a0f7bcd6e191 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Tests/Editor/LogFindKeysTests.cs b/Tests/Editor/LogFindKeysTests.cs new file mode 100644 index 0000000..5a4b301 --- /dev/null +++ b/Tests/Editor/LogFindKeysTests.cs @@ -0,0 +1,45 @@ +namespace WallstopStudios.DxCommandTerminal.Tests.Runtime +{ + using NUnit.Framework; + using UI; + using UnityEngine; + + /* + Which key steps a search, and which key it is not. + + The command line owns panel focus for as long as the console is open, + so F3 arrives at the command field rather than at the log view, and + routing it is the terminal's job. This is that decision; pressing it is + TerminalUILogFilterTests. + + One table for the whole rule, because the failures are the same class: + a key the log answers that is not its own, and a key of its own that + it leaves to the field. + */ + public sealed class LogFindKeysTests + { + [TestCase(KeyCode.F3, false, true, false)] + [TestCase(KeyCode.F3, true, false, true)] + [TestCase(KeyCode.F4, false, false, false)] + [TestCase(KeyCode.F4, true, false, false)] + [TestCase(KeyCode.PageDown, false, false, false)] + [TestCase(KeyCode.LeftArrow, false, false, false)] + [TestCase(KeyCode.A, false, false, false)] + [TestCase(KeyCode.Return, false, false, false)] + [TestCase(KeyCode.Escape, false, false, false)] + [TestCase(KeyCode.Tab, false, false, false)] + public void OnlyF3StepsASearch(KeyCode keyCode, bool shiftKey, bool forward, bool backward) + { + Assert.That( + LogFindKeys.IsStepForward(keyCode, shiftKey), + Is.EqualTo(forward), + $"{keyCode} with shift={shiftKey} moves the search forward" + ); + Assert.That( + LogFindKeys.IsStepBackward(keyCode, shiftKey), + Is.EqualTo(backward), + $"{keyCode} with shift={shiftKey} moves the search backward" + ); + } + } +} diff --git a/Tests/Editor/LogFindKeysTests.cs.meta b/Tests/Editor/LogFindKeysTests.cs.meta new file mode 100644 index 0000000..dc60be2 --- /dev/null +++ b/Tests/Editor/LogFindKeysTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: f4b588a9282844eebfd8b6ef845f5b6a +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Tests/Runtime/TerminalUILogFilterTests.cs b/Tests/Runtime/TerminalUILogFilterTests.cs new file mode 100644 index 0000000..9a96720 --- /dev/null +++ b/Tests/Runtime/TerminalUILogFilterTests.cs @@ -0,0 +1,717 @@ +namespace WallstopStudios.DxCommandTerminal.Tests.Runtime +{ + using System; + using System.Collections; + using System.Globalization; + using System.Text; + using Backend; + using Components; + using Input; + using NUnit.Framework; + using Themes; + using UI; + using UnityEngine; + using UnityEngine.TestTools; + using UnityEngine.UIElements; +#if UNITY_EDITOR + using UnityEditor; +#endif + + /* + A search over the log view: `find` hides the lines that do not match, + F3 steps through the ones that do, and `clear-filter` puts the log + back. + + The decision itself is engine-independent and lives in LogFilter and + LogFindKeys; this suite is the wiring - that the command reaches the + view, that the view draws only the matches, and that the key the + command line holds focus for arrives where it is routed. + + The drawn lines are read straight off the view's children, so most of + this suite answers on a host that lays out no UI at all. What it cannot + answer is the scroll to the match: a child's position is a layout + result, so the jump tests skip with the reason rather than asserting + into geometry the host never produced. + */ + public sealed class TerminalUILogFilterTests + { + private const string PackageRoot = "Packages/com.wallstop-studios.dxcommandterminal"; + private const int FrameBudget = 300; + private const float Tolerance = 0.5f; + + /* + Sized so the FILTERED log overflows the view, not just the whole + one. The two jump tests assert on a scroll offset, and a search over + 20 matches leaves 20 lines in a log view far taller than that - so + the scroller would hold no range at all, the layout guard would + blame the environment, and the jump would never be measured even on + a host with a rendered view. 240 lines at one match in three leaves + 80 to scroll through. + + Under the default 256-entry buffer on purpose. Past it the ring + rotates and the oldest lines - matches included - are gone, so the + counts these tests assert would depend on the fill order rather + than on the rule. + */ + private const int FillLines = 240; + private const string MissMarker = "routine frame"; + private const string HitMarker = "the hit"; + + private TerminalUI _terminal; + private GameObject _terminalObject; + private PanelSettings _panelSettings; + + private static T LoadAsset(string relativePath) + where T : ScriptableObject + { +#if UNITY_EDITOR + return UnityEditor.AssetDatabase.LoadAssetAtPath($"{PackageRoot}/{relativePath}"); +#else + return null; +#endif + } + + /* + Three frames, which is what a write this frame needs before a read + can mean anything: the command's own log line is drawn by the next + pass, and the pass after that is the first one that can see the + result of it. + */ + private static IEnumerator Settle() + { + yield return null; + yield return null; + yield return null; + } + + private static IEnumerator FillTheLog() + { + /* + Cleared first: the log buffer is the session's, so a previous + test's lines would be counted as the search's, and these tests + assert exact numbers. + */ + Terminal.Buffer?.Clear(); + + for (int line = 1; line <= FillLines; ++line) + { + bool hit = line % 3 == 0; + Terminal.Log( + hit + ? $"line {line.ToString(CultureInfo.InvariantCulture)} with {HitMarker} in it" + : $"line {line.ToString(CultureInfo.InvariantCulture)} {MissMarker}" + ); + } + + yield return null; + yield return null; + yield return null; + } + + /* + Runs the command the way the console runs it: the echo into the log + first, then the shell. The echo is the newest entry the log holds + while a handler reads it, and a suite that skipped it would answer + questions the product never sees. + */ + private static IEnumerator RunCommand(string line) + { + Terminal.Log(TerminalLogType.Input, line); + Terminal.Shell.RunCommand(line); + yield return null; + yield return null; + } + + [SetUp] + public void SetUp() + { + DefaultTerminalInput.Instance.CommandText = string.Empty; + } + + [TearDown] + public void TearDown() + { + DefaultTerminalInput.Instance.CommandText = string.Empty; + if (_terminalObject != null) + { + UnityEngine.Object.Destroy(_terminalObject); + } + + if (_panelSettings != null) + { + UnityEngine.Object.Destroy(_panelSettings); + } + } + + [UnityTest] + public IEnumerator FindHidesEveryLineThatDoesNotMatch() + { + yield return SpawnOpenTerminal(); + yield return FillTheLog(); + yield return WaitForDrawnCount(FillLines, "The log draws every line before a search"); + + yield return RunCommand("find " + HitMarker); + yield return WaitForDrawnText("the hit", "A search draws the line it found"); + + Assert.That( + DrawnText(), + Does.Not.Contain(MissMarker), + "A line that does not hold the text is not what the developer asked to see" + ); + } + + [UnityTest] + public IEnumerator ClearFilterPutsEveryLineBack() + { + yield return SpawnOpenTerminal(); + yield return FillTheLog(); + yield return WaitForDrawnCount(FillLines, "The log draws every line before a search"); + + yield return RunCommand("find " + HitMarker); + yield return WaitForDrawnText("the hit", "The search landed"); + yield return RunCommand("clear-filter"); + yield return WaitForDrawnText( + MissMarker, + "Clearing a search brings back the lines it hid, including the newest" + ); + } + + [UnityTest] + public IEnumerator AQuotedQueryReachesTheSearchWhole() + { + yield return SpawnOpenTerminal(); + yield return FillTheLog(); + + /* + The search is over the text as the log shows it, so the query + has to reach it the same way any other multi-word argument + would. A query the tokenizer split finds nothing and says so, + which is the failure this pins: the developer asked for a + phrase and got a search for its first word. + */ + yield return RunCommand("find \"" + HitMarker + "\""); + yield return WaitForLastLogLine( + "of 240 log lines", + "A quoted query is one string, so the phrase is what was searched" + ); + } + + [UnityTest] + public IEnumerator FindReportsHowManyLinesItFound() + { + yield return SpawnOpenTerminal(); + yield return FillTheLog(); + + yield return RunCommand("find " + HitMarker); + yield return WaitForLastLogLine( + "of 240 log lines", + "The count is the answer to whether the search hit, which a view showing some lines cannot give" + ); + } + + [UnityTest] + public IEnumerator FindWithNoMatchSaysSoAndHidesEverything() + { + yield return SpawnOpenTerminal(); + yield return FillTheLog(); + yield return WaitForDrawnCount(FillLines, "The log draws every line before a search"); + + yield return RunCommand("find nothing-holds-this-text"); + yield return WaitForLastLogLine( + "No log line matches", + "A search that hit nothing says so" + ); + + yield return Settle(); + Assert.That( + DrawnText(), + Is.Empty, + "A search with no match shows nothing rather than the log it did not find in" + ); + } + + [UnityTest] + public IEnumerator ARefusedQueryLeavesTheSearchAlone() + { + yield return SpawnOpenTerminal(); + yield return FillTheLog(); + yield return RunCommand("find " + HitMarker); + yield return WaitForDrawnText("the hit", "The search landed"); + + yield return RunCommand("find \"\""); + yield return WaitForLastLogLine( + "Nothing to search for", + "An empty query cannot be a search, and cannot silently drop the one that is set" + ); + + yield return Settle(); + Assert.That( + DrawnText(), + Does.Contain("the hit"), + "The search the developer set survives a query that was not one" + ); + } + + [UnityTest] + public IEnumerator FindWithNoArgumentStepsToTheNextMatch() + { + yield return SpawnOpenTerminal(); + yield return FillTheLog(); + yield return RunCommand("find " + HitMarker); + yield return WaitForDrawnText("the hit", "The search landed"); + + yield return RunCommand("find"); + yield return WaitForLastLogLine( + "Match 2 of 80", + "A repeat steps to the next match, and reports which one" + ); + } + + [UnityTest] + public IEnumerator SteppingWithNoSearchSaysSoInsteadOfSilentlyDoingNothing() + { + yield return SpawnOpenTerminal(); + yield return FillTheLog(); + + yield return RunCommand("find"); + yield return WaitForLastLogLine( + "No search is set", + "A command that reached the console and did nothing says so" + ); + } + + [UnityTest] + public IEnumerator F3StepsTheSearchTheCommandLineHoldsFocusFor() + { + yield return SpawnOpenTerminal(); + yield return RequireReachableKeys(); + yield return FillTheLog(); + yield return RunCommand("find " + HitMarker); + yield return WaitForDrawnText("the hit", "The search landed"); + + yield return SendKey(KeyCode.F3); + yield return WaitForLastLogLine( + "Match 2 of 80", + "The key the field holds focus for reaches the search" + ); + } + + [UnityTest] + public IEnumerator F3WithNoSearchIsLeftToTheCommandLine() + { + /* + A negative test that cannot fail is not a negative test, and the + obvious version of this one is exactly that: clear the search, + press F3, and assert the log is still whole - which it is + whether or not F3 was ever routed. So the key is pressed while + a search is set, where a router that answered it must move the + view, and the whole log is only the thing to compare against + once the search is dropped. + + The step is what makes it falsifiable: if F3 were consumed + without stepping, the position would stay at the match the + search set and the view would not move either. So the order is + step, then clear, and the assertion is that clearing is what + restored the log. + */ + yield return SpawnOpenTerminal(); + yield return RequireReachableKeys(); + yield return FillTheLog(); + yield return RunCommand("find " + HitMarker); + yield return WaitForDrawnText("the hit", "The search landed"); + + yield return SendKey(KeyCode.F3); + yield return WaitForLastLogLine( + "Match 2 of 80", + "F3 moved the search while one was set" + ); + + yield return RunCommand("clear-filter"); + yield return WaitForDrawnText( + MissMarker, + "Clearing the search brought the rest of the log back" + ); + + yield return SendKey(KeyCode.F3); + yield return Settle(); + + Assert.That( + DrawnText(), + Does.Contain(MissMarker), + "A key with no search set is left to the field, which does nothing with it either" + ); + } + + [UnityTest] + public IEnumerator TheSearchSurvivesANewLineThatDoesNotMatch() + { + yield return SpawnOpenTerminal(); + yield return FillTheLog(); + yield return RunCommand("find " + HitMarker); + yield return WaitForDrawnText("the hit", "The search landed"); + + Terminal.Log("output that arrived after the search and does not match"); + yield return Settle(); + + Assert.That( + DrawnText(), + Does.Contain("the hit"), + "New output does not end a search the developer is reading" + ); + Assert.That( + DrawnText(), + Does.Not.Contain("arrived after the search"), + "A line that does not match stays out of the view while the search is set" + ); + } + + [UnityTest] + public IEnumerator ANewLineThatMatchesStaysInTheSearch() + { + yield return SpawnOpenTerminal(); + yield return FillTheLog(); + yield return RunCommand("find " + HitMarker); + yield return WaitForDrawnText("the hit", "The search landed"); + + Terminal.Log("a later line with " + HitMarker + " in it"); + yield return WaitForDrawnText( + "a later line with", + "Output that matches joins the search" + ); + } + + [UnityTest] + public IEnumerator ASearchScrollsToItsFirstMatch() + { + yield return SpawnOpenTerminal(); + yield return FillTheLog(); + yield return RunCommand("find " + HitMarker); + yield return WaitForDrawnText("the hit", "The search landed"); + + yield return RequireLaidOutLog(); + + Scroller scroller = LogScroller(); + float highValue = scroller.highValue; + Assert.That( + 0f < highValue, + Is.True, + "The filtered log overflows the view, so the jump has somewhere to go" + ); + Assert.That( + scroller.value, + Is.EqualTo(0f).Within(Tolerance), + "A search lands on its first match, which is the top of the filtered log" + ); + } + + [UnityTest] + public IEnumerator SteppingScrollsToTheMatchItMovedTo() + { + yield return SpawnOpenTerminal(); + yield return FillTheLog(); + yield return RunCommand("find " + HitMarker); + yield return WaitForDrawnText("the hit", "The search landed"); + + yield return RequireLaidOutLog(); + + Assert.That( + 0f < LogScroller().highValue, + Is.True, + "The filtered log overflows the view, so the step has somewhere to go" + ); + + yield return RunCommand("find"); + yield return WaitForLogScrolledToChild( + 1, + "Stepping to the next match moves the view to it" + ); + + Assert.That( + LogScroller().value, + Is.GreaterThan(0f), + "The second match is below the first, so the view left the top for it" + ); + } + + /* + Waits for the scroller to hold the offset of one drawn child, which + is what a search jump writes: the line's own position in the + content. Asserting "greater than zero" instead would pass on any + scroll in either direction, including the scroll-to-end a run + triggers - which is the thing the jump has to beat. + */ + private IEnumerator WaitForLogScrolledToChild(int childIndex, string message) + { + Scroller scroller = LogScroller(); + int frameBudget = FrameBudget; + while (0 < frameBudget--) + { + float expected = ExpectedScrollForChild(childIndex); + if (0f < expected && Mathf.Abs(scroller.value - expected) < Tolerance) + { + break; + } + + yield return null; + } + + Assert.That( + scroller.value, + Is.EqualTo(ExpectedScrollForChild(childIndex)).Within(Tolerance), + message + ); + } + + /* + Where a search jump puts the view for a given drawn line, derived + the way the jump derives it. Zero when the line has no layout yet, + which is what keeps the poll from waiting on a surface its own + assert does not need. + */ + private float ExpectedScrollForChild(int childIndex) + { + VisualElement content = LogContent(); + if (childIndex < 0 || content.childCount <= childIndex) + { + return 0f; + } + + return content[childIndex].layout.yMin; + } + + private IEnumerator SpawnOpenTerminal() + { +#if UNITY_EDITOR + _panelSettings = ScriptableObject.CreateInstance(); + _terminalObject = new GameObject("TerminalUILogFilter"); + _terminalObject.SetActive(false); + UIDocument document = _terminalObject.AddComponent(); + document.panelSettings = _panelSettings; + _terminal = _terminalObject.AddComponent(); + _terminal._uiDocument = document; + _terminal.resetStateOnInit = true; + _terminal._themePack = LoadAsset("Packs/Themes/Medium.asset"); + _terminal._fontPack = LoadAsset("Packs/Fonts/Medium.asset"); + StartTracker tracker = _terminalObject.AddComponent(); + _terminalObject.SetActive(true); + yield return new WaitUntil(() => tracker.Started); +#else + Assert.Ignore("The log filter needs the editor Play Mode suite."); + yield break; +#endif + + _terminal.SetState(TerminalState.OpenFull); + + yield return null; + yield return null; + + int frameBudget = 600; + while (0 < frameBudget-- && _terminal._commandInput == null) + { + yield return null; + } + + Assert.That( + _terminal._commandInput, + Is.Not.Null, + "The terminal input field exists once it is open" + ); + DefaultTerminalInput.Instance.CommandText = string.Empty; + } + + private IEnumerator SendKey(KeyCode keyCode, EventModifiers modifiers = EventModifiers.None) + { + using (KeyDownEvent key = KeyDownEvent.GetPooled('\0', keyCode, modifiers)) + { + /* Sent at the root, which is what makes the key travel down to + the field the way a real keystroke does: a panel routes a + key by focus, and this synthetic panel has none, so an event + aimed at the field itself is dropped. */ + _terminal._uiDocument.rootVisualElement.SendEvent(key); + } + + yield return null; + yield return null; + } + + /* + Whether a synthetic key can reach the command field on this host at + all, measured rather than assumed. + + The F3 tests cannot be answered by a key that never arrives, and the + obvious way to find that out is to assert the step and read a + failure - which is indistinguishable from the routing being broken. + So the field is asked directly, with a callback registered the same + way the terminal registers its own: trickle-down, on the same + element, from an event sent at the same root. A host that drops + such an event cannot answer anything about key routing, and that + is a host limit to report rather than a defect to hunt for. + + `TerminalUIPasteTests.PasteReachesTheCommandTextAsAUserEdit` fails on + unmodified master on this host for this reason, measured both ways: + the same synthetic Ctrl+V never reaches the same field. + */ + private IEnumerator RequireReachableKeys() + { + bool reached = false; + _terminal._commandInput.RegisterCallback( + _ => reached = true, + TrickleDown.TrickleDown + ); + + using (KeyDownEvent key = KeyDownEvent.GetPooled('\0', KeyCode.F3, EventModifiers.None)) + { + _terminal._uiDocument.rootVisualElement.SendEvent(key); + } + + yield return null; + if (!reached) + { + Assert.Ignore( + "A synthetic key does not reach the command field in this environment, so " + + "nothing about key routing can be measured here: a key that never arrives " + + "looks exactly like routing that is broken. " + + "TerminalUIPasteTests.PasteReachesTheCommandTextAsAUserEdit fails on " + + "unmodified master on this host for the same reason. Run these where a " + + "panel routes keys." + ); + } + } + + private ScrollView LogView() + { + ScrollView logView = + _terminal._uiDocument.rootVisualElement.Q("LogScrollView") as ScrollView; + Assert.That(logView, Is.Not.Null, "The log scroll view exists on an open terminal"); + return logView; + } + + private VisualElement LogContent() + { + return LogView().contentContainer; + } + + private Scroller LogScroller() + { + ScrollView logView = LogView(); + Assert.That(logView.verticalScroller, Is.Not.Null, "It exposes a vertical scroller"); + return logView.verticalScroller; + } + + private int DrawnCount() + { + return LogContent().childCount; + } + + private string DrawnText() + { + VisualElement content = LogContent(); + int childCount = content.childCount; + StringBuilder builder = new(); + for (int i = 0; i < childCount; ++i) + { + if (0 < i) + { + builder.Append('\n'); + } + + builder.Append((content[i] as Label)?.text); + } + + return builder.ToString(); + } + + /* + The newest log line, read through `CopyTo` rather than `Logs`. A + count followed by an index is two reads, and a line that arrived + between them would make the poll wait for text that is no longer + the newest - which is a poll that can hang on a log that keeps + logging. One consistent read is the whole window. + */ + private string LastLogText() + { + CommandLog buffer = Terminal.Buffer; + if (buffer == null) + { + return string.Empty; + } + + LogItem[] window = new LogItem[Math.Max(buffer.Capacity, 2)]; + int count = buffer.CopyTo(window); + return count < 1 ? string.Empty : window[count - 1].message; + } + + private IEnumerator WaitForDrawnCount(int expected, string message) + { + int frameBudget = FrameBudget; + while (0 < frameBudget-- && DrawnCount() != expected) + { + yield return null; + } + + Assert.That(DrawnCount(), Is.EqualTo(expected), message); + } + + private IEnumerator WaitForDrawnText(string expected, string message) + { + int frameBudget = FrameBudget; + while (0 < frameBudget-- && !DrawnText().Contains(expected, StringComparison.Ordinal)) + { + yield return null; + } + + Assert.That(DrawnText(), Does.Contain(expected), message); + } + + private IEnumerator WaitForLastLogLine(string expected, string message) + { + int frameBudget = FrameBudget; + while (0 < frameBudget-- && !LastLogText().Contains(expected, StringComparison.Ordinal)) + { + yield return null; + } + + Assert.That(LastLogText(), Does.Contain(expected), message); + } + + /* + The two jump tests need a child position, and a child position is a + layout result. A headless editor lays out no UI Toolkit panel, so + the filtered log has no geometry and the scroller has nothing to + scroll; the other tests here read the drawn children, which exist + without a layout, and answer on this host. + */ + private IEnumerator RequireLaidOutLog() + { + /* + The scroller's range AND the viewport's own height. The range is + what the jump writes, but a view with a range and no height has + a scroller over a content that was never given room, and a child + in it has a position the product never showed - which would make + the offset this file asserts against a number no developer ever + saw. Both, held across frames, so a panel left half-built by an + earlier test cannot satisfy either one for a frame and lose it. + */ + int stableFrames = 0; + int frameBudget = FrameBudget; + while (0 < frameBudget-- && stableFrames < 5) + { + ScrollView view = LogView(); + bool laidOut = + 0f < LogScroller().highValue && 0f < view.contentViewport.layout.height; + stableFrames = laidOut ? stableFrames + 1 : 0; + yield return null; + } + + if (stableFrames < 5) + { + Assert.Ignore( + "The log view did not hold a layout in this environment, so a child has no " + + "position and the search cannot be seen to jump to its match. A headless " + + "editor with no rendered view cannot answer this; run it where the Game " + + "view renders." + ); + } + } + } +} diff --git a/Tests/Runtime/TerminalUILogFilterTests.cs.meta b/Tests/Runtime/TerminalUILogFilterTests.cs.meta new file mode 100644 index 0000000..444671f --- /dev/null +++ b/Tests/Runtime/TerminalUILogFilterTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 62435b0e0ff940318eb216b5488b4584 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: From aac68fd4b4c66e8101ea3a6c82f30bc866e074ab Mon Sep 17 00:00:00 2001 From: opencode Date: Tue, 29 Sep 2026 21:26:30 +0000 Subject: [PATCH 2/3] Stop a search counting and losing to the console around it Two review findings, both in the product. EnterCommand re-attaches the tail and asks for a scroll to the end after the handler returns, overwriting the jump a search queued, so the view went to the end and the jump lost the race. A search also matched the line it wrote to say what it found, so a query that was a word in that line reported a hit for a string appearing nowhere, and every repeat added another. Co-Authored-By: Claude Opus 4.8 (1M context) --- .llm/skills/index.md | 2 +- .llm/skills/log-echo-contract/SKILL.md | 71 ++++++++++++++- CHANGELOG.md | 2 +- README.md | 4 +- Runtime/CommandTerminal/UI/LogFilter.cs | 42 ++++++++- Runtime/CommandTerminal/UI/TerminalUI.cs | 91 ++++++++++++++++---- Tests/Runtime/TerminalUILogFilterTests.cs | 100 ++++++++++++++++++++++ 7 files changed, 290 insertions(+), 22 deletions(-) diff --git a/.llm/skills/index.md b/.llm/skills/index.md index 04c57eb..d285bbd 100644 --- a/.llm/skills/index.md +++ b/.llm/skills/index.md @@ -34,7 +34,7 @@ Agent Skills ([SKILL.md format](https://agentskills.io)) for specific tasks. Inv | [context-aware-completion](./context-aware-completion/SKILL.md) | Register context-aware commands with CommandDefinition, gate execution by CommandExecutionContexts, and attach CommandCompletionProvider argument completion (staged providers, TryComplete, TerminalUI token cycling) in DxCommandTerminal. Use when writing commands that need argument completion or execution-context gating, wiring chained completions like item name then item action, or debugging why a command does not run or complete. | | [custom-argument-parsing](./custom-argument-parsing/SKILL.md) | Parse CommandArg values with TryGet, register custom CommandArgParser functions, and control input cleaning, delimiters, and quote handling sets. Use when writing command handlers that read arguments, adding parsers for new types (e.g. JSON), or fixing "failed to parse" behavior. | | [input-system-integration](./input-system-integration/SKILL.md) | Wire DxCommandTerminal input - configurable keyboard hotkeys, Unity's new Input System / PlayerInput bindings, the HandlePrevious/HandleNext/ToggleSmall/ToggleFull/CompleteCommand/EnterCommand messages, and input precedence. Use when adding or changing terminal keybindings, PlayerInput wiring, or fixing input handling bugs (including WebGL). | -| [log-echo-contract](./log-echo-contract/SKILL.md) | Preserve the DxCommandTerminal log echo contract - both console surfaces write the typed line into Terminal.Buffer as TerminalLogType.Input before a command handler runs, so a handler that reads the log sees its own echo as the newest entry. Covers OutputLength, why a fixed -1/-2 offset is wrong, why the palette and the copy commands filter differently, why the log view must still SHOW echoes, and the test-helper trap that hides all of it. Use when writing or reviewing a command that reads the log, when a command returns its own name or a command line instead of a message, when touching trace/copy-last/copy-log/CommandPaletteUI.CollectOutput, or when a log-reading test passes against code the product never runs. | +| [log-echo-contract](./log-echo-contract/SKILL.md) | Preserve the DxCommandTerminal log echo contract - both console surfaces write the typed line into Terminal.Buffer as TerminalLogType.Input before a command handler runs, so a handler that reads the log sees its own echo as the newest entry, and a command that answers in the log writes console text that later readers must not count as output. Covers OutputLength, why a fixed -1/-2 offset is wrong, why the palette and the copy commands filter differently, why the log view must still SHOW echoes, why a search excludes the console's own replies by text rather than by type, why EnterCommand overwrites a handler's view state, and the test-helper traps that hide all of it. Use when writing or reviewing a command that reads or counts the log, when a command returns its own name or a command line instead of a message, when touching trace/copy-last/copy-log/find/clear-filter/CommandPaletteUI.CollectOutput, or when a log-reading test passes against code the product never runs. | | [register-terminal-command](./register-terminal-command/SKILL.md) | Register terminal commands in DxCommandTerminal via RegisterCommandAttribute or Terminal.Shell.AddCommand, including name inference, arg count bounds, hint text, editor/development-only flags, and ignoring built-in commands. Use when adding console commands/cheats, changing command signatures, debugging why a command is not recognized, or extending the typed builder and argument-spec internals. | | [run-terminal-tests](./run-terminal-tests/SKILL.md) | Write and run DxCommandTerminal PlayMode tests (Unity Test Runner, Tests/Runtime/, NUnit) following the repo's test style for CommandArg, CommandShell, and Terminal behavior. Use when adding tests, running the test suite, or debugging failing terminal tests. | | [session-facade](./session-facade/SKILL.md) | Explain and preserve the DxCommandTerminal backend session nullability contract - TerminalSession.Current is never null, the Terminal facade getters (Buffer/Shell/History/AutoComplete) read null before bootstrap and after play-session reset, TerminalUI and CommandPaletteUI apply or bootstrap the session on enable, and command registration stays deferred to first use. Use when touching TerminalUI/CommandPaletteUI lifecycle, adding session consumers, writing bootstrap code, reviewing null-chain questions on Terminal accessors, or debugging why Terminal.Log or Terminal.Shell is null. | diff --git a/.llm/skills/log-echo-contract/SKILL.md b/.llm/skills/log-echo-contract/SKILL.md index a360327..87723ad 100644 --- a/.llm/skills/log-echo-contract/SKILL.md +++ b/.llm/skills/log-echo-contract/SKILL.md @@ -1,6 +1,6 @@ --- name: log-echo-contract -description: Preserve the DxCommandTerminal log echo contract - both console surfaces write the typed line into Terminal.Buffer as TerminalLogType.Input before a command handler runs, so a handler that reads the log sees its own echo as the newest entry. Covers OutputLength, why a fixed -1/-2 offset is wrong, why the palette and the copy commands filter differently, why the log view must still SHOW echoes, and the test-helper trap that hides all of it. Use when writing or reviewing a command that reads the log, when a command returns its own name or a command line instead of a message, when touching trace/copy-last/copy-log/CommandPaletteUI.CollectOutput, or when a log-reading test passes against code the product never runs. +description: Preserve the DxCommandTerminal log echo contract - both console surfaces write the typed line into Terminal.Buffer as TerminalLogType.Input before a command handler runs, so a handler that reads the log sees its own echo as the newest entry, and a command that answers in the log writes console text that later readers must not count as output. Covers OutputLength, why a fixed -1/-2 offset is wrong, why the palette and the copy commands filter differently, why the log view must still SHOW echoes, why a search excludes the console's own replies by text rather than by type, why EnterCommand overwrites a handler's view state, and the test-helper traps that hide all of it. Use when writing or reviewing a command that reads or counts the log, when a command returns its own name or a command line instead of a message, when touching trace/copy-last/copy-log/find/clear-filter/CommandPaletteUI.CollectOutput, or when a log-reading test passes against code the product never runs. metadata: category: Feature --- @@ -66,6 +66,67 @@ belongs on screen. The contract above is about reading the log programmatically for a result, not about what is displayed. Do not "fix" the view by filtering echoes out. +## The console's own replies are not output either + +The other writer of non-game text is the console answering itself. A command +that must answer in the log (`#186`) writes ordinary log text, and that text is +in the window every later reader sees. + +This bit the log search: a query that happened to be a word in the search's own +answer - `search`, `log`, `line`, `clear-filter` - matched the answer, so a +search that hit nothing reported a hit and every repeat added another. The +echo exclusion above does not help; a reply is a `Message` or a `Warning`, not +an `Input`. + +**A search excludes the console's own lines by exact text, not by type.** The +types belong to the game - a `Warning` is what the developer is looking for, and +a `ShellMessage` is any `Terminal.Log` the game made - so no type means "the +console said this". `LogFilter.IgnoreOwnReply` is called from the same door +that logs (`TerminalUI.LogFindReply` / `LogFindWarning`), so a new reply cannot +be added without registering it. Register the exact string that is logged: a +message with format arguments reaches the log formatted and the filter holding +the format, and the two stop being the same line. + +Do not exclude the whole `Warning` type, and do not exclude trailing replies +positionally - a positional exclusion makes the count depend on whether +anything has been logged since, so the same search reports a different number +on the frame the developer runs their next command. + +`copy-log` keeps console replies in its transcript on purpose (see the table +above): a transcript wants the record of what you did. The difference is that +`copy-log` reports no count the developer decides anything from, and the search +does. + +## A handler's UI intent is overwritten by the code that ran it + +`TerminalUI.EnterCommand` re-attaches the log tail and asks for a scroll to the +end **after** the handler returns, because running a command is a request for +its output: + +``` +Terminal.Log(Input, text); shell.RunCommand(text); // <- the handler sets the view +_logTail.Attach(); _needsScrollToEnd = true; // <- and then overrides it +``` + +A handler whose whole purpose is where the view ends up has to survive that. +`EnterCommand` skips the re-assert when a search jump is queued, and it is the +only site that can know, so the decision belongs there. + +Two consequences for tests: + +- **Dispatch through `EnterCommand`, not the shell**, for anything about where + the view ends up. `shell.RunCommand` never re-attaches, so a test that uses + it cannot see this class of bug at all. +- **Assert the state synchronously.** The jump is dropped once its budget runs + out, so a test that yields a frame or two before looking finds the flag + cleared on a *correct* build and passes straight over a broken one. Expose + the flag (`TerminalUI.FindScrollQueued`, `WantsScrollToEnd`) and read it in + the frame the call returns. + +Do not expose the follower's `Detached` for this: it only flips when a scroll +is actually placed, so on a host with no laid-out view it reads the same whether +or not anything is wrong. + ## The test-helper trap (this is how the bug survived two review rounds) A test helper that calls `shell.RunCommand(line)` directly exercises a path the @@ -99,6 +160,14 @@ When a command starts reading the log: 4. Check the other two consumers still agree: `CommandTrace`, `CommandPaletteUI.CollectOutput`, `TerminalUI.RefreshLogs`. +When a command starts **counting** the log, or reading it to drive a view: + +5. Which of the lines in the window are the console's own? Echoes (`Input`) and + the command's own replies. A count that includes either is a number the + developer will act on and be wrong by. +6. Does the handler's effect on the view survive the call that ran it? See + "A handler's UI intent is overwritten by the code that ran it". + ## Related - [session-facade](../session-facade/SKILL.md) - the nullability boundary on diff --git a/CHANGELOG.md b/CHANGELOG.md index fc204a3..c8fef57 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Added -- `find ` searches the log view: it keeps only the lines holding the text, jumps to the first match, and reports how many lines matched out of how many the search ranged over. `find` with no argument steps to the next match and wraps at the end, F3 and Shift+F3 do the same from the keyboard, and `clear-filter` shows every line again. Until now a log too long to read had no way in but dragging a 10px scrollbar: with the default 256-entry buffer, finding one error meant paging through every line that was not it, and the log view is the only place a message ever appears. The count is the answer to "did my search hit", which a view showing some lines cannot give. A search that matched nothing reports that and stays set, so a typo does not silently put the whole log back on screen. A query is the arguments joined back into one string, so `find "two words"` searches for two words, and an empty or whitespace-only query is refused so it cannot drop the search already in place. The search runs over the text as the log shows it, so what is on screen is what can be found, and it ignores case. It does not find commands you ran: both console surfaces echo the typed line into the log as an `Input` entry before the handler runs, and a search that matched its own echo reported a hit for every query, including one that appears nowhere. `clear-console` does not drop the search - the view goes empty until the next `clear-filter` - and the query is recorded only as the `find` line in the log, so it is not recoverable once that line rotates out of the buffer. +- `find ` searches the log view: it keeps only the lines holding the text, jumps to the first match, and reports how many lines matched out of how many the search ranged over. `find` with no argument steps to the next match and wraps at the end, F3 and Shift+F3 do the same from the keyboard, and `clear-filter` shows every line again. Until now a log too long to read had no way in but dragging a 10px scrollbar: with the default 256-entry buffer, finding one error meant paging through every line that was not it, and the log view is the only place a message ever appears. The count is the answer to "did my search hit", which a view showing some lines cannot give. A search that matched nothing reports that and stays set, so a typo does not silently put the whole log back on screen, and the line the search writes to say so is never one of its own results - a query that happened to be a word in that line (`search`, `log`, `clear-filter`) otherwise matched it, and the count grew with every repeat. A query is the arguments joined back into one string, so `find "two words"` searches for two words, and an empty or whitespace-only query is refused so it cannot drop the search already in place. The search runs over the text as the log shows it, so what is on screen is what can be found, and it ignores case. It does not find commands you ran: both console surfaces echo the typed line into the log as an `Input` entry before the handler runs, and a search that matched its own echo reported a hit for every query, including one that appears nowhere. `clear-console` does not drop the search - the view goes empty until the next `clear-filter` - and the query is recorded only as the `find` line in the log, so it is not recoverable once that line rotates out of the buffer. - `copy-last` and `copy-log [n]` put the console log on the system clipboard: the newest line, or the last N, joined with newlines. Until now nothing could be read out of the console at all - 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 `Command 'give' threw ArgumentException` in a device build had to retype it or screenshot the game window. Both surfaces echo the line you typed into the log before the command runs, so a copy skips the echoes at the newest end: `copy-last` returns the line you were reading, not the word `copy-last`. An echo further back stays, because a transcript of a session wants the commands in it as much as the output. A copied line is the line the log shows, so the stack trace is not included (`trace` is how you read one) and copying 256 lines does not carry 256 traces. A clipboard write is a request a platform can decline - tvOS has none, and a browser may refuse without a user gesture - so the write is read back and compared, and a copy that did not happen says so instead of silently doing nothing. An empty log, a log holding only commands, a count that is not a number, and a count below one are each reported rather than ignored. - The log scrolls with the keyboard while the command line holds focus. Page Up and Page Down page it by one viewport, and Ctrl+Home (Cmd+Home on macOS) and Ctrl+End reach its oldest and newest lines, so a developer looking for one error in a full 256-entry buffer no longer has to drag a 10px scrollbar. Home and End without the modifier stay with the command line, where they move the caret as a text field should, and every other key is untouched. Paging back detaches the tail exactly as scrolling back does, so new output no longer pulls the view to the end while it is being read; Ctrl+End is the way back to following. A page that reaches the end leaves the log following rather than parked a line short of an end that is still growing. - `CommandLog.CopyTo(LogItem[] destination)` copies the log's visible window, oldest first, into a caller-owned array and returns how many entries it wrote. Reading `CommandLog.Logs` counts and then indexes as two separate reads, so a log written from another thread between them can be missed or, if the buffer was cleared or shrunk, throw. One `CopyTo` call is one consistent view. A destination shorter than the window truncates to its oldest entries, so size it for the largest buffer the session configures. diff --git a/README.md b/README.md index 121ba48..5c54ede 100644 --- a/README.md +++ b/README.md @@ -491,13 +491,13 @@ The log holds the newest 256 lines by default and nothing else, so reaching one - `find` on its own steps to the next match, wrapping at the end; **F3** and **Shift+F3** do the same from the keyboard. - `clear-filter` shows every line again. -Each reports what it did - `Showing 20 of 240 log lines.`, or `Match 3 of 80.` - so a search that hit nothing says so instead of leaving you looking at an empty log, and the search stays set so a typo does not silently put everything back. A query is the arguments joined back into one string, so `find "two words"` searches for two words. An empty query is refused and leaves the search you had alone. +Each reports what it did - `Showing 20 of 240 log lines.`, or `Match 3 of 80.` - so a search that hit nothing says so instead of leaving you looking at an empty log, and the search stays set so a typo does not silently put everything back. A query is the arguments joined back into one string, so `find "two words"` searches for two words. An empty query is refused and leaves the search you had alone. The count does not move because the search talked: its own answer is not one of the results, so the same search reports the same number however many times you run it. The search is over the text as the log shows it, so what you can see is what you can find, and it ignores case: `nullref` reaches `NullReferenceException`. Four limits worth knowing before you rely on it: -- It does not find the commands you ran. Both surfaces echo the line you typed into the log, and a search that matched its own echo would report a hit for every query, including one that appears nowhere. Use `copy-log` for a transcript with your commands in it. +- It does not find the commands you ran, or the console's own answers. Both surfaces echo the line you typed into the log, and a search that matched its own echo would report a hit for every query, including one that appears nowhere; the same goes for the line the search writes when it tells you what it found. Use `copy-log` for a transcript with your commands in it. - The query is only recorded as the `find` line in the log. Once that line rotates out of the buffer - or you run `clear-console` - nothing on screen says the view is filtered, and the query is gone. `clear-filter` is how you put the log back. - `clear-console` does not drop the search. The view goes empty until the next `clear-filter` or a new `find`. - F3 and Shift+F3 need the command line to hold focus, as the paging keys do, and the jump to a match needs a log view that has been laid out. A view with no rendered Game view filters but does not scroll. diff --git a/Runtime/CommandTerminal/UI/LogFilter.cs b/Runtime/CommandTerminal/UI/LogFilter.cs index 2993a59..1c067cd 100644 --- a/Runtime/CommandTerminal/UI/LogFilter.cs +++ b/Runtime/CommandTerminal/UI/LogFilter.cs @@ -1,6 +1,7 @@ namespace WallstopStudios.DxCommandTerminal.UI { using System; + using System.Collections.Generic; using Backend; /* @@ -39,6 +40,26 @@ internal sealed class LogFilter /* One-based, so the number a developer reads is the number reported. */ public int? CurrentMatch { get; private set; } + /* + The console's own answers about the search, which are not results. + + Every command has to answer in the console, and the answer is + ordinary log text. A search that answered into its own results + would be searching its own sentences: the words in them are + ordinary words, so a query that happens to be one of them - + "search", "log", "line", "clear-filter" - matched the answer, and + every repeat of the search added another match. A search that hit + nothing then reported a hit, which is the one answer it must never + give. + + By exact text rather than by type, because the types belong to the + game: a `Warning` is what the developer is looking for, and a + `ShellMessage` is any `Terminal.Log` the game made. The search's + own lines are neither, and there are only ever a handful - one per + command the developer ran. + */ + private readonly HashSet _ownReplies = new(StringComparer.Ordinal); + /* Whether a line survives the search. The message is never null - `LogItem` is the only way to make one and it coalesces - and the @@ -87,6 +108,21 @@ private static bool IsSearchable(LogItem item) return item.type != TerminalLogType.Input; } + /* + Registers the text of a line the search is about to write, so + `Apply` will not count it. The caller logs exactly the string it + registers: a message carrying format arguments would reach the log + formatted and the filter holding the format, and the two would + stop being the same line. + */ + public void IgnoreOwnReply(string message) + { + if (!string.IsNullOrEmpty(message)) + { + _ownReplies.Add(message); + } + } + /* Replaces the query and forgets the position: the new query has a different set of matches, and the old position names one of them @@ -174,7 +210,11 @@ would report the two from different sets. } ++searchable; - if (kept < destination.Length && Matches(item, query)) + if ( + kept < destination.Length + && !_ownReplies.Contains(item.message) + && Matches(item, query) + ) { destination[kept] = item; ++kept; diff --git a/Runtime/CommandTerminal/UI/TerminalUI.cs b/Runtime/CommandTerminal/UI/TerminalUI.cs index 12852ff..59e7687 100644 --- a/Runtime/CommandTerminal/UI/TerminalUI.cs +++ b/Runtime/CommandTerminal/UI/TerminalUI.cs @@ -76,6 +76,26 @@ window height is still settling toward its target. */ internal TerminalState State => _state; + /* + Internal for test coverage of the search's scroll state (see + WallstopStudios.DxCommandTerminal.Tests.Runtime). + + A search queues a jump to its match and suppresses the + scroll-to-end that running a command asks for; `EnterCommand` + re-asserts that scroll after the handler returns, and whether the + search's suppression survived is invisible without a laid-out view - + which is exactly the gap that let the re-assert overwrite the jump + silently. The flags are the decision, and they read the same on a + host that lays out no panel. + + The follower's own `Detached` is deliberately not exposed here: it + only flips when a scroll is actually placed, so on a host with no + layout it reads the same whether or not anything is wrong. + */ + internal bool FindScrollQueued => _pendingFindScroll.HasValue; + + internal bool WantsScrollToEnd => _needsScrollToEnd; + [Header("Window")] [Range(0, 1)] public float maxHeight = 0.7f; @@ -1588,13 +1608,27 @@ public void EnterCommand() _input.CommandText = string.Empty; _needsFocus = true; + /* Running a command is an explicit request for its output, so the tail follows again even when the developer had scrolled away to read an earlier one. + + A search is the exception, and this is the only site that + can know it. The handler ran three lines above, and a + search that queued a jump to its match asked for that line, + not for the newest one. Re-attaching here overwrites the + suppression the jump sets for itself, so the view scrolls + to the end and the jump then either loses the race or + spends its budget waiting for a layout that keeps moving. + A developer watching that sees a search that filtered + correctly and then ignored them. */ - _logTail.Attach(); - _needsScrollToEnd = true; + if (!_pendingFindScroll.HasValue) + { + _logTail.Attach(); + _needsScrollToEnd = true; + } } finally { @@ -2018,7 +2052,7 @@ a play-session reset. Reported rather than treated as "no matches", which would be an answer about a log that is not there. */ - Terminal.Log(TerminalLogType.Warning, "There is no log to search yet."); + LogFindWarning("There is no log to search yet."); return; } @@ -2029,10 +2063,7 @@ a play-session reset. Reported rather than treated as "no jump it had queued: a refused query is not a request to change what is on screen. */ - Terminal.Log( - TerminalLogType.Warning, - "Nothing to search for. clear-filter shows every log line again." - ); + LogFindWarning("Nothing to search for. clear-filter shows every log line again."); return; } @@ -2046,8 +2077,7 @@ would have to type the search again to see that it had hit nothing. */ DropFindScroll(); - Terminal.Log( - TerminalLogType.Warning, + LogFindWarning( "No log line matches the search. clear-filter shows every line again." ); return; @@ -2061,7 +2091,7 @@ ones it did - so the total and the match count are not interchangeable words here, and "matching log lines" on the total would say the opposite of what the number means. */ - Terminal.Log($"Showing {matches} of {_logFilter.TotalCount} log lines."); + LogFindReply($"Showing {matches} of {_logFilter.TotalCount} log lines."); } /* @@ -2077,7 +2107,7 @@ internal void StepLogFilter(bool forward) { if (!_logFilter.IsActive) { - Terminal.Log(TerminalLogType.Warning, "No search is set. find sets one."); + LogFindWarning("No search is set. find sets one."); return; } @@ -2088,32 +2118,61 @@ internal void StepLogFilter(bool forward) no log there is nothing to step through - stepping anyway would answer "Match 7 of 20" for a log that holds nothing. */ - Terminal.Log(TerminalLogType.Warning, "There is no log to search yet."); + LogFindWarning("There is no log to search yet."); return; } ReadRenderedLogWindow(Terminal.Buffer, out _); if (!(forward ? _logFilter.StepForward() : _logFilter.StepBackward())) { - Terminal.Log(TerminalLogType.Warning, "No log line matches the search."); + LogFindWarning("No log line matches the search."); return; } QueueFindScroll(); - Terminal.Log($"Match {_logFilter.CurrentMatch} of {_logFilter.MatchCount}."); + LogFindReply($"Match {_logFilter.CurrentMatch} of {_logFilter.MatchCount}."); } internal void ClearLogFilter() { if (!_logFilter.IsActive) { - Terminal.Log("No search is set."); + LogFindReply("No search is set."); return; } _logFilter.Clear(); DropFindScroll(); - Terminal.Log("Search cleared. The log shows every line again."); + LogFindReply("Search cleared. The log shows every line again."); + } + + /* + Every line the search writes, through one door. + + A command that answers in the console writes ordinary log text, and + the search's answer is no different from any other until something + stops counting it. The words in it are ordinary words, so a query + that happened to be one of them - "search", "log", "clear-filter" - + matched the answer, and every repeat of the search added another + match. A search that hit nothing would then report a hit, which is + the one answer it must never give. + + Registering the text is what keeps the count honest; the log type + cannot do it. A `Warning` is exactly what a developer is looking + for, and a `ShellMessage` is any `Terminal.Log` the game made. The + lines that are neither are the console's own, and there are only + ever a handful - one per command the developer ran. + */ + private void LogFindReply(string message) + { + _logFilter.IgnoreOwnReply(message); + Terminal.Log(message); + } + + private void LogFindWarning(string message) + { + _logFilter.IgnoreOwnReply(message); + Terminal.Log(TerminalLogType.Warning, message); } /* diff --git a/Tests/Runtime/TerminalUILogFilterTests.cs b/Tests/Runtime/TerminalUILogFilterTests.cs index 9a96720..9acbf4e 100644 --- a/Tests/Runtime/TerminalUILogFilterTests.cs +++ b/Tests/Runtime/TerminalUILogFilterTests.cs @@ -380,6 +380,87 @@ public IEnumerator ANewLineThatMatchesStaysInTheSearch() ); } + [UnityTest] + public IEnumerator ASearchSurvivesTheCommandThatRanIt() + { + /* + The finding this pins: `EnterCommand` re-attaches the tail and + asks for a scroll to the end AFTER the handler returns, because + running a command is a request for its output. A search handler + queues a jump to its match, and the attach overwrote it - so + the view went to the end and the jump either lost the race or + spent its budget waiting for a layout that kept moving. + + Driven through `EnterCommand` rather than the shell, because the + shell never re-attaches and so cannot see this at all. + + Asserted synchronously, in the frame `EnterCommand` returns, + and that is not a shortcut. The jump is dropped once its budget + runs out - four refreshes with no layout - so a test that + waited a frame or two to look would find the flag cleared on a + correct build and pass straight over a broken one. The decision + is made inside that call and nowhere else, so this is the only + moment it is observable - on any host, laid out or not, which + is what a state no view can show has to be. + */ + yield return SpawnOpenTerminal(); + yield return FillTheLog(); + + DefaultTerminalInput.Instance.CommandText = "find " + HitMarker; + _terminal.EnterCommand(); + + Assert.That( + _terminal.FindScrollQueued, + Is.True, + "The jump the search queued survived the run that queued it" + ); + Assert.That( + _terminal.WantsScrollToEnd, + Is.False, + "The run did not re-assert a scroll to the end over the search's jump" + ); + } + + [UnityTest] + public IEnumerator ASearchNeverCountsItsOwnAnswer() + { + /* + The second finding. A search that hit nothing answers in the + log, and the answer is ordinary text - so a query that happens + to be a word in it, "search" here, matched the answer. The + search reported a hit for a string that appears nowhere, and + each repeat of the search added another. + + The query is chosen to be a word the answer really contains, so + this fails whenever the exclusion is removed, and it is the + answer's own words that make it collide - not the query. + */ + yield return SpawnOpenTerminal(); + yield return FillTheLog(); + + yield return RunThroughConsole("find search"); + yield return WaitForLastLogLine( + "No log line matches", + "Nothing in the log holds the word the search looked for" + ); + yield return Settle(); + + Assert.That( + DrawnText(), + Is.Empty, + "The answer to a search that hit nothing is not itself a result" + ); + + yield return RunThroughConsole("find"); + yield return Settle(); + + Assert.That( + LastLogText(), + Does.Contain("No log line matches"), + "A repeat of the search does not find the answer the first one wrote" + ); + } + [UnityTest] public IEnumerator ASearchScrollsToItsFirstMatch() { @@ -586,6 +667,25 @@ private ScrollView LogView() return logView; } + /* + The same command through the real funnel, `EnterCommand` and all. + + `EnterCommand` re-attaches the log tail after the handler returns, + because running a command is a request for its output. That runs + AFTER anything the handler set up, so a handler whose whole point + is where the view ends up has to survive the code that called it - + and only this path exercises that ordering. Every other test here + dispatches through the shell, which never re-attaches, so none of + them can see it. + */ + private IEnumerator RunThroughConsole(string line) + { + DefaultTerminalInput.Instance.CommandText = line; + _terminal.EnterCommand(); + yield return null; + yield return null; + } + private VisualElement LogContent() { return LogView().contentContainer; From 86da0b197826a9d5824b911f8b966d95d8d21796 Mon Sep 17 00:00:00 2001 From: opencode Date: Tue, 29 Sep 2026 21:51:10 +0000 Subject: [PATCH 3/3] Keep the search's own answers out of the total as well They were already out of the matches, so the numerator held while the denominator grew by one per run: 20 of 240, then 20 of 241. The exclusion now runs before both counters. Co-Authored-By: Claude Opus 4.8 (1M context) --- .llm/skills/log-echo-contract/SKILL.md | 10 +++++- Runtime/CommandTerminal/UI/LogFilter.cs | 22 ++++++------- Tests/Editor/LogFilterTests.cs | 40 +++++++++++++++++++++++ Tests/Runtime/TerminalUILogFilterTests.cs | 34 +++++++++++++++++++ 4 files changed, 94 insertions(+), 12 deletions(-) diff --git a/.llm/skills/log-echo-contract/SKILL.md b/.llm/skills/log-echo-contract/SKILL.md index 87723ad..670ffa6 100644 --- a/.llm/skills/log-echo-contract/SKILL.md +++ b/.llm/skills/log-echo-contract/SKILL.md @@ -87,6 +87,13 @@ be added without registering it. Register the exact string that is logged: a message with format arguments reaches the log formatted and the filter holding the format, and the two stop being the same line. +**Apply the exclusion to the denominator as well as the numerator.** Excluding +a line from the matches but leaving it in the total reports "20 of 240" and then +"20 of 241" on two runs of the same search. The matches hold; the number the +developer reads moves because the search said something. `LogFilter.Apply` skips +own replies before both counters for exactly this reason, and both halves are +asserted separately, because checking only the matches passes the broken shape. + Do not exclude the whole `Warning` type, and do not exclude trailing replies positionally - a positional exclusion makes the count depend on whether anything has been logged since, so the same search reports a different number @@ -164,7 +171,8 @@ When a command starts **counting** the log, or reading it to drive a view: 5. Which of the lines in the window are the console's own? Echoes (`Input`) and the command's own replies. A count that includes either is a number the - developer will act on and be wrong by. + developer will act on and be wrong by - and a count that includes either in + only one of its two halves is the same defect wearing a pass. 6. Does the handler's effect on the view survive the call that ran it? See "A handler's UI intent is overwritten by the code that ran it". diff --git a/Runtime/CommandTerminal/UI/LogFilter.cs b/Runtime/CommandTerminal/UI/LogFilter.cs index 1c067cd..d2e7a79 100644 --- a/Runtime/CommandTerminal/UI/LogFilter.cs +++ b/Runtime/CommandTerminal/UI/LogFilter.cs @@ -187,11 +187,15 @@ rather than an index past the end of its own array. string query = Query; /* - Two counters, one pass. `searchable` is the denominator the - developer reads, and it is every line the search could have - matched: an echo is excluded from the matches, so counting it - here would report "20 of 61" beside a rule that says 61 lines - were never candidates. + Two counters, one pass, and the same exclusion from both. + + `searchable` is the denominator the developer reads: the lines + the search could have matched. A console reply is not one of + them, because the rule below rejects it - so counting it here + would report "20 of 241" and then "20 of 242" on two runs of + the same search, a number that moved because the search said + something. A count the developer uses to decide whether their + query is real has to be the same number every time. The loop runs to the end of the window even once the destination is full, because a truncated `kept` with a complete @@ -204,17 +208,13 @@ would report the two from different sets. for (int i = 0; i < searchableEnd; ++i) { LogItem item = window[i]; - if (!IsSearchable(item)) + if (!IsSearchable(item) || _ownReplies.Contains(item.message)) { continue; } ++searchable; - if ( - kept < destination.Length - && !_ownReplies.Contains(item.message) - && Matches(item, query) - ) + if (kept < destination.Length && Matches(item, query)) { destination[kept] = item; ++kept; diff --git a/Tests/Editor/LogFilterTests.cs b/Tests/Editor/LogFilterTests.cs index 89ff2e4..35823d0 100644 --- a/Tests/Editor/LogFilterTests.cs +++ b/Tests/Editor/LogFilterTests.cs @@ -378,6 +378,46 @@ public void ClearingDropsTheQueryAndTheCounts() Assert.That(filter.StepForward(), Is.False); } + [Test] + public void TheSearchsOwnAnswersAreInNeitherHalfOfTheCount() + { + /* + The exclusion has to cover the total as well as the matches. A + line dropped from the matches but kept in the denominator + reports "20 of 241" and then "20 of 242" on two runs of the + same search: the matches hold, and the number the developer + reads moves because the search said something. + */ + LogFilter filter = new(); + filter.IgnoreOwnReply("Showing 20 of 3 log lines."); + Assert.That(filter.SetQuery("hit"), Is.True); + + LogItem[] first = + { + new LogItem(TerminalLogType.Message, "hit one", string.Empty), + new LogItem(TerminalLogType.Message, "miss", string.Empty), + new LogItem(TerminalLogType.Message, "hit two", string.Empty), + }; + int firstKept = filter.Apply(first, first.Length, new LogItem[first.Length]); + int firstTotal = filter.TotalCount; + + LogItem[] second = + { + new LogItem(TerminalLogType.Message, "hit one", string.Empty), + new LogItem(TerminalLogType.Message, "miss", string.Empty), + new LogItem(TerminalLogType.Message, "hit two", string.Empty), + new LogItem(TerminalLogType.Message, "Showing 20 of 3 log lines.", string.Empty), + }; + int secondKept = filter.Apply(second, second.Length, new LogItem[second.Length]); + + Assert.That(secondKept, Is.EqualTo(firstKept), "The answer is not a match"); + Assert.That( + filter.TotalCount, + Is.EqualTo(firstTotal), + "And it is not a line the search ranged over either" + ); + } + [Test] public void AnUnfilteredFilterKeepsTheWholeWindow() { diff --git a/Tests/Runtime/TerminalUILogFilterTests.cs b/Tests/Runtime/TerminalUILogFilterTests.cs index 9acbf4e..cb48e54 100644 --- a/Tests/Runtime/TerminalUILogFilterTests.cs +++ b/Tests/Runtime/TerminalUILogFilterTests.cs @@ -380,6 +380,40 @@ public IEnumerator ANewLineThatMatchesStaysInTheSearch() ); } + [UnityTest] + public IEnumerator TheReportedCountIsTheSameEveryTimeTheSameSearchRuns() + { + /* + The finding this pins, and the second half of the same one. The + search's own answer was excluded from the matches but still + counted in the total, so every run added one to the + denominator: "20 of 240", then "20 of 241", then "20 of 242". + The matches held and the number the developer reads did not, + and a count that moves when nothing else did is the same + failure as a count that never settles. + + Three runs, because the first is the one that would pass either + way - the defect needs a second answer to count. + */ + yield return SpawnOpenTerminal(); + yield return FillTheLog(); + + yield return RunThroughConsole("find " + HitMarker); + yield return WaitForLastLogLine("of 240 log lines", "The first run reports a count"); + + yield return RunThroughConsole("find " + HitMarker); + yield return WaitForLastLogLine( + "of 240 log lines", + "The second run reports the same count, so the first answer is in neither half" + ); + + yield return RunThroughConsole("find " + HitMarker); + yield return WaitForLastLogLine( + "of 240 log lines", + "The third run too, so neither answer moved the number" + ); + } + [UnityTest] public IEnumerator ASearchSurvivesTheCommandThatRanIt() {