TASK-064: ignore a second star tap in flight; keep -- and ++ lines in diffs

Agent: main (opus)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
최민영 2026-09-29 20:25:49 +09:00
parent f6ac20c8ed
commit 6a149087c0
6 changed files with 163 additions and 7 deletions

View File

@ -62,16 +62,20 @@ object UnifiedDiffParser {
val headers = section.filter { it.startsWith("@@") } val headers = section.filter { it.startsWith("@@") }
val entries = mutableListOf<DiffEntry>() val entries = mutableListOf<DiffEntry>()
parseHunks(section).forEachIndexed { index, hunk -> parseHunks(section).forEachIndexed { index, hunk ->
val header = HUNK_HEADER.find(headers.getOrElse(index) { "" }) val headerLine = headers.getOrElse(index) { "" }
var leftNumber = header?.groupValues?.get(1)?.toIntOrNull() ?: 1 val header = HUNK_HEADER.find(headerLine)
var rightNumber = header?.groupValues?.get(2)?.toIntOrNull() ?: 1 // A header this parser cannot read gets no line numbers rather than numbers counted
entries += DiffEntry(null, null, hunkLabel = header?.groupValues?.get(3)?.trim().orEmpty()) // from 1, which would look right and be wrong. Its raw text stands in for the label.
var leftNumber = header?.groupValues?.get(1)?.toIntOrNull()
var rightNumber = header?.groupValues?.get(2)?.toIntOrNull()
val label = header?.groupValues?.get(3)?.trim() ?: headerLine.trim()
entries += DiffEntry(null, null, hunkLabel = label)
hunkToRows(hunk).forEach { row -> hunkToRows(hunk).forEach { row ->
entries += DiffEntry( entries += DiffEntry(
left = row.left, left = row.left,
right = row.right, right = row.right,
leftNumber = if (row.left != null) leftNumber++ else null, leftNumber = if (row.left != null) leftNumber?.also { leftNumber = it + 1 } else null,
rightNumber = if (row.right != null) rightNumber++ else null, rightNumber = if (row.right != null) rightNumber?.also { rightNumber = it + 1 } else null,
) )
} }
} }
@ -109,8 +113,9 @@ object UnifiedDiffParser {
for (line in section) { for (line in section) {
when { when {
line.startsWith("@@") -> hunks.add(mutableListOf<DiffLine>()) line.startsWith("@@") -> hunks.add(mutableListOf<DiffLine>())
// The file's own "---"/"+++" lines come before its first hunk and are skipped here.
// Inside a hunk, "+++x" is an added "++x" and "--- x" a removed "-- x".
hunks.isEmpty() -> Unit hunks.isEmpty() -> Unit
line.startsWith("+++") || line.startsWith("---") -> Unit
line.startsWith("+") -> hunks.last() += DiffLine.Added(line.substring(1)) line.startsWith("+") -> hunks.last() += DiffLine.Added(line.substring(1))
line.startsWith("-") -> hunks.last() += DiffLine.Removed(line.substring(1)) line.startsWith("-") -> hunks.last() += DiffLine.Removed(line.substring(1))
line.startsWith(" ") -> hunks.last() += DiffLine.Context(line.substring(1)) line.startsWith(" ") -> hunks.last() += DiffLine.Context(line.substring(1))

View File

@ -185,6 +185,11 @@ class RepositoryListViewModel(
val browserRequests: SharedFlow<String> = mutableBrowserRequests.asSharedFlow() val browserRequests: SharedFlow<String> = mutableBrowserRequests.asSharedFlow()
private var operation: Job? = null private var operation: Job? = null
private val rowBranchJobs = mutableMapOf<RepositoryCoordinates, Job>() private val rowBranchJobs = mutableMapOf<RepositoryCoordinates, Job>()
// Repositories whose star/unstar request is still on its way. A second tap on the same row is
// ignored until the first request settles: the tap flips the star at once, so without this a
// quick double tap sent a star and an unstar together, and whichever reached the server last
// won -- not necessarily the one the screen ended up showing.
private val favoriteRequestsInFlight = mutableSetOf<RepositoryCoordinates>()
private var activeAccountName: String? = null private var activeAccountName: String? = null
private var hasAuthenticatedSession = false private var hasAuthenticatedSession = false
private var repositoryContent: RepositoryListUiState.Content? = null private var repositoryContent: RepositoryListUiState.Content? = null
@ -317,6 +322,7 @@ class RepositoryListViewModel(
fun toggleFavorite(repository: RepositoryRowUiModel) { fun toggleFavorite(repository: RepositoryRowUiModel) {
val content = mutableState.value as? RepositoryListUiState.Content ?: return val content = mutableState.value as? RepositoryListUiState.Content ?: return
if (!favoriteRequestsInFlight.add(repository.coordinates)) return
val mode = content.mode val mode = content.mode
// Once this session has confirmed a star succeeded (favoriteAction == Starred), a repeat // Once this session has confirmed a star succeeded (favoriteAction == Starred), a repeat
// tap on the ALL list now un-stars instead of starring again -- a real toggle, matching // tap on the ALL list now un-stars instead of starring again -- a real toggle, matching
@ -404,6 +410,8 @@ class RepositoryListViewModel(
updateRow(repository.coordinates) { it.copy(favoriteAction = FavoriteActionUiState.Failed) } updateRow(repository.coordinates) { it.copy(favoriteAction = FavoriteActionUiState.Failed) }
} }
} }
} finally {
favoriteRequestsInFlight -= repository.coordinates
} }
} }
} }

View File

@ -216,4 +216,43 @@ class UnifiedDiffParserTest {
assertNull(added.leftNumber) assertNull(added.leftNumber)
assertEquals(2, added.rightNumber) assertEquals(2, added.rightNumber)
} }
@Test
fun `lines whose own text starts with -- or ++ are kept inside a hunk`() {
val patch = """
diff --git a/query.sql b/query.sql
--- a/query.sql
+++ b/query.sql
@@ -1,2 +1,2 @@
--- old comment
+++i;
SELECT 1;
""".trimIndent()
val rows = UnifiedDiffParser.parseFileEntries(patch, "query.sql").drop(1)
assertEquals(2, rows.size)
assertEquals(DiffLine.Removed("-- old comment"), rows[0].left)
assertEquals(DiffLine.Added("++i;"), rows[0].right)
assertEquals(2, rows[1].leftNumber)
assertEquals(2, rows[1].rightNumber)
}
@Test
fun `a hunk header that cannot be read leaves its rows unnumbered instead of counting from 1`() {
val patch = """
diff --git a/foo.kt b/foo.kt
--- a/foo.kt
+++ b/foo.kt
@@ something unexpected @@
unchanged
+added
""".trimIndent()
val entries = UnifiedDiffParser.parseFileEntries(patch, "foo.kt")
assertEquals("@@ something unexpected @@", entries[0].hunkLabel)
assertTrue(entries.drop(1).all { it.leftNumber == null && it.rightNumber == null })
assertEquals(2, entries.drop(1).size)
}
} }

