No task waits on a Codex review; the redesign tasks are confirmed on the product owner's Z Fold7; TASK-016's leftovers are on hold; no biometric reauthentication. Agent: main (opus) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
138 lines
9.3 KiB
Markdown
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 -- no Codex review pending (Codex retired, 2026-09-29).
|