Bound logLines and reset it on deactivate - #25
Open
Chirag6722 wants to merge 1 commit into
Open
Conversation
Fixes thegoodengineer#5 logLines had no ceiling. One line per recorded command meant it grew for as long as the window stayed open, in a codebase that otherwise caps what it retains: history has truthlog.maxEntries, output has truthlog.maxOutputBytes, this had nothing. It is also module state that deactivate() did not clear while outputChannel was. Across a deactivate/activate cycle in the same extension host, a new session started holding the previous session's lines, and getLogLines() handed tests both sessions mixed together. Cap at 5000 lines, drop oldest first, and clear on deactivate. The OutputChannel is untouched and still holds the full log, which is what a user actually reads; this array exists only because VS Code has no API for reading a channel back. The trimming lives in an exported appendBounded() so it can be tested against a local array. Driving it through log() would mean pushing thousands of lines into the shared buffer that cmdExeDetection and the activation suite read by offset, which is exactly the sort of index shift that would break them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #5, per your direction: ring buffer plus a reset in
deactivate().logLineshad no ceiling, in a codebase that caps everything else it retains: history hastruthlog.maxEntries, output hastruthlog.maxOutputBytes, this had nothing. One line per recorded command means it grows for as long as the window stays open.It was also module state that
deactivate()left alone while clearingoutputChannel. Across a deactivate/activate cycle in the same extension host, a new session starts holding the previous session's lines, andgetLogLines()hands the tests both sessions mixed together.Capped at 5000 lines, oldest dropped first, cleared on deactivate. The
OutputChannelis untouched and still holds the full log, which is what a user actually reads; this array exists only because VS Code gives no way to read a channel back.One thing worth a look in review. The trimming lives in an exported
appendBounded()and the tests drive that against a local array, rather than drivinglog()itself. That is not squeamishness about a wide test API, it is a hazard I hit while writing it:cmdExeDetectionand the activation suite readgetLogLines()by recording an offset and slicing from it. Pushing thousands of lines through the shared buffer to reach the cap shifts those offsets, and once the buffer is full, an offset captured earlier points past the end, soslice(before)silently returns[]and the assertion fails for a reason unrelated to what it is testing. Testing the helper directly avoids putting that trap in the suite for whoever adds the next log-reading test.