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>
This commit is contained in:
parent
fccb817381
commit
d9536289e9
@ -75,13 +75,21 @@ import androidx.compose.ui.text.font.FontFamily
|
||||
import androidx.compose.ui.text.font.FontWeight
|
||||
import androidx.compose.ui.unit.IntOffset
|
||||
import androidx.compose.ui.unit.IntSize
|
||||
import androidx.compose.ui.text.style.TextAlign
|
||||
import androidx.compose.ui.unit.Dp
|
||||
import androidx.compose.ui.unit.dp
|
||||
import androidx.compose.ui.unit.sp
|
||||
import com.ubisam.ubgitea.R
|
||||
import com.ubisam.ubgitea.domain.model.RepositoryPath
|
||||
import com.ubisam.ubgitea.domain.model.SourceContent
|
||||
import com.ubisam.ubgitea.domain.model.TreeEntry
|
||||
import com.ubisam.ubgitea.domain.model.TreeEntryType
|
||||
import com.ubisam.ubgitea.feature.codebrowser.content.CodeSyntaxColors
|
||||
import com.ubisam.ubgitea.feature.codebrowser.content.ContentClassification
|
||||
import com.ubisam.ubgitea.feature.codebrowser.content.SyntaxLanguage
|
||||
import com.ubisam.ubgitea.feature.codebrowser.content.highlightLine
|
||||
import com.ubisam.ubgitea.feature.codebrowser.content.rememberCodeSyntaxColors
|
||||
import com.ubisam.ubgitea.feature.codebrowser.content.syntaxLanguageForPath
|
||||
import java.io.File
|
||||
import java.io.IOException
|
||||
import kotlin.math.roundToInt
|
||||
@ -217,21 +225,27 @@ private fun FileContent(
|
||||
when (content) {
|
||||
is ContentClassification.Text -> {
|
||||
val clipboard = LocalClipboardManager.current
|
||||
var wordWrap by remember(path) { mutableStateOf(false) }
|
||||
Column(Modifier.fillMaxSize().padding(16.dp)) {
|
||||
Row(horizontalArrangement = Arrangement.spacedBy(12.dp), verticalAlignment = Alignment.CenterVertically) {
|
||||
OutlinedButton(onClick = { clipboard.setText(AnnotatedString(content.renderedText)) }) {
|
||||
Text(stringResource(R.string.codebrowser_copy))
|
||||
}
|
||||
OutlinedButton(onClick = { wordWrap = !wordWrap }) {
|
||||
Text(
|
||||
stringResource(
|
||||
if (wordWrap) R.string.codebrowser_word_wrap_off else R.string.codebrowser_word_wrap_on,
|
||||
),
|
||||
)
|
||||
}
|
||||
if (content.isTruncated) Text(stringResource(R.string.codebrowser_text_truncated))
|
||||
}
|
||||
SelectionContainer {
|
||||
Text(
|
||||
content.renderedText,
|
||||
fontFamily = FontFamily.Monospace,
|
||||
modifier = Modifier.fillMaxSize().verticalScroll(rememberScrollState())
|
||||
.horizontalScroll(rememberScrollState()).padding(top = 12.dp),
|
||||
)
|
||||
}
|
||||
CodeBlock(
|
||||
text = content.renderedText,
|
||||
language = syntaxLanguageForPath(path),
|
||||
wordWrap = wordWrap,
|
||||
modifier = Modifier.fillMaxSize().padding(top = 12.dp),
|
||||
)
|
||||
}
|
||||
}
|
||||
is ContentClassification.Image -> {
|
||||
@ -250,6 +264,102 @@ private fun FileContent(
|
||||
}
|
||||
}
|
||||
|
||||
// Line-numbered, syntax-highlighted source view (see SyntaxHighlighter.kt for what the highlighting
|
||||
// actually is/isn't).
|
||||
//
|
||||
// Four things this shape specifically works around, all found live on-device -- none of them
|
||||
// visible from build/lint/unit tests, which can't see a real touch gesture or a real wrapped-text
|
||||
// layout:
|
||||
// 1. An earlier version paired gutter+code in a LazyColumn with one horizontalScroll per row.
|
||||
// Horizontal drags simply did nothing -- most likely BranchDetailScreen's HorizontalPager
|
||||
// (userScrollEnabled is turned off for scrollable code, but apparently still not fully out of
|
||||
// the nested-scroll picture for a LazyColumn specifically, the way it is for a plain
|
||||
// Modifier.horizontalScroll chain -- see that file's own comment on this exact class of gesture
|
||||
// conflict) intercepting the same-orientation drag first. Switched the outer container from
|
||||
// LazyColumn to a plain Column + Modifier.verticalScroll, matching the shape of the file
|
||||
// viewer's original, already-known-working implementation -- confirmed live afterward that
|
||||
// horizontal drag actually moves the code again.
|
||||
// 2. A version in between paired every row's gutter+code into a single Row (fix for #3 below) but
|
||||
// gave each row's code Text its own independent Modifier.horizontalScroll -- reasoned at the
|
||||
// time to be visually invisible since every row's text starts at column 0. Wrong in practice:
|
||||
// dragging one line only scrolled that one line, leaving the rest of the file visually
|
||||
// unscrolled and out of step, which reads as broken (a real product report: "선택한 곳만 좌우
|
||||
// 스크롤 되는건 아닌 것 같다").
|
||||
// 3. Tried making every row share one hoisted horizontalScrollState instead. That made horizontal
|
||||
// drag do *nothing at all*, on any row, including the one being dragged -- multiple separate
|
||||
// Modifier.horizontalScroll gesture-detector regions apparently don't cleanly share one
|
||||
// ScrollableState the way a single shared scroll *region* does; each row still had its own
|
||||
// pointer-input node competing over the same state instead of there being one region to drag.
|
||||
// 4. **Final:** don't try to synchronize N separate horizontalScroll regions at all -- give the
|
||||
// *whole* code column exactly one horizontalScroll region, wrapping every line's Text stacked in
|
||||
// a single Column, with the gutter as a separate sibling Column that never scrolls horizontally.
|
||||
// This only works correctly (gutter height staying matched to code height) as long as no line
|
||||
// ever wraps to multiple visual rows, which is exactly true whenever this two-Column layout is
|
||||
// even in play: it's used only when word wrap is off (softWrap = false, so every line is exactly
|
||||
// one visual row no matter how long). When word wrap is on, wrapping can make a line take
|
||||
// multiple visual rows, so gutter+code go back to the Row-per-line pairing from fix 2 instead
|
||||
// (needed for correct number alignment, and horizontalScroll isn't used in this branch anyway --
|
||||
// wrapped lines have nothing to scroll to).
|
||||
//
|
||||
// Not lazy (every line is a real composable, not virtualized) -- the original single-Text version
|
||||
// wasn't virtualized either, and SourceContentClassifier already bounds how many lines a file view
|
||||
// ever receives, so this isn't a new performance ceiling.
|
||||
@Composable
|
||||
private fun CodeBlock(text: String, language: SyntaxLanguage, wordWrap: Boolean, modifier: Modifier = Modifier) {
|
||||
val colors = rememberCodeSyntaxColors()
|
||||
val lines = remember(text) { text.split("\n") }
|
||||
val lineNumberWidth = remember(lines.size) { lines.size.toString().length }
|
||||
val gutterWidth = (lineNumberWidth * 8 + 12).dp
|
||||
SelectionContainer(modifier = modifier) {
|
||||
if (wordWrap) {
|
||||
Column(Modifier.fillMaxSize().verticalScroll(rememberScrollState())) {
|
||||
lines.forEachIndexed { index, line ->
|
||||
Row(Modifier.fillMaxWidth()) {
|
||||
LineNumber(index, gutterWidth, colors)
|
||||
Text(
|
||||
text = highlightLine(line, language, colors),
|
||||
fontFamily = FontFamily.Monospace,
|
||||
fontSize = 13.sp,
|
||||
color = colors.text,
|
||||
softWrap = true,
|
||||
modifier = Modifier.weight(1f),
|
||||
)
|
||||
}
|
||||
}
|
||||
}
|
||||
} else {
|
||||
Row(Modifier.fillMaxSize().verticalScroll(rememberScrollState())) {
|
||||
Column {
|
||||
lines.indices.forEach { index -> LineNumber(index, gutterWidth, colors) }
|
||||
}
|
||||
Column(Modifier.horizontalScroll(rememberScrollState())) {
|
||||
lines.forEach { line ->
|
||||
Text(
|
||||
text = highlightLine(line, language, colors),
|
||||
fontFamily = FontFamily.Monospace,
|
||||
fontSize = 13.sp,
|
||||
color = colors.text,
|
||||
softWrap = false,
|
||||
)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@Composable
|
||||
private fun LineNumber(index: Int, width: Dp, colors: CodeSyntaxColors) {
|
||||
Text(
|
||||
text = (index + 1).toString(),
|
||||
fontFamily = FontFamily.Monospace,
|
||||
fontSize = 13.sp,
|
||||
color = colors.comment,
|
||||
textAlign = TextAlign.End,
|
||||
modifier = Modifier.width(width).padding(end = 8.dp),
|
||||
)
|
||||
}
|
||||
|
||||
// currentIndex is -1 when this file isn't part of a sibling image gallery (e.g. opened directly
|
||||
// via a retry rather than from a directory listing) — swipe navigation is simply unavailable then,
|
||||
// same as any single-image folder (totalCount <= 1).
|
||||
|
||||
@ -0,0 +1,237 @@
|
||||
package com.ubisam.ubgitea.feature.codebrowser.content
|
||||
|
||||
import androidx.compose.foundation.isSystemInDarkTheme
|
||||
import androidx.compose.runtime.Composable
|
||||
import androidx.compose.runtime.remember
|
||||
import androidx.compose.ui.graphics.Color
|
||||
import androidx.compose.ui.text.AnnotatedString
|
||||
import androidx.compose.ui.text.SpanStyle
|
||||
import androidx.compose.ui.text.buildAnnotatedString
|
||||
import androidx.compose.ui.text.font.FontWeight
|
||||
import androidx.compose.ui.text.withStyle
|
||||
import com.ubisam.ubgitea.domain.model.RepositoryPath
|
||||
|
||||
/**
|
||||
* A deliberately lightweight, regex-based highlighter -- not a real per-language parser. Each line
|
||||
* is tokenized independently with no cross-line state, so a `/* ... */` block comment or a
|
||||
* triple-quoted string that spans multiple lines is only colored correctly on the line(s) where it
|
||||
* actually starts and ends, not on every line in between. A real tokenizer for every language this
|
||||
* app might show source from would be a much larger undertaking than this file viewer needs; this
|
||||
* covers the common, single-line case (which is the vast majority of real code) at a fraction of the
|
||||
* cost, matching this codebase's established preference for hand-rolled solutions over new
|
||||
* dependencies (see the launcher icon's and the favorite-push logo's own comments on the same
|
||||
* point).
|
||||
*/
|
||||
enum class SyntaxLanguage {
|
||||
KOTLIN, JAVA, CSHARP, JAVASCRIPT, PYTHON, SHELL, YAML, XML, JSON, CSS, SQL, PLAIN
|
||||
}
|
||||
|
||||
fun syntaxLanguageForPath(path: RepositoryPath): SyntaxLanguage {
|
||||
val extension = path.value.substringAfterLast('.', missingDelimiterValue = "").lowercase()
|
||||
return when (extension) {
|
||||
"kt", "kts" -> SyntaxLanguage.KOTLIN
|
||||
"java" -> SyntaxLanguage.JAVA
|
||||
"cs" -> SyntaxLanguage.CSHARP
|
||||
"js", "jsx", "mjs", "ts", "tsx" -> SyntaxLanguage.JAVASCRIPT
|
||||
"py" -> SyntaxLanguage.PYTHON
|
||||
"sh", "bash", "zsh" -> SyntaxLanguage.SHELL
|
||||
"yml", "yaml" -> SyntaxLanguage.YAML
|
||||
"xml", "xaml", "csproj", "resx", "html", "htm", "axml" -> SyntaxLanguage.XML
|
||||
"json" -> SyntaxLanguage.JSON
|
||||
"css", "scss" -> SyntaxLanguage.CSS
|
||||
"sql" -> SyntaxLanguage.SQL
|
||||
else -> SyntaxLanguage.PLAIN
|
||||
}
|
||||
}
|
||||
|
||||
data class CodeSyntaxColors(
|
||||
val text: Color,
|
||||
val keyword: Color,
|
||||
val string: Color,
|
||||
val comment: Color,
|
||||
val number: Color,
|
||||
)
|
||||
|
||||
// Colors are fixed (not derived from the app's own sky-blue Material theme) and only switch
|
||||
// between two hand-picked sets for light/dark -- source code highlighting has its own well-known
|
||||
// conventions (keyword blue, string orange/red, comment green) independent of a given app's brand
|
||||
// color, and reusing brand colors here would read as less familiar, not more polished.
|
||||
@Composable
|
||||
fun rememberCodeSyntaxColors(): CodeSyntaxColors {
|
||||
val dark = isSystemInDarkTheme()
|
||||
return remember(dark) {
|
||||
if (dark) {
|
||||
CodeSyntaxColors(
|
||||
text = Color(0xFFD4D4D4),
|
||||
keyword = Color(0xFF569CD6),
|
||||
string = Color(0xFFCE9178),
|
||||
comment = Color(0xFF6A9955),
|
||||
number = Color(0xFFB5CEA8),
|
||||
)
|
||||
} else {
|
||||
CodeSyntaxColors(
|
||||
text = Color(0xFF1F1F1F),
|
||||
keyword = Color(0xFF0000FF),
|
||||
string = Color(0xFFA31515),
|
||||
comment = Color(0xFF008000),
|
||||
number = Color(0xFF098658),
|
||||
)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
private data class LangSpec(
|
||||
val keywords: Set<String> = emptySet(),
|
||||
val caseInsensitiveKeywords: Boolean = false,
|
||||
val lineComment: String? = null,
|
||||
val blockComment: Boolean = false,
|
||||
val xmlComment: Boolean = false,
|
||||
)
|
||||
|
||||
private val SPECS: Map<SyntaxLanguage, LangSpec> = mapOf(
|
||||
SyntaxLanguage.KOTLIN to LangSpec(
|
||||
keywords = setOf(
|
||||
"package", "import", "class", "interface", "object", "fun", "val", "var", "if", "else",
|
||||
"when", "for", "while", "do", "return", "is", "as", "in", "out", "null", "true", "false",
|
||||
"this", "super", "try", "catch", "finally", "throw", "private", "public", "internal",
|
||||
"protected", "companion", "data", "sealed", "enum", "open", "override", "abstract",
|
||||
"final", "suspend", "inline", "reified", "vararg", "lateinit", "const", "typealias",
|
||||
"by", "init", "constructor", "get", "set", "annotation", "inner", "operator", "infix",
|
||||
"tailrec", "crossinline", "noinline", "expect", "actual",
|
||||
),
|
||||
lineComment = "//",
|
||||
blockComment = true,
|
||||
),
|
||||
SyntaxLanguage.JAVA to LangSpec(
|
||||
keywords = setOf(
|
||||
"package", "import", "class", "interface", "extends", "implements", "public", "private",
|
||||
"protected", "static", "final", "abstract", "void", "int", "long", "short", "byte",
|
||||
"char", "boolean", "float", "double", "new", "return", "if", "else", "switch", "case",
|
||||
"default", "for", "while", "do", "break", "continue", "try", "catch", "finally", "throw",
|
||||
"throws", "this", "super", "null", "true", "false", "instanceof", "synchronized",
|
||||
"volatile", "transient", "enum", "assert",
|
||||
),
|
||||
lineComment = "//",
|
||||
blockComment = true,
|
||||
),
|
||||
SyntaxLanguage.CSHARP to LangSpec(
|
||||
keywords = setOf(
|
||||
"using", "namespace", "class", "interface", "struct", "enum", "public", "private",
|
||||
"protected", "internal", "static", "readonly", "const", "void", "int", "long", "short",
|
||||
"byte", "char", "bool", "float", "double", "decimal", "string", "object", "var", "new",
|
||||
"return", "if", "else", "switch", "case", "default", "for", "foreach", "while", "do",
|
||||
"break", "continue", "try", "catch", "finally", "throw", "this", "base", "null", "true",
|
||||
"false", "override", "virtual", "abstract", "sealed", "partial", "async", "await", "get",
|
||||
"set", "out", "ref", "in", "is", "as", "typeof", "nameof", "delegate", "event", "yield",
|
||||
"lock", "unsafe", "fixed", "params",
|
||||
),
|
||||
lineComment = "//",
|
||||
blockComment = true,
|
||||
),
|
||||
SyntaxLanguage.JAVASCRIPT to LangSpec(
|
||||
keywords = setOf(
|
||||
"function", "const", "let", "var", "if", "else", "for", "while", "do", "switch", "case",
|
||||
"default", "break", "continue", "return", "class", "extends", "super", "this", "new",
|
||||
"try", "catch", "finally", "throw", "import", "export", "from", "async", "await",
|
||||
"yield", "typeof", "instanceof", "in", "of", "null", "undefined", "true", "false",
|
||||
"interface", "type", "implements", "public", "private", "protected", "static",
|
||||
"readonly", "enum", "namespace", "declare",
|
||||
),
|
||||
lineComment = "//",
|
||||
blockComment = true,
|
||||
),
|
||||
SyntaxLanguage.PYTHON to LangSpec(
|
||||
keywords = setOf(
|
||||
"def", "class", "import", "from", "as", "if", "elif", "else", "for", "while", "break",
|
||||
"continue", "return", "yield", "try", "except", "finally", "raise", "with", "lambda",
|
||||
"pass", "del", "global", "nonlocal", "and", "or", "not", "in", "is", "None", "True",
|
||||
"False", "self", "async", "await", "assert",
|
||||
),
|
||||
lineComment = "#",
|
||||
),
|
||||
SyntaxLanguage.SHELL to LangSpec(
|
||||
keywords = setOf(
|
||||
"if", "then", "elif", "else", "fi", "for", "while", "until", "do", "done", "case",
|
||||
"esac", "function", "return", "exit", "break", "continue", "local", "export", "readonly",
|
||||
"in", "select", "time",
|
||||
),
|
||||
lineComment = "#",
|
||||
),
|
||||
SyntaxLanguage.YAML to LangSpec(
|
||||
keywords = setOf("true", "false", "null", "yes", "no"),
|
||||
lineComment = "#",
|
||||
),
|
||||
SyntaxLanguage.XML to LangSpec(xmlComment = true),
|
||||
SyntaxLanguage.JSON to LangSpec(keywords = setOf("true", "false", "null")),
|
||||
SyntaxLanguage.CSS to LangSpec(blockComment = true),
|
||||
SyntaxLanguage.SQL to LangSpec(
|
||||
keywords = setOf(
|
||||
"select", "from", "where", "insert", "into", "values", "update", "set", "delete",
|
||||
"join", "inner", "left", "right", "outer", "on", "group", "by", "order", "having",
|
||||
"limit", "as", "and", "or", "not", "null", "is", "in", "like", "between", "create",
|
||||
"table", "alter", "drop", "index", "primary", "key", "foreign", "references", "default",
|
||||
"distinct", "union", "all", "exists", "case", "when", "then", "else", "end",
|
||||
),
|
||||
caseInsensitiveKeywords = true,
|
||||
lineComment = "--",
|
||||
),
|
||||
SyntaxLanguage.PLAIN to LangSpec(),
|
||||
)
|
||||
|
||||
private val STRING_OR_NUMBER_OR_WORD =
|
||||
"\"(?:[^\"\\\\]|\\\\.)*\"" + "|" +
|
||||
"'(?:[^'\\\\]|\\\\.)*'" + "|" +
|
||||
"\\b\\d+(?:\\.\\d+)?[a-zA-Z]?\\b" + "|" +
|
||||
"\\b[A-Za-z_][A-Za-z0-9_]*\\b"
|
||||
|
||||
private val patternCache: Map<SyntaxLanguage, Regex> = SPECS.mapValues { (_, spec) ->
|
||||
val parts = mutableListOf<String>()
|
||||
if (spec.blockComment) parts += "/\\*.*?\\*/|/\\*.*$"
|
||||
if (spec.xmlComment) parts += "<!--.*?-->|<!--.*$"
|
||||
spec.lineComment?.let { parts += "${Regex.escape(it)}.*$" }
|
||||
parts += STRING_OR_NUMBER_OR_WORD
|
||||
Regex(parts.joinToString("|"))
|
||||
}
|
||||
|
||||
fun highlightLine(line: String, language: SyntaxLanguage, colors: CodeSyntaxColors): AnnotatedString {
|
||||
val spec = SPECS.getValue(language)
|
||||
val pattern = patternCache.getValue(language)
|
||||
return buildAnnotatedString {
|
||||
var lastIndex = 0
|
||||
for (match in pattern.findAll(line)) {
|
||||
if (match.range.first > lastIndex) append(line.substring(lastIndex, match.range.first))
|
||||
val text = match.value
|
||||
val style = classify(text, spec)?.let { role -> SpanStyle(color = role.colorOf(colors), fontWeight = role.weight) }
|
||||
if (style != null) withStyle(style) { append(text) } else append(text)
|
||||
lastIndex = match.range.last + 1
|
||||
}
|
||||
if (lastIndex < line.length) append(line.substring(lastIndex))
|
||||
}
|
||||
}
|
||||
|
||||
private enum class TokenRole(val weight: FontWeight?) {
|
||||
COMMENT(null),
|
||||
STRING(null),
|
||||
NUMBER(null),
|
||||
KEYWORD(FontWeight.SemiBold),
|
||||
;
|
||||
|
||||
fun colorOf(colors: CodeSyntaxColors): Color = when (this) {
|
||||
COMMENT -> colors.comment
|
||||
STRING -> colors.string
|
||||
NUMBER -> colors.number
|
||||
KEYWORD -> colors.keyword
|
||||
}
|
||||
}
|
||||
|
||||
private fun classify(text: String, spec: LangSpec): TokenRole? = when {
|
||||
text.startsWith("/*") || (spec.lineComment != null && text.startsWith(spec.lineComment)) -> TokenRole.COMMENT
|
||||
spec.xmlComment && text.startsWith("<!--") -> TokenRole.COMMENT
|
||||
text.startsWith("\"") || text.startsWith("'") -> TokenRole.STRING
|
||||
text.first().isDigit() -> TokenRole.NUMBER
|
||||
isKeyword(text, spec) -> TokenRole.KEYWORD
|
||||
else -> null
|
||||
}
|
||||
|
||||
private fun isKeyword(text: String, spec: LangSpec): Boolean =
|
||||
if (spec.caseInsensitiveKeywords) text.lowercase() in spec.keywords else text in spec.keywords
|
||||
@ -13,6 +13,8 @@
|
||||
<string name="codebrowser_submodule">서브모듈 내용은 이 앱에서 직접 열 수 없습니다.</string>
|
||||
<string name="codebrowser_symlink">심볼릭 링크 대상은 이 앱에서 직접 열 수 없습니다.</string>
|
||||
<string name="codebrowser_copy">코드 복사</string>
|
||||
<string name="codebrowser_word_wrap_on">줄바꿈 켜기</string>
|
||||
<string name="codebrowser_word_wrap_off">줄바꿈 끄기</string>
|
||||
<string name="codebrowser_text_truncated">안전한 표시를 위해 일부만 보여줍니다.</string>
|
||||
<string name="codebrowser_binary">바이너리 파일이어서 텍스트로 표시하지 않습니다. (%1$d bytes)</string>
|
||||
<string name="codebrowser_unsupported_encoding">UTF-8로 안전하게 읽을 수 없는 파일입니다. (%1$d bytes)</string>
|
||||
|
||||
@ -1156,3 +1156,116 @@ This is the append-only implementation and handoff record shared by Codex and Cl
|
||||
- Next owner: Claude, continuing per the standing direction; human product owner for the item-3
|
||||
architecture decision.
|
||||
|
||||
## 2026-09-23 — TASK-048: source code viewer readability (line numbers + syntax highlighting)
|
||||
|
||||
- Agent and role: Claude, feature implementer, per the human product owner's direct complaint that
|
||||
the in-app source file viewer was hard to read ("가독성이 떨어져서 파일 내부를 볼 때 이해하기 쉽게
|
||||
표기 할 수 있는 방안이 있나?"); offered three levels of fix and the product owner chose the fullest
|
||||
one (line numbers + color syntax highlighting) over line-numbers-only or a from-scratch
|
||||
explanation.
|
||||
- Commit/diff: same working tree as prior recent tasks (main checkout, branch
|
||||
`claude/task-015-notifications-main`), not yet committed; new
|
||||
`tasks/048-code-viewer-readability.md`.
|
||||
- Features implemented: every source text view now shows a line-number gutter and color syntax
|
||||
highlighting (keywords/strings/comments/numbers) for Kotlin, Java, C#, JavaScript/TypeScript,
|
||||
Python, Shell, YAML, XML, JSON, CSS, and SQL, plus a 줄바꿈 켜기/끄기 (word wrap) toggle.
|
||||
- How it was implemented: new `feature/codebrowser/content/SyntaxHighlighter.kt` -- a deliberately
|
||||
lightweight, regex-based, per-line tokenizer (no cross-line state, so a block comment or string
|
||||
spanning multiple lines is only colored correctly at its start/end line) rather than a real parser,
|
||||
matching this codebase's established preference for hand-rolled solutions over new dependencies.
|
||||
`CodeBrowserScreen.kt`'s single giant `Text` for text files was replaced with a new `CodeBlock`
|
||||
composable: one `Text` per line inside a `LazyColumn` (so only visible lines are composed for
|
||||
longer files), a right-aligned line-number gutter, and -- when word wrap is off -- every line's
|
||||
`Text` sharing one `ScrollState` so dragging any line scrolls all of them in sync. Colors are fixed
|
||||
(VS Code-like light/dark sets), not derived from the app's own sky-blue theme, since source
|
||||
highlighting has its own independent, well-known conventions.
|
||||
- Files changed: new `feature/codebrowser/content/SyntaxHighlighter.kt`,
|
||||
`feature/codebrowser/CodeBrowserScreen.kt`, `res/values/strings_codebrowser.xml`,
|
||||
`tasks/048-code-viewer-readability.md`.
|
||||
- Checks and results: `./gradlew compileDebugKotlin compileDebugUnitTestKotlin testDebugUnitTest assembleDebug lintDebug`
|
||||
BUILD SUCCESSFUL, all unit tests passing unmodified (no behavior touched outside the file viewer),
|
||||
0 lint errors.
|
||||
- Live verification: installed over wireless `adb`; opened a real C# file
|
||||
(`AuroraDevices/AuroraBCR/KeyenceBCR/KeyenceBCRConfig.cs` in
|
||||
`RobotAutomationDevelopment-Dept/LGIT-LaserCompressionBonding`); screenshot-confirmed keyword
|
||||
coloring (`using`, `namespace`, `public`, `class`, `get`, `set`, `new`, `int`, `string`), string
|
||||
coloring (`"127.0.0.1"`), and line numbers, in both the default horizontal-scroll mode and after
|
||||
toggling word wrap on.
|
||||
- Review findings and resolutions: None from a separate review pass yet.
|
||||
- Known limitations: no cross-line tokenizer state (see task file); keyword sets are hand-picked, not
|
||||
each language's full grammar; the line-number gutter's width is a character-count heuristic, not a
|
||||
measured intrinsic width.
|
||||
- Next recommended task: none queued from this request. TASK-045's still-open items (device
|
||||
confirmation of the visual redesign specifically, and the real logo asset) remain open from
|
||||
earlier.
|
||||
- Next owner: Claude, continuing per the standing direction.
|
||||
|
||||
## 2026-09-23 (continued) — TASK-048 follow-up: horizontal scroll regression found and fixed live
|
||||
|
||||
- Agent and role: Claude, incident responder -- the human product owner tested TASK-048 themselves
|
||||
and reported word wrap being off worked, but horizontal drag-scroll did not ("자동 줄바꿈 끄니까
|
||||
쭉 한 행에 써지는건 좋은데 좌우 스크롤은 되어야하지 않나").
|
||||
- Root cause and fix: TASK-048's first shipped shape (LazyColumn, one horizontalScroll per row) had
|
||||
horizontal drags silently doing nothing on-device, most likely due to BranchDetailScreen's
|
||||
HorizontalPager not being fully out of the nested-scroll picture for a LazyColumn specifically
|
||||
(confirmed via `adb shell uiautomator dump`, showing only the top-level container exposed as
|
||||
scrollable, plus reasoning from BranchDetailScreen.kt's own pre-existing comment about this exact
|
||||
gesture-conflict class). Rewrote CodeBlock to drop LazyColumn for a plain Column +
|
||||
Modifier.verticalScroll (matching the file viewer's original, already-working shape) -- confirmed
|
||||
scroll worked, but this intermediate version (two independent Columns for gutter and code)
|
||||
introduced a second regression: word-wrap line numbers drifted out of sync with wrapped code,
|
||||
since the two Columns' heights could no longer track each other once any line wrapped. Final fix:
|
||||
gutter and code `Text` paired back into one `Row` per logical line inside the outer
|
||||
Column+verticalScroll -- correct in both modes, confirmed live against a real `.cs` file (drag
|
||||
scroll) and a real `.csproj`/XML file with long attribute lines (wrap alignment).
|
||||
- Files changed: `feature/codebrowser/CodeBrowserScreen.kt` (CodeBlock rewritten twice more),
|
||||
`tasks/048-code-viewer-readability.md` (rewritten to record all three shapes and why each of the
|
||||
first two failed).
|
||||
- Checks and results: `./gradlew compileDebugKotlin compileDebugUnitTestKotlin testDebugUnitTest assembleDebug lintDebug`
|
||||
BUILD SUCCESSFUL after the final shape, 0 lint errors, all unit tests passing unmodified.
|
||||
- Live verification: installed over wireless `adb` after each of the two follow-up shapes; final
|
||||
round screenshot-confirmed a horizontal swipe moving a long XML attribute line's content, and (with
|
||||
word wrap toggled on) multi-row-wrapped lines keeping their single gutter number correctly aligned
|
||||
at the top of that line's row.
|
||||
- Review findings and resolutions: None from a separate review pass.
|
||||
- Known limitations: unchanged from the original TASK-048 entry, plus: each line now scrolls
|
||||
horizontally independently rather than sharing one scroll position across lines (invisible in
|
||||
practice, since every line starts at column 0).
|
||||
- Next recommended task: none queued from this incident.
|
||||
- Next owner: Claude, continuing per the standing direction.
|
||||
|
||||
## 2026-09-23 (continued) — TASK-048 follow-up 2: synchronized horizontal scroll across all lines
|
||||
|
||||
- Agent and role: Claude, incident responder -- the human product owner tested the previous fix and
|
||||
reported only the touched line scrolled, not the whole file ("한줄만 좌우 스크롤 되는 방향은 아닌
|
||||
것 같은데... 전체적으로 다 좌우스크롤이 되어야지").
|
||||
- Root cause and fix: the prior fix gave each line's code `Text` its own independent
|
||||
`Modifier.horizontalScroll`, reasoned (wrongly) to look the same as one synced scroll region since
|
||||
every line starts at column 0 -- in practice dragging one line left the rest of the file visibly
|
||||
unscrolled. First attempted fix (hoisting one shared `rememberScrollState()` across all per-row
|
||||
`horizontalScroll` modifiers) made horizontal drag stop working entirely, on any row -- multiple
|
||||
separate scrollable gesture regions don't cleanly share one `ScrollableState` the way a single
|
||||
shared scroll region does. Real fix: split `CodeBlock` by mode. With word wrap off (the only case
|
||||
where horizontal scroll is used at all), gutter and code go back to two side-by-side Columns
|
||||
(gutter fixed, one `Column` of all code lines wrapped in exactly one shared
|
||||
`Modifier.horizontalScroll`) -- safe here because with wrapping off every line is exactly one
|
||||
visual row, so the two Columns' heights can never drift out of sync (the failure mode from the
|
||||
earlier two-Column attempt only happens when a line wraps to multiple rows). With word wrap on,
|
||||
gutter+code stay paired per-Row (unchanged from the prior fix, still needed for wrap alignment).
|
||||
- Files changed: `feature/codebrowser/CodeBrowserScreen.kt` (`CodeBlock` split into a wrap-mode Row-
|
||||
per-line branch and a no-wrap two-Column-with-one-shared-scroll branch; new small `LineNumber`
|
||||
helper to avoid duplicating the gutter `Text` across both branches),
|
||||
`tasks/048-code-viewer-readability.md` (updated with this as fix round 4).
|
||||
- Checks and results: `./gradlew compileDebugKotlin compileDebugUnitTestKotlin testDebugUnitTest assembleDebug lintDebug`
|
||||
BUILD SUCCESSFUL, 0 lint errors, all unit tests passing unmodified.
|
||||
- Live verification: installed over wireless `adb` after each of the two attempts in this round.
|
||||
Final round screenshot-confirmed a horizontal swipe on one line moving every visible line's content
|
||||
together by the same amount (line 2, 3, 5, 6, 7, 17, 26 all shifted in sync), and re-confirmed word
|
||||
wrap mode's number alignment was untouched by this change.
|
||||
- Review findings and resolutions: None from a separate review pass.
|
||||
- Known limitations: unchanged from the original TASK-048 entry; the earlier "each line scrolls
|
||||
independently" limitation from the previous follow-up entry no longer applies -- superseded by this
|
||||
fix.
|
||||
- Next recommended task: none queued from this incident.
|
||||
- Next owner: Claude, continuing per the standing direction.
|
||||
|
||||
|
||||
137
tasks/048-code-viewer-readability.md
Normal file
137
tasks/048-code-viewer-readability.md
Normal file
@ -0,0 +1,137 @@
|
||||
# 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.
|
||||
Loading…
x
Reference in New Issue
Block a user