refactor: make Terminal.SCROLL_BACK_COUNT static final - #34
Merged
loki5512344 merged 1 commit intoSep 1, 2026
Conversation
SCROLL_BACK_COUNT was a public mutable int field, read in index arithmetic buffer allocation, scrollback cap logic, and diff sizing. It was never assigned after declaration—verified via grep showing ONLY the public field declaration (line 48). Making it static final matches HEIGHT convention and prevents external mutation from desynchronizing the field from already-allocated buffers, which would cause ArrayIndexOutOfBoundsException. This is a no-behavior- change hardening change. Instance-qualified reads (terminal.SCROLL_BACK_COUNT) updated to class- qualified static access (Terminal.SCROLL_BACK_COUNT) in: - src/main/java/li/cil/oc2/common/vm/terminal/TerminalDiff.java (~2 reads) - src/main/java/li/cil/oc2/common/vm/terminal/buffer/TerminalBufferScrolling.java (~3 reads) - src/main/java/li/cil/oc2/common/vm/terminal/escapes/csi/CH8.java (1 read) - src/test/java/li/cil/oc2/common/vm/terminal/TerminalBufferTest.java (~5 reads) Internal unqualified reads in Terminal.java left as-is (conventional). §36 M6: class-qualified access for static fields preferred globally. No new tests needed—all existing buffer-sizing tests use SCROLL_BACK_COUNT for assertions and will adapt to the static qualifier automatically. Assisted by: GLM (syn:large:text on synthetic.new) — code generation and review.
pocketprobe
marked this pull request as ready for review
September 1, 2026 20:07
loki5512344
approved these changes
Sep 1, 2026
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
2 participants
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.
Summary
SCROLL_BACK_COUNTwas apublic intinstance field onTerminal, but it is neverassigned after declaration — it is a constant that controls scrollback buffer sizing
(buffer allocation, scrollback cap logic, diff row sizing). This makes it
public static final int, matching the existingHEIGHTconvention, and updates theinstance-qualified reads (
terminal.SCROLL_BACK_COUNT) to class-qualified reads.No behavior change. No new tests — the existing buffer-sizing tests already assert
against
SCROLL_BACK_COUNT.Why
A public mutable field participating in index arithmetic can be desynchronized from
buffers already allocated with the old value (external mutation after construction →
ArrayIndexOutOfBoundsException).static finalcloses that at compile time.This does not change the SpotBugs baseline: the
MS_SHOULD_BE_FINALentries inconfig/spotbugs/baseline.xmlbelong toConfigandImplementedPrivateModes, andthat rule targets static fields, which this never was.
Verification
Cherry-picked onto
upstream/work@3e48b183(original branch was five merges back;applied clean, no conflicts). Clean build in a fresh worktree (no prior build directory):
./gradlew clean build— BUILD SUCCESSFUL:test: 284 tests, 0 failures, 0 skipped5 files, +12/−12.
Assisted by: GLM (syn:large:text on synthetic.new) — code generation and review.