diff --git a/app/src/main/java/com/ubisam/ubgitea/domain/history/UnifiedDiffParser.kt b/app/src/main/java/com/ubisam/ubgitea/domain/history/UnifiedDiffParser.kt index 78c54a4..65417b7 100644 --- a/app/src/main/java/com/ubisam/ubgitea/domain/history/UnifiedDiffParser.kt +++ b/app/src/main/java/com/ubisam/ubgitea/domain/history/UnifiedDiffParser.kt @@ -62,16 +62,20 @@ object UnifiedDiffParser { val headers = section.filter { it.startsWith("@@") } val entries = mutableListOf() parseHunks(section).forEachIndexed { index, hunk -> - val header = HUNK_HEADER.find(headers.getOrElse(index) { "" }) - var leftNumber = header?.groupValues?.get(1)?.toIntOrNull() ?: 1 - var rightNumber = header?.groupValues?.get(2)?.toIntOrNull() ?: 1 - entries += DiffEntry(null, null, hunkLabel = header?.groupValues?.get(3)?.trim().orEmpty()) + val headerLine = headers.getOrElse(index) { "" } + val header = HUNK_HEADER.find(headerLine) + // A header this parser cannot read gets no line numbers rather than numbers counted + // 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 -> entries += DiffEntry( left = row.left, right = row.right, - leftNumber = if (row.left != null) leftNumber++ else null, - rightNumber = if (row.right != null) rightNumber++ else null, + leftNumber = if (row.left != null) leftNumber?.also { leftNumber = it + 1 } else null, + rightNumber = if (row.right != null) rightNumber?.also { rightNumber = it + 1 } else null, ) } } @@ -109,8 +113,9 @@ object UnifiedDiffParser { for (line in section) { when { line.startsWith("@@") -> hunks.add(mutableListOf()) + // 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 - line.startsWith("+++") || line.startsWith("---") -> Unit line.startsWith("+") -> hunks.last() += DiffLine.Added(line.substring(1)) line.startsWith("-") -> hunks.last() += DiffLine.Removed(line.substring(1)) line.startsWith(" ") -> hunks.last() += DiffLine.Context(line.substring(1)) diff --git a/app/src/main/java/com/ubisam/ubgitea/feature/repositories/RepositoryListViewModel.kt b/app/src/main/java/com/ubisam/ubgitea/feature/repositories/RepositoryListViewModel.kt index 79e4765..501d9ad 100644 --- a/app/src/main/java/com/ubisam/ubgitea/feature/repositories/RepositoryListViewModel.kt +++ b/app/src/main/java/com/ubisam/ubgitea/feature/repositories/RepositoryListViewModel.kt @@ -185,6 +185,11 @@ class RepositoryListViewModel( val browserRequests: SharedFlow = mutableBrowserRequests.asSharedFlow() private var operation: Job? = null private val rowBranchJobs = mutableMapOf() + // 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() private var activeAccountName: String? = null private var hasAuthenticatedSession = false private var repositoryContent: RepositoryListUiState.Content? = null @@ -317,6 +322,7 @@ class RepositoryListViewModel( fun toggleFavorite(repository: RepositoryRowUiModel) { val content = mutableState.value as? RepositoryListUiState.Content ?: return + if (!favoriteRequestsInFlight.add(repository.coordinates)) return val mode = content.mode // 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 @@ -404,6 +410,8 @@ class RepositoryListViewModel( updateRow(repository.coordinates) { it.copy(favoriteAction = FavoriteActionUiState.Failed) } } } + } finally { + favoriteRequestsInFlight -= repository.coordinates } } } diff --git a/app/src/test/java/com/ubisam/ubgitea/domain/history/UnifiedDiffParserTest.kt b/app/src/test/java/com/ubisam/ubgitea/domain/history/UnifiedDiffParserTest.kt index 3f9bfcc..db2b066 100644 --- a/app/src/test/java/com/ubisam/ubgitea/domain/history/UnifiedDiffParserTest.kt +++ b/app/src/test/java/com/ubisam/ubgitea/domain/history/UnifiedDiffParserTest.kt @@ -216,4 +216,43 @@ class UnifiedDiffParserTest { assertNull(added.leftNumber) 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) + } } diff --git a/app/src/test/java/com/ubisam/ubgitea/feature/repositories/RepositoryListViewModelTest.kt b/app/src/test/java/com/ubisam/ubgitea/feature/repositories/RepositoryListViewModelTest.kt index 9630a48..fba2b59 100644 --- a/app/src/test/java/com/ubisam/ubgitea/feature/repositories/RepositoryListViewModelTest.kt +++ b/app/src/test/java/com/ubisam/ubgitea/feature/repositories/RepositoryListViewModelTest.kt @@ -461,6 +461,30 @@ class RepositoryListViewModelTest { 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(), 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 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)))) diff --git a/docs/development-log.md b/docs/development-log.md index 2130e01..c6a9f81 100644 --- a/docs/development-log.md +++ b/docs/development-log.md @@ -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 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. + +## 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. diff --git a/tasks/064-review-fixes-star-double-tap-and-diff-lines.md b/tasks/064-review-fixes-star-double-tap-and-diff-lines.md new file mode 100644 index 0000000..4f7893d --- /dev/null +++ b/tasks/064-review-fixes-star-double-tap-and-diff-lines.md @@ -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.