View File

@ -461,6 +461,30 @@ class RepositoryListViewModelTest {
assertEquals(listOf(RepositoryCoordinates.of("owner", "repo")), browser.starredCalls) assertEquals(listOf(RepositoryCoordinates.of("owner", "repo")), browser.starredCalls)
} }
@Test
fun `a second tap while the first star request is still on its way is ignored`() = runTest(dispatcher.scheduler) {
val browser = FakeBrowser(repositories = listOf(page1Of(repo("repo", starsCount = 2))))
val viewModel = viewModel(browser)
advanceUntilIdle()
val row = (viewModel.state.value as RepositoryListUiState.Content).repositories.single()
viewModel.toggleFavorite(row)
val afterFirstTap = (viewModel.state.value as RepositoryListUiState.Content).repositories.single()
viewModel.toggleFavorite(afterFirstTap)
advanceUntilIdle()
val settled = (viewModel.state.value as RepositoryListUiState.Content).repositories.single()
assertEquals(FavoriteActionUiState.Starred, settled.favoriteAction)
assertEquals(3, settled.starsCount)
assertEquals(listOf(RepositoryCoordinates.of("owner", "repo")), browser.starredCalls)
assertEquals(emptyList<RepositoryCoordinates>(), browser.unstarredCalls)
// Once the first request has settled, the next tap goes through as usual.
viewModel.toggleFavorite(settled)
advanceUntilIdle()
assertEquals(listOf(RepositoryCoordinates.of("owner", "repo")), browser.unstarredCalls)
}
@Test @Test
fun `a repeat tap on an already-starred row from the all-repositories list un-stars it`() = runTest(dispatcher.scheduler) { fun `a repeat tap on an already-starred row from the all-repositories list un-stars it`() = runTest(dispatcher.scheduler) {
val browser = FakeBrowser(repositories = listOf(page1Of(repo("repo", starsCount = 4)))) val browser = FakeBrowser(repositories = listOf(page1Of(repo("repo", starsCount = 4))))

View File

@ -1434,3 +1434,24 @@ This is the append-only implementation and handoff record shared by Codex and Cl
The logic now lives in `tools/sync-main-folder.ps1` (UTF-8 with a byte order mark), moves the The logic now lives in `tools/sync-main-folder.ps1` (UTF-8 with a byte order mark), moves the
files in the way into `.git/sync-backup/` itself, and the batch file is plain ASCII. Run four files in the way into `.git/sync-backup/` itself, and the batch file is plain ASCII. Run four
ways against a throwaway clone, including a Korean file name, from a code page 949 console. ways against a throwaway clone, including a Korean file name, from a code page 949 console.
## 2026-09-29 — TASK-063: task records brought up to date
- Request: after a full code review, the human product owner said Codex is not used, every
redesign item installed on their Z Fold7 is confirmed, the release-hardening leftovers are on
hold, and biometric reauthentication is out.
- What changed: no task waits on a Codex review any more; TASK-045 and TASK-049 to TASK-061 are
marked confirmed; TASK-016's two open items are on hold; `docs/product.md` records no biometric
reauthentication and has no open product decisions left. Owner/reviewer history is unchanged.
## 2026-09-29 (continued) — TASK-064: star double tap, dropped diff lines
- Found by the full review. A quick double tap on a star sent a star and an unstar together, and
whichever reached the NAS last won; a second tap is now ignored until the first request settles.
- Found while fixing the diff parser: a changed line whose own text starts with `--` or `++` was
dropped from the diff, because the parser skipped every `---`/`+++` line, not only the file's
own header lines. SQL/Lua comments, Markdown `---` and a `++i;` at the start of a line were
affected, and the line numbers below them were off. A hunk header the parser cannot read now
leaves its rows unnumbered instead of numbering them from 1.
- Verification: `compileDebugKotlin compileDebugUnitTestKotlin testDebugUnitTest assembleDebug
lintDebug` pass; 440 unit tests, 0 failures, three of them new. Not yet confirmed on a device.

View File

@ -0,0 +1,59 @@
# TASK-064: Fixes from the full review — star double tap, diff lines
Status: implemented; required checks pass; not yet confirmed on a device
Owner: Claude
Reviewer: human product owner
## Outcome
1. A quick double tap on a repository's star no longer sends a star and an unstar together.
2. A changed line whose own text starts with `--` or `++` (an SQL or Lua comment, a Markdown
`---`, a C++ `++i;` at the start of a line) shows in the diff. Before, it was dropped, and the
line numbers under it were off by one per dropped line.
3. A hunk header the parser cannot read leaves its rows unnumbered, instead of numbering them from
1.
## Scope
- Included: `RepositoryListViewModel.toggleFavorite`, `UnifiedDiffParser`.
- Excluded: anything else the review looked at.
## Writable paths
- `app/src/main/java/com/ubisam/ubgitea/feature/repositories/RepositoryListViewModel.kt`
- `app/src/main/java/com/ubisam/ubgitea/domain/history/UnifiedDiffParser.kt`
- `app/src/test/java/com/ubisam/ubgitea/feature/repositories/RepositoryListViewModelTest.kt`
- `app/src/test/java/com/ubisam/ubgitea/domain/history/UnifiedDiffParserTest.kt`
- `tasks/064-review-fixes-star-double-tap-and-diff-lines.md`
- `docs/development-log.md`
## Acceptance criteria
- [x] While a star or unstar request for a repository is on its way, another tap on that
repository's star does nothing. After it settles, the next tap works as before.
- [x] Inside a hunk, `+++x` is an added `++x` and `--- x` is a removed `-- x`. The file's own
`---`/`+++` lines, which come before the first hunk, are still skipped.
- [x] A hunk header that does not match `@@ -a[,b] +c[,d] @@` gives rows with no line numbers, and
its raw text is the hunk label.
- [x] Unit tests cover all three.
## Security and privacy
No additional concerns.
## Required checks
```text
./gradlew compileDebugKotlin compileDebugUnitTestKotlin testDebugUnitTest assembleDebug lintDebug
```
## Evidence
See the development log entry for TASK-064.
## Review findings
Found by the full code review of 2026-09-29. Other reported candidates were checked against the
code and dropped: the offline cache cannot show another account's list (every logout path clears
it), state updates run on the main thread without a suspension between read and write, and the
highlighter's patterns cannot match an empty string.