Ubisam-MobileGit/tasks/048-code-viewer-readability.md
choiminyeong d9536289e9 TASK-048: source viewer line numbers, syntax highlighting, word wrap
Includes the two same-day follow-ups that fixed horizontal scrolling. See tasks/048-code-viewer-readability.md.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
2026-09-28 13:17:33 +09:00

138 lines
9.3 KiB
Markdown

# TASK-048: Source code viewer readability (line numbers + syntax highlighting)
Status: implemented; all required checks pass; live-confirmed on the human product owner's device,
including a horizontal-scroll regression found and fixed during that same live testing
Owner: Claude
Reviewer: Codex
## Outcome
The human product owner reported the in-app source file viewer was hard to read -- no line numbers,
no syntax highlighting, everything rendered as one plain block of monospace text with only
horizontal/vertical scroll. Chose (from three offered options) the fullest one: line numbers plus
color syntax highlighting. Both, plus a word-wrap toggle, are now live -- reached in three
iterations after the first two shipped real regressions that live device testing caught (see Design
notes' numbered account).
## Scope
- Included: new `feature/codebrowser/content/SyntaxHighlighter.kt` (a lightweight, regex-based,
per-line highlighter -- keywords/strings/comments/numbers -- for Kotlin, Java, C#, JavaScript/TS,
Python, Shell, YAML, XML, JSON, CSS, SQL, with a plain-text fallback for anything else); a new
`CodeBlock` composable in `CodeBrowserScreen.kt` replacing the single giant `Text` for
`ContentClassification.Text` with line-numbered, highlighted rows; a 줄바꿈 켜기/끄기 (word wrap
on/off) toggle button next to the existing copy button.
- Excluded: a real per-language parser/tokenizer (see Design notes on why a regex approach was
chosen instead); highlighting for languages outside the list above (falls back to a plain,
uncolored monospace line -- still gets line numbers and wrap, just no color).
## Design notes
- **Regex-based, not a real tokenizer, and explicitly per-line with no cross-line state.** A block
comment (`/* ... */`) or a string that spans multiple lines is only colored correctly on the
line(s) where it actually starts/ends, not on every line in between. A real multi-language parser
would be a much larger undertaking than a file *viewer* needs; this covers the overwhelmingly
common single-line case cheaply, in pure Kotlin, with zero new dependencies -- consistent with
this codebase's established preference (see the launcher icon's and TASK-047's own notes) for
hand-rolled solutions over pulling in a library for one narrow need.
- **Colors are fixed, not derived from the app's own sky-blue theme.** Source highlighting has its
own well-known conventions (blue keywords, orange/red strings, green comments) independent of any
given app's brand color; two hand-picked sets (light/dark, closely following VS Code's own
defaults) are used instead, switching on `isSystemInDarkTheme()`.
- **Word wrap defaults off** (matches the previous behavior exactly, so nothing changes unless the
user opts in), toggled per file-view via a plain `OutlinedButton` (no icon dependency, matching
every other button in this screen).
- **The row layout went through three shapes, in order, each caught by live on-device testing, not
by build/lint/unit tests, none of which can see a real touch gesture or a real wrapped-text
layout:**
1. *LazyColumn, one Row per line, each row's code `Text` on its own
`Modifier.horizontalScroll(sharedState)`.* Matches typical modern Compose guidance for a
virtualized, line-numbered list. Live-tested: horizontal drags did nothing at all. Most likely
cause: `BranchDetailScreen`'s `HorizontalPager` (Files/History tabs) turns off
`userScrollEnabled` for scrollable code, but apparently doesn't fully exit the nested-scroll
picture for a `LazyColumn` specifically the way it does for a plain `Modifier.horizontalScroll`
chain -- see that file's own pre-existing comment on this exact class of gesture conflict,
written for the previous (pre-TASK-048) single-`Text` implementation.
2. *Two independent Columns side by side (gutter, code) inside one outer `Row` plus
`Modifier.verticalScroll`,* moving off `LazyColumn` entirely to sidestep (1). Fixed horizontal
scroll. Live-tested with word wrap on: line numbers drifted out of sync with their code as soon
as any line wrapped to multiple visual rows -- the gutter Column (one fixed-height `Text` per
logical line) has no way to grow to match a code Column line that just became taller.
3. **Final:** one outer `Column` plus `Modifier.verticalScroll`, gutter and code `Text` paired back
into one `Row` per logical line (closer to shape 1 again, minus `LazyColumn`), each row's code
`Text` on its own independent `Modifier.horizontalScroll` (no longer a shared `ScrollState`
across rows -- an acceptable, visually invisible trade-off, since every row's text always
starts at column 0 regardless). Live-confirmed: horizontal scroll works, and word-wrap
alignment is correct, because Compose sizes each Row to fit its own (possibly multi-row-tall)
content and the single gutter number simply sits at that row's top.
Not lazy (every line is a real composable, not virtualized) in any of the three shapes -- the
original single-`Text` version wasn't virtualized either, and `SourceContentClassifier` already
bounds how many lines a file view ever receives, so this was never a new performance ceiling, only
a correctness one.
## Acceptance criteria
- [x] Every source text view shows a line-number gutter.
- [x] Kotlin/Java/C#/JavaScript-TS/Python/Shell/YAML/XML/JSON/CSS/SQL keywords, strings, comments,
and numbers render in distinct colors; an unrecognized extension still renders (plain, with
line numbers), never crashes or shows nothing.
- [x] A 줄바꿈 켜기/끄기 toggle switches between horizontal-scroll and wrapped rendering.
- [x] With word wrap off, a line longer than the viewport is horizontally scrollable by dragging it.
- [x] With word wrap on, every line's gutter number stays aligned with its own (possibly
multi-row-tall) wrapped content.
- [x] Build, unit tests, and lint pass.
- [x] Live-confirmed on the human product owner's device against real C# and XML/`.csproj` files:
keyword/string/comment coloring, line numbers, horizontal drag-scroll (off), and wrap alignment
(on) all independently verified after the shape-3 fix.
## Required checks
```text
./gradlew compileDebugKotlin compileDebugUnitTestKotlin testDebugUnitTest assembleDebug lintDebug
```
## Evidence
- New `feature/codebrowser/content/SyntaxHighlighter.kt`: `SyntaxLanguage` enum,
`syntaxLanguageForPath` (extension-based), `CodeSyntaxColors` + `rememberCodeSyntaxColors`,
`highlightLine` (regex tokenize + classify), per-language `LangSpec` keyword/comment tables.
- `feature/codebrowser/CodeBrowserScreen.kt`: new `CodeBlock` composable (final shape: outer
`Column` + `verticalScroll`, one `Row` per line pairing gutter + code `Text`, code `Text` getting
its own `horizontalScroll` when word wrap is off); `ContentClassification.Text` branch rewired to
it; new `wordWrap` state + toggle button.
- `res/values/strings_codebrowser.xml`: `codebrowser_word_wrap_on`/`_off`.
- `./gradlew compileDebugKotlin compileDebugUnitTestKotlin testDebugUnitTest assembleDebug lintDebug`:
BUILD SUCCESSFUL, all unit tests passing (no behavior touched outside the file viewer), 0 lint
errors.
- Live device: installed over wireless `adb` three times (once per shape above) as each round of
testing surfaced the next issue. Final round: opened a real `.cs` file (`KeyenceBCRConfig.cs`) and
a real `.csproj` (XML) file in `RobotAutomationDevelopment-Dept/LGIT-LaserCompressionBonding`;
screenshot-confirmed keyword coloring, string coloring, line numbers, a horizontal swipe actually
moving a long line's content, and (with word wrap on) multi-row-wrapped lines keeping
correctly-aligned single gutter numbers.
- Diagnosed the shape-1 failure via `adb shell uiautomator dump` (confirmed the accessibility tree
exposed only the top-level scroll container as `scrollable="true"`, not a per-row one -- suggestive
though not itself conclusive) and, more directly, by reasoning from `BranchDetailScreen.kt`'s own
pre-existing comment describing the identical Pager-vs-content gesture conflict for the previous
implementation.
## Known limitations
- No cross-line tokenizer state (see Design notes) -- multi-line block comments/strings only color
correctly at their start/end line.
- Highlighting rules are hand-picked keyword sets per language, not sourced from each language's
full official grammar -- covers common keywords, not every contextual one.
- Line-number gutter width is a character-count heuristic (`lineNumberWidth * 8 + 12` dp), not a
measured intrinsic width -- close enough visually, not pixel-exact across every font-scale setting.
- The final layout (see two follow-up dev-log entries the same day) branches by word-wrap mode: word
wrap off uses two side-by-side Columns (gutter, code) sharing one horizontalScroll region so every
line scrolls together; word wrap on pairs gutter+code per Row so number alignment survives wrapped
lines. The two-Column shape is only safe because word-wrap-off guarantees every line is exactly one
visual row tall -- if that assumption ever stopped holding (e.g. a future change rendering a line
as more than one row while word wrap is off), gutter/code alignment would silently break again the
same way it did in the abandoned two-Column-always attempt.
## Review findings
None yet -- pending Codex/human review.