diff --git a/app/src/main/java/com/ubisam/ubgitea/MainActivity.kt b/app/src/main/java/com/ubisam/ubgitea/MainActivity.kt index 97ddc82..5c383bf 100644 --- a/app/src/main/java/com/ubisam/ubgitea/MainActivity.kt +++ b/app/src/main/java/com/ubisam/ubgitea/MainActivity.kt @@ -121,6 +121,7 @@ class MainActivity : ComponentActivity() { onPullRequestSelected = workItemsViewModel::openPullRequest, onNextPage = workItemsViewModel::nextPage, onPreviousPage = workItemsViewModel::previousPage, + onGoToPage = workItemsViewModel::goToPage, onRetryList = workItemsViewModel::retryList, onRetryDetail = workItemsViewModel::retryDetail, onBackToList = workItemsViewModel::backToList, @@ -142,6 +143,7 @@ class MainActivity : ComponentActivity() { pop() } }, + onNavigateImage = codeBrowserViewModel::navigateImage, historyState = historyState, onCommitSelected = historyViewModel::openCommit, onHistoryNextPage = historyViewModel::nextPage, diff --git a/app/src/main/java/com/ubisam/ubgitea/data/workitems/GiteaWorkItemsApi.kt b/app/src/main/java/com/ubisam/ubgitea/data/workitems/GiteaWorkItemsApi.kt index 169bbf1..6ca04ef 100644 --- a/app/src/main/java/com/ubisam/ubgitea/data/workitems/GiteaWorkItemsApi.kt +++ b/app/src/main/java/com/ubisam/ubgitea/data/workitems/GiteaWorkItemsApi.kt @@ -31,11 +31,11 @@ import okhttp3.HttpUrl.Companion.toHttpUrl /** * Read-only pull-request and issue data access. Gitea's issue-list endpoint can also return - * pull requests (a PR is internally an issue with a non-null `pull_request` field); entries with - * that field set are dropped here so [listIssues] never exposes a duplicate of a PR. Because that - * filtering happens client-side after fetching a page, a filtered page can contain fewer than - * [pageSize] genuine issues even when more exist — an accepted, documented tradeoff rather than a - * guessed, unverified server-side "issues only" query parameter. + * pull requests (a PR is internally an issue with a non-null `pull_request` field); [listIssues] + * sends `type=issues` (verified against Gitea's own swagger docs and source — + * routers/api/v1/repo/issue.go's ListIssues handler) so both the returned items and its + * `X-Total-Count` header exclude pull requests server-side. The client-side + * `pullRequest == null` filter is kept anyway as defense in depth, not relied on alone. */ class GiteaWorkItemsApi( private val config: OAuthClientConfig, @@ -45,7 +45,15 @@ class GiteaWorkItemsApi( ) : WorkItemsRepository { override suspend fun listIssues(repository: RepositoryCoordinates, page: Int, pageSize: Int): Page { validatePage(page, pageSize) - val response = get(repoUrl(repository, "issues").withPage(page, pageSize)) + // `type=issues` asks Gitea to exclude pull requests server-side, so X-Total-Count reflects + // genuine issues only — needed for direct page-number entry to compute a correct max page, + // not just an approximation. The client-side pullRequest==null filter below is kept as + // defense in depth (documented at the class level) rather than relied on for the count. + val response = get( + repoUrl(repository, "issues").withPage(page, pageSize).newBuilder() + .addQueryParameter("type", "issues") + .build(), + ) val items = parseList(response.body) .filter { it.pullRequest == null } .map(::mapIssue) @@ -245,7 +253,13 @@ class GiteaWorkItemsApi( val hasMore = headers.entries.firstOrNull { it.key.equals("X-HasMore", true) } ?.value?.equals("true", true) val needsNext = hasMore == true || hasMore == null && items.size == pageSize - return Page.of(items, if (needsNext) page.safeNextPage() else null) + // Gitea's issue/pull-request list endpoints report the total match count via this header + // (routers/api/v1/repo/issue.go, pull.go: ctx.SetTotalCountHeader(total)) — used to power + // direct page-number entry, since it's the one list in this app where "how many pages are + // there" is actually knowable rather than guessed from whether one more page happened to fill. + val totalCount = headers.entries.firstOrNull { it.key.equals("X-Total-Count", true) } + ?.value?.toIntOrNull() + return Page.of(items, if (needsNext) page.safeNextPage() else null, totalCount) } private fun String?.required(): String = takeIf { !it.isNullOrBlank() } diff --git a/app/src/main/java/com/ubisam/ubgitea/domain/model/RepositoryModels.kt b/app/src/main/java/com/ubisam/ubgitea/domain/model/RepositoryModels.kt index a21a94b..fa4bce0 100644 --- a/app/src/main/java/com/ubisam/ubgitea/domain/model/RepositoryModels.kt +++ b/app/src/main/java/com/ubisam/ubgitea/domain/model/RepositoryModels.kt @@ -79,11 +79,16 @@ data class CommitSummary( class Page private constructor( val items: List, val nextPage: Int?, + // Total item count across every page, when the endpoint reports one (Gitea's issue/pull + // request list endpoints do, via X-Total-Count; most others here don't) — null otherwise. + // Optional and defaulted so every existing call site (none of which need it) is unaffected. + val totalCount: Int? = null, ) { companion object { - fun of(items: Collection, nextPage: Int?): Page { + fun of(items: Collection, nextPage: Int?, totalCount: Int? = null): Page { require(nextPage == null || nextPage > 0) { "Next page must be positive" } - return Page(Collections.unmodifiableList(ArrayList(items)), nextPage) + require(totalCount == null || totalCount >= 0) { "Total count cannot be negative" } + return Page(Collections.unmodifiableList(ArrayList(items)), nextPage, totalCount) } } } diff --git a/app/src/main/java/com/ubisam/ubgitea/feature/branchdetail/BranchDetailScreen.kt b/app/src/main/java/com/ubisam/ubgitea/feature/branchdetail/BranchDetailScreen.kt index 9f097db..e50c211 100644 --- a/app/src/main/java/com/ubisam/ubgitea/feature/branchdetail/BranchDetailScreen.kt +++ b/app/src/main/java/com/ubisam/ubgitea/feature/branchdetail/BranchDetailScreen.kt @@ -38,6 +38,7 @@ fun BranchDetailScreen( onBreadcrumbSelected: (RepositoryPath) -> Unit, onRetryCodeBrowser: () -> Unit, onCodeBrowserBack: () -> Unit, + onNavigateImage: (Int) -> Unit, historyState: HistoryUiState, onCommitSelected: (CommitSummary) -> Unit, onHistoryNextPage: () -> Unit, @@ -54,15 +55,19 @@ fun BranchDetailScreen( ) { val pagerState = rememberPagerState(pageCount = { 2 }) val scope = rememberCoroutineScope() - // A page showing horizontally-scrollable source text (a plain file, or a diff's before/after - // panels) has its own left/right drag handling that directly conflicts with the pager's own — - // both are horizontal, so a drag meant to scroll code was instead partly consumed by the pager, - // which is what made scrolling code feel jittery and occasionally scroll text out of view - // entirely. Swiping between tabs only makes sense one level up from that anyway (Files' folder + // A page showing horizontally-gestured content — scrollable source text, a diff's before/after + // panels, an image gallery's swipe-to-navigate, or a CSV table's horizontal scroll — has its + // own left/right drag handling that directly conflicts with the pager's own — both are + // horizontal, so a drag meant for that content was instead partly consumed by the pager, which + // is what made scrolling code feel jittery and occasionally scroll text out of view entirely. + // Swiping between tabs only makes sense one level up from any of that anyway (Files' folder // listing, History's changed-file list), so the pager's own gesture is disabled while either - // tab is showing scrollable code, and re-enabled once back at that list. + // tab is showing one of these, and re-enabled once back at that list. val currentPageShowsScrollableCode = when (pagerState.currentPage) { - 0 -> codeBrowserState is CodeBrowserUiState.File && codeBrowserState.content is ContentClassification.Text + 0 -> codeBrowserState is CodeBrowserUiState.File && + (codeBrowserState.content is ContentClassification.Text || + codeBrowserState.content is ContentClassification.Image || + codeBrowserState.content is ContentClassification.Csv) else -> historyState is HistoryUiState.CommitDetailContent && historyState.selectedFile != null } Column(modifier.fillMaxSize().statusBarsPadding()) { @@ -92,6 +97,7 @@ fun BranchDetailScreen( onBack = onCodeBrowserBack, onAuthenticationExpired = onAuthenticationExpired, onViewHistory = { scope.launch { pagerState.animateScrollToPage(1) } }, + onNavigateImage = onNavigateImage, ) else -> HistoryScreen( state = historyState, diff --git a/app/src/main/java/com/ubisam/ubgitea/feature/codebrowser/CodeBrowserScreen.kt b/app/src/main/java/com/ubisam/ubgitea/feature/codebrowser/CodeBrowserScreen.kt index 0e7cccd..46130af 100644 --- a/app/src/main/java/com/ubisam/ubgitea/feature/codebrowser/CodeBrowserScreen.kt +++ b/app/src/main/java/com/ubisam/ubgitea/feature/codebrowser/CodeBrowserScreen.kt @@ -1,18 +1,30 @@ package com.ubisam.ubgitea.feature.codebrowser +import android.graphics.BitmapFactory import androidx.activity.compose.BackHandler +import androidx.compose.foundation.Image +import androidx.compose.foundation.background import androidx.compose.foundation.clickable +import androidx.compose.foundation.gestures.Orientation +import androidx.compose.foundation.gestures.ScrollableState +import androidx.compose.foundation.gestures.detectHorizontalDragGestures +import androidx.compose.foundation.gestures.rememberScrollableState +import androidx.compose.foundation.gestures.scrollable import androidx.compose.foundation.horizontalScroll import androidx.compose.foundation.layout.Arrangement +import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.ColumnScope import androidx.compose.foundation.layout.Row import androidx.compose.foundation.layout.fillMaxSize import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.foundation.layout.offset import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.layout.width import androidx.compose.foundation.rememberScrollState import androidx.compose.foundation.lazy.LazyColumn import androidx.compose.foundation.lazy.items +import androidx.compose.foundation.lazy.itemsIndexed import androidx.compose.foundation.text.selection.SelectionContainer import androidx.compose.foundation.verticalScroll import androidx.compose.material3.Button @@ -23,19 +35,30 @@ import androidx.compose.material3.OutlinedButton import androidx.compose.material3.Scaffold import androidx.compose.material3.Text import androidx.compose.runtime.Composable +import androidx.compose.runtime.State +import androidx.compose.runtime.mutableFloatStateOf +import androidx.compose.runtime.remember import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier +import androidx.compose.ui.draw.clipToBounds +import androidx.compose.ui.graphics.asImageBitmap +import androidx.compose.ui.input.pointer.pointerInput +import androidx.compose.ui.layout.ContentScale import androidx.compose.ui.platform.LocalClipboardManager +import androidx.compose.ui.platform.LocalDensity import androidx.compose.ui.res.stringResource import androidx.compose.ui.semantics.Role import androidx.compose.ui.text.AnnotatedString 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.dp import com.ubisam.ubgitea.R import com.ubisam.ubgitea.domain.model.RepositoryPath import com.ubisam.ubgitea.domain.model.TreeEntry import com.ubisam.ubgitea.domain.model.TreeEntryType import com.ubisam.ubgitea.feature.codebrowser.content.ContentClassification +import kotlin.math.roundToInt @Composable fun CodeBrowserScreen( @@ -46,6 +69,7 @@ fun CodeBrowserScreen( onBack: () -> Unit, onAuthenticationExpired: () -> Unit, onViewHistory: (CodeBrowserTarget) -> Unit, + onNavigateImage: (Int) -> Unit, modifier: Modifier = Modifier, ) { BackHandler(onBack = onBack) @@ -71,7 +95,7 @@ fun CodeBrowserScreen( } is CodeBrowserUiState.File -> { BrowserHeader(state.target, state.path, true, onBreadcrumbSelected, onViewHistory) - FileContent(state.content) + FileContent(state.content, state.path, state.siblingImagePaths, onNavigateImage) } is CodeBrowserUiState.UnsupportedEntry -> { BrowserHeader(state.target, state.path, true, onBreadcrumbSelected, onViewHistory) @@ -163,7 +187,12 @@ private fun EntryRow(entry: TreeEntry, onSelected: (TreeEntry) -> Unit) { @Composable @Suppress("DEPRECATION") -private fun FileContent(content: ContentClassification) { +private fun FileContent( + content: ContentClassification, + path: RepositoryPath, + siblingImagePaths: List, + onNavigateImage: (Int) -> Unit, +) { when (content) { is ContentClassification.Text -> { val clipboard = LocalClipboardManager.current @@ -184,6 +213,11 @@ private fun FileContent(content: ContentClassification) { } } } + is ContentClassification.Image -> { + val index = siblingImagePaths.indexOf(path) + ImageContent(content, index, siblingImagePaths.size, onNavigateImage) + } + is ContentClassification.Csv -> CsvTable(content) is ContentClassification.Binary -> Fallback(R.string.codebrowser_binary, content.sizeBytes) is ContentClassification.UnsupportedEncoding -> Fallback(R.string.codebrowser_unsupported_encoding, content.sizeBytes) is ContentClassification.Oversized -> Fallback(R.string.codebrowser_oversized, content.sizeBytes) @@ -194,6 +228,134 @@ private fun FileContent(content: ContentClassification) { } } +// 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). +@Composable +private fun ImageContent(content: ContentClassification.Image, currentIndex: Int, totalCount: Int, onNavigateImage: (Int) -> Unit) { + val bitmap = remember(content) { + val bytes = content.content.copyBytes() + BitmapFactory.decodeByteArray(bytes, 0, bytes.size)?.asImageBitmap() + } + val canNavigate = currentIndex != -1 && totalCount > 1 + Column(Modifier.fillMaxSize()) { + Row( + Modifier.fillMaxWidth().padding(horizontal = 16.dp, vertical = 8.dp), + horizontalArrangement = Arrangement.SpaceBetween, + ) { + Text(content.format.label, style = MaterialTheme.typography.bodySmall) + if (canNavigate) { + Text(stringResource(R.string.codebrowser_image_index, currentIndex + 1, totalCount), style = MaterialTheme.typography.bodySmall) + } + } + Box( + Modifier.fillMaxSize().let { base -> + if (canNavigate) { + // A discrete "which way, how far" swipe — not a continuous scroll position like + // the diff view's — so a plain drag-distance threshold is enough; no shared + // offset state (and none of that pattern's per-row bound bookkeeping) needed. + base.pointerInput(content) { + var accumulatedDragPx = 0f + detectHorizontalDragGestures( + onDragEnd = { + if (accumulatedDragPx <= -ImageSwipeThresholdPx) onNavigateImage(1) + else if (accumulatedDragPx >= ImageSwipeThresholdPx) onNavigateImage(-1) + accumulatedDragPx = 0f + }, + onHorizontalDrag = { _, dragAmount -> accumulatedDragPx += dragAmount }, + ) + } + } else { + base + } + }, + contentAlignment = Alignment.Center, + ) { + val decoded = bitmap + if (decoded != null) { + Image(decoded, contentDescription = null, contentScale = ContentScale.Fit, modifier = Modifier.fillMaxSize()) + } else { + Text(stringResource(R.string.codebrowser_image_decode_failed)) + } + } + } +} + +@Composable +private fun CsvTable(content: ContentClassification.Csv) { + if (content.rows.isEmpty()) { + CenterMessage { Text(stringResource(R.string.codebrowser_csv_empty)) } + return + } + val columnCount = remember(content) { content.rows.maxOf { it.size } } + val cellWidthPx = with(LocalDensity.current) { CsvCellWidth.toPx() } + val maxHorizontalOffsetPx = remember(content, cellWidthPx) { + (columnCount * cellWidthPx).coerceAtLeast(0f) + } + val horizontalOffsetPx = remember(content) { mutableFloatStateOf(0f) } + // Same manually-driven shared offset as the History diff view's DiffSideLine, and for the same + // reason: many rows sharing one Modifier.horizontalScroll's ScrollState fight over its bounds. + // Every CSV cell is the same fixed width here, so that specific failure mode couldn't actually + // occur — but reusing the already-verified-correct pattern costs nothing and stays consistent. + val horizontalScrollableState = rememberScrollableState { delta -> + val previousOffset = horizontalOffsetPx.floatValue + val newOffset = (previousOffset - delta).coerceIn(0f, maxHorizontalOffsetPx) + horizontalOffsetPx.floatValue = newOffset + previousOffset - newOffset + } + Column(Modifier.fillMaxSize()) { + if (content.isTruncated) { + Text( + stringResource(R.string.codebrowser_text_truncated), + modifier = Modifier.padding(horizontal = 16.dp, vertical = 4.dp), + ) + } + LazyColumn(Modifier.fillMaxSize()) { + itemsIndexed(content.rows) { rowIndex, row -> + CsvRow(row, columnCount, rowIndex == 0, horizontalScrollableState, horizontalOffsetPx) + } + } + } +} + +@Composable +private fun CsvRow( + row: List, + columnCount: Int, + isHeader: Boolean, + scrollableState: ScrollableState, + horizontalOffsetPx: State, +) { + Box( + Modifier.fillMaxWidth().clipToBounds() + .scrollable(state = scrollableState, orientation = Orientation.Horizontal), + ) { + Row(Modifier.offset { IntOffset(-horizontalOffsetPx.value.roundToInt(), 0) }) { + for (column in 0 until columnCount) { + CsvCell(row.getOrElse(column) { "" }, isHeader) + } + } + } + HorizontalDivider() +} + +@Composable +private fun CsvCell(text: String, isHeader: Boolean) { + Box( + Modifier.width(CsvCellWidth) + .background(if (isHeader) MaterialTheme.colorScheme.surfaceVariant else MaterialTheme.colorScheme.surface) + .padding(horizontal = 8.dp, vertical = 6.dp), + ) { + Text( + text, + style = MaterialTheme.typography.bodySmall, + fontWeight = if (isHeader) FontWeight.Bold else null, + maxLines = 1, + softWrap = false, + ) + } +} + @Composable private fun Fallback(message: Int, sizeBytes: Int) = CenterMessage { Text(stringResource(message, sizeBytes)) @@ -209,6 +371,9 @@ private fun CenterMessage(content: @Composable ColumnScope.() -> Unit) { ) } +private const val ImageSwipeThresholdPx = 120f +private val CsvCellWidth = 120.dp + private val CodeBrowserError.messageResource: Int get() = when (this) { CodeBrowserError.OFFLINE -> R.string.codebrowser_offline diff --git a/app/src/main/java/com/ubisam/ubgitea/feature/codebrowser/CodeBrowserViewModel.kt b/app/src/main/java/com/ubisam/ubgitea/feature/codebrowser/CodeBrowserViewModel.kt index 2de8f25..5a96c03 100644 --- a/app/src/main/java/com/ubisam/ubgitea/feature/codebrowser/CodeBrowserViewModel.kt +++ b/app/src/main/java/com/ubisam/ubgitea/feature/codebrowser/CodeBrowserViewModel.kt @@ -57,6 +57,11 @@ sealed interface CodeBrowserUiState { val target: CodeBrowserTarget, val path: RepositoryPath, val content: ContentClassification, + // Sibling image files in the same directory, in listing order, captured at the moment this + // file was opened — lets the image viewer swipe to the previous/next image without a + // second directory fetch. Empty for anything opened outside a directory listing (e.g. a + // retry from Error) or when the containing folder has no other images. + val siblingImagePaths: List = emptyList(), ) : CodeBrowserUiState data class UnsupportedEntry( val target: CodeBrowserTarget, @@ -90,7 +95,14 @@ class CodeBrowserViewModel( val target = currentTarget() ?: return when (entry.type) { TreeEntryType.DIRECTORY -> loadDirectory(target, entry.path) - TreeEntryType.FILE -> loadFile(target, entry.path) + TreeEntryType.FILE -> { + val siblingImagePaths = (mutableState.value as? CodeBrowserUiState.Directory) + ?.entries + ?.filter { it.type == TreeEntryType.FILE && it.path.hasImageExtension() } + ?.map { it.path } + ?: emptyList() + loadFile(target, entry.path, siblingImagePaths) + } TreeEntryType.SYMLINK, TreeEntryType.SUBMODULE -> { operation?.cancel() mutableState.value = CodeBrowserUiState.UnsupportedEntry(target, entry.path, entry.type) @@ -98,6 +110,18 @@ class CodeBrowserViewModel( } } + // Moves to the previous (-1) or next (+1) image among this file's siblingImagePaths, or does + // nothing at either end of the list or if the current file isn't part of an image gallery. + fun navigateImage(delta: Int) { + val current = mutableState.value as? CodeBrowserUiState.File ?: return + val siblings = current.siblingImagePaths + val index = siblings.indexOf(current.path) + if (index == -1) return + val newIndex = index + delta + if (newIndex !in siblings.indices) return + loadFile(current.target, siblings[newIndex], siblings) + } + fun openBreadcrumb(path: RepositoryPath) { val target = currentTarget() ?: return loadDirectory(target, path) @@ -142,14 +166,18 @@ class CodeBrowserViewModel( } } - private fun loadFile(target: CodeBrowserTarget, path: RepositoryPath) = startOperation { + private fun loadFile( + target: CodeBrowserTarget, + path: RepositoryPath, + siblingImagePaths: List = emptyList(), + ) = startOperation { mutableState.value = CodeBrowserUiState.Loading(target, path, isFile = true) try { val classification = withContext(defaultDispatcher) { val file = browser.getFile(target.repository, target.ref, path) - classifier.classify(file.content) + classifier.classify(file.content, path) } - mutableState.value = CodeBrowserUiState.File(target, path, classification) + mutableState.value = CodeBrowserUiState.File(target, path, classification, siblingImagePaths) } catch (failure: Throwable) { handleFailure(target, path, failure, isFile = true) } @@ -220,6 +248,15 @@ class CodeBrowserViewModel( } } +// Used only to pre-build the swipeable image gallery from a directory listing without fetching +// every sibling's bytes first; the actual format is still verified by magic bytes on load +// (SourceContentClassifier), so a wrongly-named file here just fails to classify as an image, +// it doesn't get misrendered. +private val IMAGE_EXTENSIONS = setOf("png", "jpg", "jpeg", "gif", "bmp", "webp") + +private fun RepositoryPath.hasImageExtension(): Boolean = + value.substringAfterLast('.', missingDelimiterValue = "").lowercase() in IMAGE_EXTENSIONS + internal fun Throwable.toCodeBrowserError(): CodeBrowserError = when (this) { is ApiFailure.AuthenticationExpired -> CodeBrowserError.AUTHENTICATION_EXPIRED is ApiFailure.PermissionDenied -> CodeBrowserError.PERMISSION_DENIED diff --git a/app/src/main/java/com/ubisam/ubgitea/feature/codebrowser/content/ContentClassification.kt b/app/src/main/java/com/ubisam/ubgitea/feature/codebrowser/content/ContentClassification.kt index cbb3510..b600f26 100644 --- a/app/src/main/java/com/ubisam/ubgitea/feature/codebrowser/content/ContentClassification.kt +++ b/app/src/main/java/com/ubisam/ubgitea/feature/codebrowser/content/ContentClassification.kt @@ -1,5 +1,7 @@ package com.ubisam.ubgitea.feature.codebrowser.content +import com.ubisam.ubgitea.domain.model.SourceContent + sealed class ContentClassification { abstract val sizeBytes: Int @@ -10,6 +12,24 @@ sealed class ContentClassification { val isTruncated: Boolean, ) : ContentClassification() + // A dedicated image classification is worth its own case rather than folding into Binary: + // Binary means "nothing sensible to render", while an image file is exactly the opposite — + // recognized bytes the UI can decode and show. `content` (not raw ByteArray) reuses + // SourceContent's already-correct content-based equals()/hashCode() instead of duplicating it. + data class Image( + override val sizeBytes: Int, + val content: SourceContent, + val format: ImageFormat, + ) : ContentClassification() + + // A CSV file's rows/columns are worth their own case for the same reason: distinct rendering + // (a table) needs distinct data (rows of cells), not a raw truncated character blob. + data class Csv( + override val sizeBytes: Int, + val rows: List>, + val isTruncated: Boolean, + ) : ContentClassification() + data class Binary(override val sizeBytes: Int) : ContentClassification() data class LfsPointer( @@ -21,4 +41,12 @@ sealed class ContentClassification { data class UnsupportedEncoding(override val sizeBytes: Int) : ContentClassification() data class Oversized(override val sizeBytes: Int) : ContentClassification() + + enum class ImageFormat(val label: String) { + PNG("PNG"), + JPEG("JPEG"), + GIF("GIF"), + BMP("BMP"), + WEBP("WebP"), + } } diff --git a/app/src/main/java/com/ubisam/ubgitea/feature/codebrowser/content/SourceContentClassifier.kt b/app/src/main/java/com/ubisam/ubgitea/feature/codebrowser/content/SourceContentClassifier.kt index f981379..d16ad31 100644 --- a/app/src/main/java/com/ubisam/ubgitea/feature/codebrowser/content/SourceContentClassifier.kt +++ b/app/src/main/java/com/ubisam/ubgitea/feature/codebrowser/content/SourceContentClassifier.kt @@ -1,5 +1,6 @@ package com.ubisam.ubgitea.feature.codebrowser.content +import com.ubisam.ubgitea.domain.model.RepositoryPath import com.ubisam.ubgitea.domain.model.SourceContent import java.nio.ByteBuffer import java.nio.charset.CharacterCodingException @@ -21,13 +22,21 @@ class SourceContentClassifier( require(maxRenderedLines > 0) { "maxRenderedLines must be positive" } } - fun classify(content: SourceContent): ContentClassification { + // `path` is optional (defaulted) purely so every existing call site/test that doesn't care + // about extension-based classification (CSV) keeps compiling unchanged; image classification + // never needs it, since image formats are detected from the bytes themselves, not the name. + fun classify(content: SourceContent, path: RepositoryPath? = null): ContentClassification { val bytes = content.copyBytes() val sizeBytes = bytes.size if (sizeBytes > maxInputBytes) { return ContentClassification.Oversized(sizeBytes) } + + detectImageFormat(bytes)?.let { format -> + return ContentClassification.Image(sizeBytes, SourceContent.from(bytes), format) + } + if (bytes.any { it == NUL_BYTE }) { return ContentClassification.Binary(sizeBytes) } @@ -37,9 +46,29 @@ class SourceContentClassifier( parseLfsPointer(decoded, sizeBytes)?.let { return it } + if (path != null && path.value.endsWith(".csv", ignoreCase = true)) { + return renderCsv(decoded, sizeBytes) + } + return renderText(decoded, sizeBytes) } + // Checked by magic bytes, not extension, so a mislabeled or extensionless file still renders + // as an image when it genuinely is one (and a renamed non-image file isn't misclassified). + private fun detectImageFormat(bytes: ByteArray): ContentClassification.ImageFormat? = when { + bytes.startsWith(PNG_SIGNATURE) -> ContentClassification.ImageFormat.PNG + bytes.size >= 3 && bytes[0] == 0xFF.toByte() && bytes[1] == 0xD8.toByte() && bytes[2] == 0xFF.toByte() -> + ContentClassification.ImageFormat.JPEG + bytes.startsWith(GIF87A_SIGNATURE) || bytes.startsWith(GIF89A_SIGNATURE) -> ContentClassification.ImageFormat.GIF + bytes.size >= 2 && bytes[0] == 'B'.code.toByte() && bytes[1] == 'M'.code.toByte() -> ContentClassification.ImageFormat.BMP + bytes.size >= 12 && bytes.copyOfRange(0, 4).contentEquals(RIFF_SIGNATURE) && + bytes.copyOfRange(8, 12).contentEquals(WEBP_SIGNATURE) -> ContentClassification.ImageFormat.WEBP + else -> null + } + + private fun ByteArray.startsWith(prefix: ByteArray): Boolean = + size >= prefix.size && prefix.indices.all { this[it] == prefix[it] } + private fun decodeStrictUtf8(bytes: ByteArray): String? { val decoder = Charsets.UTF_8.newDecoder() .onMalformedInput(CodingErrorAction.REPORT) @@ -97,11 +126,81 @@ class SourceContentClassifier( ) } + // Same truncation shape as renderText (bound by lines, then by characters) so a CSV can't + // blow the same budgets a huge text file would; the bounded text is then parsed into cells. + private fun renderCsv(text: String, sizeBytes: Int): ContentClassification.Csv { + val lines = text.split("\n") + val isLineTruncated = lines.size > maxRenderedLines + val boundedByLines = if (isLineTruncated) lines.take(maxRenderedLines).joinToString("\n") else text + + val renderedCharacterCount = boundedByLines.codePointCount(0, boundedByLines.length) + val isCharTruncated = renderedCharacterCount > maxRenderedCharacters + val bounded = if (isCharTruncated) { + val endIndex = boundedByLines.offsetByCodePoints(0, maxRenderedCharacters) + boundedByLines.substring(0, endIndex) + } else { + boundedByLines + } + + return ContentClassification.Csv(sizeBytes, parseCsv(bounded), isLineTruncated || isCharTruncated) + } + + // RFC 4180-ish: comma-separated, double-quote-quoted fields, "" as an escaped quote inside a + // quoted field, CRLF or LF line endings. No trailing empty row for text ending in a newline. + private fun parseCsv(text: String): List> { + val rows = mutableListOf>() + var row = mutableListOf() + var field = StringBuilder() + var inQuotes = false + var index = 0 + while (index < text.length) { + val char = text[index] + if (inQuotes) { + when { + char == '"' && index + 1 < text.length && text[index + 1] == '"' -> { + field.append('"') + index++ + } + char == '"' -> inQuotes = false + else -> field.append(char) + } + } else { + when (char) { + '"' -> inQuotes = true + ',' -> { + row.add(field.toString()) + field = StringBuilder() + } + '\r' -> Unit + '\n' -> { + row.add(field.toString()) + field = StringBuilder() + rows.add(row) + row = mutableListOf() + } + else -> field.append(char) + } + } + index++ + } + if (field.isNotEmpty() || row.isNotEmpty()) { + row.add(field.toString()) + rows.add(row) + } + return rows + } + private companion object { const val NUL_BYTE: Byte = 0 const val LFS_POINTER_MAX_BYTES = 1024 const val LFS_VERSION_LINE = "version https://git-lfs.github.com/spec/v1" val LFS_OID_LINE = Regex("^oid sha256:([0-9a-f]{64})$") val LFS_SIZE_LINE = Regex("^size (0|[1-9][0-9]*)$") + + val PNG_SIGNATURE = byteArrayOf(0x89.toByte(), 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A) + val GIF87A_SIGNATURE = "GIF87a".toByteArray(Charsets.US_ASCII) + val GIF89A_SIGNATURE = "GIF89a".toByteArray(Charsets.US_ASCII) + val RIFF_SIGNATURE = "RIFF".toByteArray(Charsets.US_ASCII) + val WEBP_SIGNATURE = "WEBP".toByteArray(Charsets.US_ASCII) } } diff --git a/app/src/main/java/com/ubisam/ubgitea/feature/workitems/WorkItemsScreen.kt b/app/src/main/java/com/ubisam/ubgitea/feature/workitems/WorkItemsScreen.kt index bd78cef..5d932bc 100644 --- a/app/src/main/java/com/ubisam/ubgitea/feature/workitems/WorkItemsScreen.kt +++ b/app/src/main/java/com/ubisam/ubgitea/feature/workitems/WorkItemsScreen.kt @@ -2,29 +2,44 @@ package com.ubisam.ubgitea.feature.workitems import androidx.activity.compose.BackHandler import androidx.compose.foundation.clickable +import androidx.compose.foundation.horizontalScroll import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.ColumnScope +import androidx.compose.foundation.layout.PaddingValues import androidx.compose.foundation.layout.Row import androidx.compose.foundation.layout.fillMaxSize import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.layout.width import androidx.compose.foundation.lazy.LazyColumn import androidx.compose.foundation.lazy.items +import androidx.compose.foundation.rememberScrollState +import androidx.compose.foundation.text.KeyboardActions +import androidx.compose.foundation.text.KeyboardOptions import androidx.compose.material3.Button +import androidx.compose.material3.ButtonDefaults import androidx.compose.material3.CircularProgressIndicator import androidx.compose.material3.HorizontalDivider import androidx.compose.material3.MaterialTheme import androidx.compose.material3.OutlinedButton +import androidx.compose.material3.OutlinedTextField import androidx.compose.material3.Scaffold import androidx.compose.material3.Tab import androidx.compose.material3.TabRow import androidx.compose.material3.Text +import androidx.compose.material3.TextButton import androidx.compose.runtime.Composable +import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.saveable.rememberSaveable +import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.res.stringResource import androidx.compose.ui.semantics.Role +import androidx.compose.ui.text.input.ImeAction +import androidx.compose.ui.text.input.KeyboardType import androidx.compose.ui.unit.dp import com.ubisam.ubgitea.R import com.ubisam.ubgitea.domain.workitems.CommentSummary @@ -45,6 +60,7 @@ fun WorkItemsScreen( onPullRequestSelected: (PullRequestSummary) -> Unit, onNextPage: () -> Unit, onPreviousPage: () -> Unit, + onGoToPage: (Int) -> Unit, onRetryList: () -> Unit, onRetryDetail: () -> Unit, onBackToList: () -> Unit, @@ -69,7 +85,7 @@ fun WorkItemsScreen( IssueRow(issue) { onIssueSelected(issue) } } } - PageFooter(state.page, state.page > 1, state.hasNextPage, onPreviousPage, onNextPage) + PageFooter(state.page, state.page > 1, state.hasNextPage, state.totalPages, onPreviousPage, onNextPage, onGoToPage) } is WorkItemsUiState.PullRequestList -> { BackHandler(onBack = onBack) @@ -79,7 +95,7 @@ fun WorkItemsScreen( PullRequestRow(pr) { onPullRequestSelected(pr) } } } - PageFooter(state.page, state.page > 1, state.hasNextPage, onPreviousPage, onNextPage) + PageFooter(state.page, state.page > 1, state.hasNextPage, state.totalPages, onPreviousPage, onNextPage, onGoToPage) } is WorkItemsUiState.Empty -> { BackHandler(onBack = onBack) @@ -248,19 +264,98 @@ private fun ErrorMessage(type: WorkItemsError, onRetry: () -> Unit, onAuthentica } } +// Issues/PRs are the one list in this app where the server actually reports a total match count +// (Gitea's X-Total-Count header), so a real max page number is known rather than guessed — that's +// specifically why numbered jump-to-page pagination is offered only here (repository/history/ +// notification lists keep plain previous/next, since Gitea gives no total for those). Falls back +// to plain previous/next itself whenever totalPages is unavailable (e.g. an older Gitea instance +// that doesn't send the header), rather than showing a broken or misleading page count. @Composable -private fun PageFooter(page: Int, hasPrevious: Boolean, hasNext: Boolean, onPrevious: () -> Unit, onNext: () -> Unit) { - Row( - Modifier.fillMaxWidth().padding(16.dp), - horizontalArrangement = Arrangement.SpaceBetween, - verticalAlignment = Alignment.CenterVertically, - ) { - OutlinedButton(onClick = onPrevious, enabled = hasPrevious) { Text(stringResource(R.string.workitems_previous_page)) } - Text(stringResource(R.string.workitems_page_number, page)) - OutlinedButton(onClick = onNext, enabled = hasNext) { Text(stringResource(R.string.workitems_next_page)) } +private fun PageFooter( + page: Int, + hasPrevious: Boolean, + hasNext: Boolean, + totalPages: Int?, + onPrevious: () -> Unit, + onNext: () -> Unit, + onGoToPage: (Int) -> Unit, +) { + if (totalPages == null) { + Row( + Modifier.fillMaxWidth().padding(16.dp), + horizontalArrangement = Arrangement.SpaceBetween, + verticalAlignment = Alignment.CenterVertically, + ) { + OutlinedButton(onClick = onPrevious, enabled = hasPrevious) { Text(stringResource(R.string.workitems_previous_page)) } + Text(stringResource(R.string.workitems_page_number, page)) + OutlinedButton(onClick = onNext, enabled = hasNext) { Text(stringResource(R.string.workitems_next_page)) } + } + } else { + NumberedPageFooter(page, totalPages, onPrevious, onNext, onGoToPage) } } +@Composable +private fun NumberedPageFooter(page: Int, totalPages: Int, onPrevious: () -> Unit, onNext: () -> Unit, onGoToPage: (Int) -> Unit) { + Column(Modifier.fillMaxWidth().padding(vertical = 8.dp)) { + Row( + Modifier.fillMaxWidth().horizontalScroll(rememberScrollState()).padding(horizontal = 16.dp), + horizontalArrangement = Arrangement.spacedBy(4.dp), + verticalAlignment = Alignment.CenterVertically, + ) { + TextButton(onClick = onPrevious, enabled = page > 1) { Text(stringResource(R.string.workitems_previous_page)) } + for (candidate in pageWindow(page, totalPages)) { + val selected = candidate == page + OutlinedButton( + onClick = { onGoToPage(candidate) }, + enabled = !selected, + colors = if (selected) { + ButtonDefaults.outlinedButtonColors( + containerColor = MaterialTheme.colorScheme.primaryContainer, + disabledContainerColor = MaterialTheme.colorScheme.primaryContainer, + disabledContentColor = MaterialTheme.colorScheme.onPrimaryContainer, + ) + } else { + ButtonDefaults.outlinedButtonColors() + }, + contentPadding = PaddingValues(horizontal = 14.dp, vertical = 6.dp), + ) { Text(candidate.toString()) } + } + TextButton(onClick = onNext, enabled = page < totalPages) { Text(stringResource(R.string.workitems_next_page)) } + } + Row( + Modifier.fillMaxWidth().padding(horizontal = 16.dp, vertical = 8.dp), + horizontalArrangement = Arrangement.Center, + verticalAlignment = Alignment.CenterVertically, + ) { + var input by rememberSaveable(page) { mutableStateOf(page.toString()) } + fun submit() { + input.toIntOrNull()?.let { onGoToPage(it.coerceIn(1, totalPages)) } + } + Text("[", style = MaterialTheme.typography.bodyLarge) + OutlinedTextField( + value = input, + onValueChange = { input = it.filter(Char::isDigit).take(6) }, + modifier = Modifier.width(72.dp), + singleLine = true, + keyboardOptions = KeyboardOptions(keyboardType = KeyboardType.Number, imeAction = ImeAction.Go), + keyboardActions = KeyboardActions(onGo = { submit() }), + ) + Text(stringResource(R.string.workitems_page_jump_suffix, totalPages), style = MaterialTheme.typography.bodyLarge) + TextButton(onClick = { submit() }) { Text(stringResource(R.string.workitems_page_jump_go)) } + } + } +} + +// A window of up to 5 consecutive page numbers around the current one — e.g. page 1 of a large +// list shows 1..5, matching what was asked for ("1페이지에선 5까지 보여줌"), and the window keeps +// sliding rather than sticking once it nears the last page. +private fun pageWindow(current: Int, totalPages: Int, windowSize: Int = 5): IntRange { + if (totalPages <= windowSize) return 1..totalPages + val start = (current - windowSize / 2).coerceIn(1, totalPages - windowSize + 1) + return start..(start + windowSize - 1) +} + @Composable private fun CenterMessage(content: @Composable ColumnScope.() -> Unit) { Column( diff --git a/app/src/main/java/com/ubisam/ubgitea/feature/workitems/WorkItemsViewModel.kt b/app/src/main/java/com/ubisam/ubgitea/feature/workitems/WorkItemsViewModel.kt index 3007951..2bcc951 100644 --- a/app/src/main/java/com/ubisam/ubgitea/feature/workitems/WorkItemsViewModel.kt +++ b/app/src/main/java/com/ubisam/ubgitea/feature/workitems/WorkItemsViewModel.kt @@ -10,6 +10,7 @@ import com.ubisam.ubgitea.core.network.ApiFailure import com.ubisam.ubgitea.core.network.OkHttpTransport import com.ubisam.ubgitea.data.repositories.BearerTokenProvider import com.ubisam.ubgitea.data.workitems.GiteaWorkItemsApi +import com.ubisam.ubgitea.domain.model.Page import com.ubisam.ubgitea.domain.model.RepositoryCoordinates import com.ubisam.ubgitea.domain.workitems.CommentSummary import com.ubisam.ubgitea.domain.workitems.IssueDetail @@ -41,6 +42,9 @@ sealed interface WorkItemsUiState { items: List, val page: Int, val hasNextPage: Boolean, + // Gitea's issue list reports a total match count (X-Total-Count); null if that header was + // ever missing/unparseable, in which case the UI falls back to plain previous/next. + val totalPages: Int? = null, ) : WorkItemsUiState { val items: List = Collections.unmodifiableList(items.toList()) } @@ -49,6 +53,7 @@ sealed interface WorkItemsUiState { items: List, val page: Int, val hasNextPage: Boolean, + val totalPages: Int? = null, ) : WorkItemsUiState { val items: List = Collections.unmodifiableList(items.toList()) } @@ -104,6 +109,15 @@ class WorkItemsViewModel(private val repository: WorkItemsRepository) : ViewMode loadList(target, tab, page - 1) } + // Direct page-number entry (only meaningful once totalPages is known — the UI only offers this + // where it is). An out-of-range request just loads whatever the server returns for it, same as + // any other page number; the server, not a client-side guess, is the source of truth. + fun goToPage(page: Int) { + val (target, tab, _) = mutableState.value.currentPageInfo() ?: return + if (page < 1) return + loadList(target, tab, page) + } + fun retryList() { val current = mutableState.value as? WorkItemsUiState.ListError ?: return loadList(current.target, current.tab, page = 1) @@ -150,7 +164,7 @@ class WorkItemsViewModel(private val repository: WorkItemsRepository) : ViewMode mutableState.value = if (items.isEmpty() && page == 1) { WorkItemsUiState.Empty(target, tab) } else { - WorkItemsUiState.IssueList(target, items, page, result.nextPage != null) + WorkItemsUiState.IssueList(target, items, page, result.nextPage != null, result.totalPages(PAGE_SIZE)) } } WorkItemsTab.PULL_REQUESTS -> { @@ -159,7 +173,7 @@ class WorkItemsViewModel(private val repository: WorkItemsRepository) : ViewMode mutableState.value = if (items.isEmpty() && page == 1) { WorkItemsUiState.Empty(target, tab) } else { - WorkItemsUiState.PullRequestList(target, items, page, result.nextPage != null) + WorkItemsUiState.PullRequestList(target, items, page, result.nextPage != null, result.totalPages(PAGE_SIZE)) } } } @@ -239,6 +253,9 @@ class WorkItemsViewModel(private val repository: WorkItemsRepository) : ViewMode } } +private fun Page.totalPages(pageSize: Int): Int? = + totalCount?.let { total -> ((total + pageSize - 1) / pageSize).coerceAtLeast(1) } + internal fun Throwable.toWorkItemsError(): WorkItemsError = when (this) { is ApiFailure.AuthenticationExpired -> WorkItemsError.AUTHENTICATION_EXPIRED is ApiFailure.PermissionDenied -> WorkItemsError.PERMISSION_DENIED diff --git a/app/src/main/res/values/strings_codebrowser.xml b/app/src/main/res/values/strings_codebrowser.xml index 0143b7d..ba718e8 100644 --- a/app/src/main/res/values/strings_codebrowser.xml +++ b/app/src/main/res/values/strings_codebrowser.xml @@ -26,4 +26,7 @@ 다시 시도 다시 로그인 히스토리 + %1$d / %2$d + 이미지를 표시할 수 없습니다. + 이 CSV 파일은 비어 있습니다. diff --git a/app/src/main/res/values/strings_workitems.xml b/app/src/main/res/values/strings_workitems.xml index 512c486..95866f5 100644 --- a/app/src/main/res/values/strings_workitems.xml +++ b/app/src/main/res/values/strings_workitems.xml @@ -7,6 +7,8 @@ 이전 페이지 다음 페이지 %1$d페이지 + / %1$d ] + 이동 %1$s · 댓글 %2$d개 작성자: %1$s (알 수 없는 사용자) diff --git a/app/src/test/java/com/ubisam/ubgitea/data/workitems/GiteaWorkItemsApiTest.kt b/app/src/test/java/com/ubisam/ubgitea/data/workitems/GiteaWorkItemsApiTest.kt index 9b5dd48..3a37a4d 100644 --- a/app/src/test/java/com/ubisam/ubgitea/data/workitems/GiteaWorkItemsApiTest.kt +++ b/app/src/test/java/com/ubisam/ubgitea/data/workitems/GiteaWorkItemsApiTest.kt @@ -46,6 +46,54 @@ class GiteaWorkItemsApiTest { assertEquals(listOf("api", "v1", "repos", "team name", "mobile+#", "issues"), url.pathSegments) } + @Test + fun `issue list requests server-side issues-only filtering`() = runTest { + val transport = QueueTransport(response("[]")) + + api(transport).listIssues(repository, 1, 20) + + val url = transport.urls.single().toHttpUrl() + assertEquals("issues", url.queryParameter("type")) + } + + @Test + fun `issue list exposes X-Total-Count as page total`() = runTest { + val transport = QueueTransport( + response( + """[{"number":1,"title":"Bug","state":"open","user":{"login":"dev"},"updated_at":"2026-08-25T10:20:30Z"}]""", + mapOf("X-Total-Count" to "42"), + ), + ) + + val page = api(transport).listIssues(repository, 1, 20) + + assertEquals(42, page.totalCount) + } + + @Test + fun `issue list total count is null when header is absent`() = runTest { + val transport = QueueTransport(response("[]")) + + val page = api(transport).listIssues(repository, 1, 20) + + assertNull(page.totalCount) + } + + @Test + fun `pull request list exposes X-Total-Count as page total`() = runTest { + val transport = QueueTransport( + response( + """[{"number":1,"title":"Add feature","state":"open","user":{"login":"dev"}, + "base":{"ref":"main"},"head":{"ref":"feature"},"updated_at":"2026-08-25T10:20:30Z"}]""", + mapOf("X-Total-Count" to "7"), + ), + ) + + val page = api(transport).listPullRequests(repository, 1, 20) + + assertEquals(7, page.totalCount) + } + @Test fun `issue detail maps labels assignees and body`() = runTest { val transport = QueueTransport( diff --git a/app/src/test/java/com/ubisam/ubgitea/domain/model/RepositoryModelsTest.kt b/app/src/test/java/com/ubisam/ubgitea/domain/model/RepositoryModelsTest.kt index 9f7e2c2..5d59e9b 100644 --- a/app/src/test/java/com/ubisam/ubgitea/domain/model/RepositoryModelsTest.kt +++ b/app/src/test/java/com/ubisam/ubgitea/domain/model/RepositoryModelsTest.kt @@ -38,4 +38,25 @@ class RepositoryModelsTest { } assertEquals(listOf("first", "second"), page.items) } + + @Test + fun pageTotalCountDefaultsToNull() { + val page = Page.of(listOf("first"), nextPage = null) + + assertEquals(null, page.totalCount) + } + + @Test + fun pageExposesGivenTotalCount() { + val page = Page.of(listOf("first"), nextPage = 2, totalCount = 45) + + assertEquals(45, page.totalCount) + } + + @Test + fun pageRejectsNegativeTotalCount() { + assertThrows(IllegalArgumentException::class.java) { + Page.of(listOf("first"), nextPage = null, totalCount = -1) + } + } } diff --git a/app/src/test/java/com/ubisam/ubgitea/feature/codebrowser/CodeBrowserViewModelTest.kt b/app/src/test/java/com/ubisam/ubgitea/feature/codebrowser/CodeBrowserViewModelTest.kt index d7bbb60..86d8e2c 100644 --- a/app/src/test/java/com/ubisam/ubgitea/feature/codebrowser/CodeBrowserViewModelTest.kt +++ b/app/src/test/java/com/ubisam/ubgitea/feature/codebrowser/CodeBrowserViewModelTest.kt @@ -49,7 +49,7 @@ class CodeBrowserViewModelTest { advanceUntilIdle() val root = viewModel.state.value as CodeBrowserUiState.Directory - viewModel.openEntry(root.entries.single()) + viewModel.openEntry(root.entries.single { it.name == "src" }) advanceUntilIdle() val directory = viewModel.state.value as CodeBrowserUiState.Directory viewModel.openEntry(directory.entries.single()) @@ -67,7 +67,7 @@ class CodeBrowserViewModelTest { val viewModel = CodeBrowserViewModel(FakeBrowser(), defaultDispatcher = dispatcher) viewModel.open(target) advanceUntilIdle() - viewModel.openEntry((viewModel.state.value as CodeBrowserUiState.Directory).entries.single()) + viewModel.openEntry((viewModel.state.value as CodeBrowserUiState.Directory).entries.single { it.name == "src" }) advanceUntilIdle() viewModel.openEntry((viewModel.state.value as CodeBrowserUiState.Directory).entries.single()) advanceUntilIdle() @@ -85,18 +85,87 @@ class CodeBrowserViewModelTest { val viewModel = CodeBrowserViewModel(FakeBrowser(rootEntryType = TreeEntryType.SUBMODULE), defaultDispatcher = dispatcher) viewModel.open(target) advanceUntilIdle() - viewModel.openEntry((viewModel.state.value as CodeBrowserUiState.Directory).entries.single()) + viewModel.openEntry((viewModel.state.value as CodeBrowserUiState.Directory).entries.single { it.name == "src" }) assertTrue(viewModel.state.value is CodeBrowserUiState.UnsupportedEntry) } + @Test + fun `opening an image captures sibling images from the directory listing in order`() = runTest(dispatcher.scheduler) { + val browser = FakeBrowser() + val viewModel = CodeBrowserViewModel(browser, defaultDispatcher = dispatcher) + viewModel.open(target) + advanceUntilIdle() + viewModel.openEntry((viewModel.state.value as CodeBrowserUiState.Directory).entries.first { it.name == "gallery" }) + advanceUntilIdle() + val gallery = viewModel.state.value as CodeBrowserUiState.Directory + + viewModel.openEntry(gallery.entries.single { it.name == "a.png" }) + advanceUntilIdle() + + val file = viewModel.state.value as CodeBrowserUiState.File + assertTrue(file.content is ContentClassification.Image) + // notes.txt (not an image) is excluded from the gallery, in listing order. + assertEquals(listOf("gallery/a.png", "gallery/b.png", "gallery/c.png"), file.siblingImagePaths.map { it.value }) + } + + @Test + fun `navigateImage moves within the gallery and stops at either end`() = runTest(dispatcher.scheduler) { + val browser = FakeBrowser() + val viewModel = CodeBrowserViewModel(browser, defaultDispatcher = dispatcher) + viewModel.open(target) + advanceUntilIdle() + viewModel.openEntry((viewModel.state.value as CodeBrowserUiState.Directory).entries.first { it.name == "gallery" }) + advanceUntilIdle() + viewModel.openEntry((viewModel.state.value as CodeBrowserUiState.Directory).entries.single { it.name == "a.png" }) + advanceUntilIdle() + + viewModel.navigateImage(1) + advanceUntilIdle() + assertEquals("gallery/b.png", (viewModel.state.value as CodeBrowserUiState.File).path.value) + + viewModel.navigateImage(1) + advanceUntilIdle() + assertEquals("gallery/c.png", (viewModel.state.value as CodeBrowserUiState.File).path.value) + + // Already at the last image — navigating further forward does nothing. + viewModel.navigateImage(1) + advanceUntilIdle() + assertEquals("gallery/c.png", (viewModel.state.value as CodeBrowserUiState.File).path.value) + + viewModel.navigateImage(-1) + advanceUntilIdle() + assertEquals("gallery/b.png", (viewModel.state.value as CodeBrowserUiState.File).path.value) + } + + @Test + fun `navigateImage does nothing for a file opened outside a directory listing`() = runTest(dispatcher.scheduler) { + val browser = FakeBrowser(fileFailure = ApiFailure.NotFound()) + val viewModel = CodeBrowserViewModel(browser, defaultDispatcher = dispatcher) + viewModel.open(target) + advanceUntilIdle() + viewModel.openEntry((viewModel.state.value as CodeBrowserUiState.Directory).entries.first { it.name == "gallery" }) + advanceUntilIdle() + viewModel.openEntry((viewModel.state.value as CodeBrowserUiState.Directory).entries.single { it.name == "a.png" }) + advanceUntilIdle() + browser.fileFailure = null + viewModel.retry() + advanceUntilIdle() + + val beforePath = (viewModel.state.value as CodeBrowserUiState.File).path + viewModel.navigateImage(1) + advanceUntilIdle() + + assertEquals(beforePath, (viewModel.state.value as CodeBrowserUiState.File).path) + } + @Test fun `not found failure is typed and retry preserves file operation`() = runTest(dispatcher.scheduler) { val browser = FakeBrowser(fileFailure = ApiFailure.NotFound()) val viewModel = CodeBrowserViewModel(browser, defaultDispatcher = dispatcher) viewModel.open(target) advanceUntilIdle() - viewModel.openEntry((viewModel.state.value as CodeBrowserUiState.Directory).entries.single()) + viewModel.openEntry((viewModel.state.value as CodeBrowserUiState.Directory).entries.single { it.name == "src" }) advanceUntilIdle() viewModel.openEntry((viewModel.state.value as CodeBrowserUiState.Directory).entries.single()) advanceUntilIdle() @@ -111,7 +180,7 @@ class CodeBrowserViewModelTest { private class FakeBrowser( private val rootEntryType: TreeEntryType = TreeEntryType.DIRECTORY, - private val fileFailure: Throwable? = null, + var fileFailure: Throwable? = null, ) : GiteaRepositoryBrowser { val refs = mutableListOf() var fileRequests = 0 @@ -122,8 +191,17 @@ class CodeBrowserViewModelTest { override suspend fun listTree(repository: RepositoryCoordinates, ref: RepositoryRef, path: RepositoryPath, page: Int, pageSize: Int): Page { refs += ref.value val entries = when (path.value) { - "" -> listOf(TreeEntry("src", RepositoryPath.of("src"), rootEntryType, null)) + "" -> listOf( + TreeEntry("src", RepositoryPath.of("src"), rootEntryType, null), + TreeEntry("gallery", RepositoryPath.of("gallery"), TreeEntryType.DIRECTORY, null), + ) "src" -> listOf(TreeEntry("한글.kt", RepositoryPath.of("src/한글.kt"), TreeEntryType.FILE, 6)) + "gallery" -> listOf( + TreeEntry("a.png", RepositoryPath.of("gallery/a.png"), TreeEntryType.FILE, 4), + TreeEntry("b.png", RepositoryPath.of("gallery/b.png"), TreeEntryType.FILE, 4), + TreeEntry("notes.txt", RepositoryPath.of("gallery/notes.txt"), TreeEntryType.FILE, 4), + TreeEntry("c.png", RepositoryPath.of("gallery/c.png"), TreeEntryType.FILE, 4), + ) else -> emptyList() } return Page.of(entries, null) @@ -132,7 +210,12 @@ class CodeBrowserViewModelTest { refs += ref.value fileRequests++ fileFailure?.let { throw it } - return SourceFile(path, ref, SourceContent.from("코드".encodeToByteArray()), "text/plain") + val bytes = if (path.value.endsWith(".png")) { + byteArrayOf(0x89.toByte(), 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A) + } else { + "코드".encodeToByteArray() + } + return SourceFile(path, ref, SourceContent.from(bytes), "text/plain") } override suspend fun listCommits(repository: RepositoryCoordinates, ref: RepositoryRef, path: RepositoryPath?, page: Int, pageSize: Int): Page = Page.of(emptyList(), null) } diff --git a/app/src/test/java/com/ubisam/ubgitea/feature/codebrowser/content/SourceContentClassifierTest.kt b/app/src/test/java/com/ubisam/ubgitea/feature/codebrowser/content/SourceContentClassifierTest.kt index db80d56..18b61ab 100644 --- a/app/src/test/java/com/ubisam/ubgitea/feature/codebrowser/content/SourceContentClassifierTest.kt +++ b/app/src/test/java/com/ubisam/ubgitea/feature/codebrowser/content/SourceContentClassifierTest.kt @@ -1,5 +1,6 @@ package com.ubisam.ubgitea.feature.codebrowser.content +import com.ubisam.ubgitea.domain.model.RepositoryPath import com.ubisam.ubgitea.domain.model.SourceContent import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse @@ -84,6 +85,110 @@ class SourceContentClassifierTest { assertEquals(ContentClassification.Binary(sizeBytes = 4), result) } + // --- image --- + + @Test + fun classifiesPngSignatureAsImage() { + // The PNG signature already contains a NUL byte, so this also proves image detection + // runs before the NUL-byte-means-Binary check, not after it. + val bytes = byteArrayOf(0x89.toByte(), 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A, 0, 0, 0, 0) + val result = classifier.classify(SourceContent.from(bytes)) + + assertTrue(result is ContentClassification.Image) + assertEquals(ContentClassification.ImageFormat.PNG, (result as ContentClassification.Image).format) + assertEquals(12, result.sizeBytes) + } + + @Test + fun classifiesJpegSignatureAsImage() { + val bytes = byteArrayOf(0xFF.toByte(), 0xD8.toByte(), 0xFF.toByte(), 0xE0.toByte(), 0, 0) + val result = classifier.classify(SourceContent.from(bytes)) + + assertEquals(ContentClassification.ImageFormat.JPEG, (result as ContentClassification.Image).format) + } + + @Test + fun classifiesGifSignatureAsImage() { + val bytes = "GIF89a".toByteArray(Charsets.US_ASCII) + byteArrayOf(0, 0, 0, 0) + val result = classifier.classify(SourceContent.from(bytes)) + + assertEquals(ContentClassification.ImageFormat.GIF, (result as ContentClassification.Image).format) + } + + @Test + fun classifiesBmpSignatureAsImage() { + val bytes = "BM".toByteArray(Charsets.US_ASCII) + byteArrayOf(0, 0, 0, 0) + val result = classifier.classify(SourceContent.from(bytes)) + + assertEquals(ContentClassification.ImageFormat.BMP, (result as ContentClassification.Image).format) + } + + @Test + fun classifiesWebpSignatureAsImage() { + val bytes = "RIFF".toByteArray(Charsets.US_ASCII) + byteArrayOf(0, 0, 0, 0) + "WEBP".toByteArray(Charsets.US_ASCII) + val result = classifier.classify(SourceContent.from(bytes)) + + assertEquals(ContentClassification.ImageFormat.WEBP, (result as ContentClassification.Image).format) + } + + @Test + fun oversizedImageContentIsRejectedBeforeImageDetection() { + val exact = SourceContentClassifier(maxInputBytes = 2, maxRenderedCharacters = 40, maxRenderedLines = 5) + val bytes = byteArrayOf(0x89.toByte(), 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A) + + val result = exact.classify(SourceContent.from(bytes)) + + assertEquals(ContentClassification.Oversized(sizeBytes = 8), result) + } + + // --- CSV --- + + @Test + fun classifiesDotCsvPathAsCsvTable() { + val result = classifier.classify(content("a,b,c\n1,2,3\n"), RepositoryPath.of("data.csv")) + + assertEquals( + ContentClassification.Csv( + sizeBytes = "a,b,c\n1,2,3\n".toByteArray(Charsets.UTF_8).size, + rows = listOf(listOf("a", "b", "c"), listOf("1", "2", "3")), + isTruncated = false, + ), + result, + ) + } + + @Test + fun leavesNonCsvExtensionAsPlainText() { + val result = classifier.classify(content("a,b,c\n1,2,3\n"), RepositoryPath.of("data.txt")) + + assertTrue(result is ContentClassification.Text) + } + + @Test + fun leavesCsvContentAsTextWhenNoPathGiven() { + val result = classifier.classify(content("a,b,c\n1,2,3\n")) + + assertTrue(result is ContentClassification.Text) + } + + @Test + fun csvParsingHandlesQuotedFieldsWithCommasAndEscapedQuotes() { + val result = classifier.classify(content("name,note\n\"Doe, John\",\"He said \"\"hi\"\"\"\n"), RepositoryPath.of("d.csv")) + + val csv = result as ContentClassification.Csv + assertEquals(listOf(listOf("name", "note"), listOf("Doe, John", "He said \"hi\"")), csv.rows) + } + + @Test + fun csvRowCountIsTruncatedAtLineLimit() { + val limited = SourceContentClassifier(maxInputBytes = 1_000, maxRenderedCharacters = 1_000, maxRenderedLines = 2) + + val result = limited.classify(content("a\nb\nc\n"), RepositoryPath.of("d.csv")) as ContentClassification.Csv + + assertEquals(listOf(listOf("a"), listOf("b")), result.rows) + assertTrue(result.isTruncated) + } + // --- unsupported encoding --- @Test diff --git a/app/src/test/java/com/ubisam/ubgitea/feature/workitems/WorkItemsViewModelTest.kt b/app/src/test/java/com/ubisam/ubgitea/feature/workitems/WorkItemsViewModelTest.kt index 24d7d27..d0f30fd 100644 --- a/app/src/test/java/com/ubisam/ubgitea/feature/workitems/WorkItemsViewModelTest.kt +++ b/app/src/test/java/com/ubisam/ubgitea/feature/workitems/WorkItemsViewModelTest.kt @@ -159,6 +159,53 @@ class WorkItemsViewModelTest { assertEquals(WorkItemsError.AUTHENTICATION_EXPIRED, error.type) } + @Test + fun `total count from the API is exposed as a rounded-up total page count`() = runTest(dispatcher.scheduler) { + // 45 total issues at a page size of 20 is 3 pages, not 2 — the fractional last page must + // round up, not truncate. + val repo = FakeWorkItems(issues = listOf(Page.of(listOf(issue(1)), nextPage = 2, totalCount = 45))) + val viewModel = WorkItemsViewModel(repo) + + viewModel.open(target) + advanceUntilIdle() + + val state = viewModel.state.value as WorkItemsUiState.IssueList + assertEquals(3, state.totalPages) + } + + @Test + fun `missing total count leaves totalPages null`() = runTest(dispatcher.scheduler) { + val repo = FakeWorkItems(issues = listOf(Page.of(listOf(issue(1)), nextPage = 2))) + val viewModel = WorkItemsViewModel(repo) + + viewModel.open(target) + advanceUntilIdle() + + val state = viewModel.state.value as WorkItemsUiState.IssueList + assertEquals(null, state.totalPages) + } + + @Test + fun `goToPage jumps directly to the requested page`() = runTest(dispatcher.scheduler) { + val repo = FakeWorkItems( + issues = listOf( + Page.of(listOf(issue(1)), nextPage = 2, totalCount = 60), + Page.of(listOf(issue(2)), nextPage = 3, totalCount = 60), + Page.of(listOf(issue(3)), nextPage = null, totalCount = 60), + ), + ) + val viewModel = WorkItemsViewModel(repo) + viewModel.open(target) + advanceUntilIdle() + + viewModel.goToPage(3) + advanceUntilIdle() + + val state = viewModel.state.value as WorkItemsUiState.IssueList + assertEquals(listOf(3L), state.items.map { it.number }) + assertEquals(3, state.page) + } + @Test fun `an empty first page maps to Empty rather than an empty list state`() = runTest(dispatcher.scheduler) { val repo = FakeWorkItems(issues = listOf(Page.of(emptyList(), null))) diff --git a/docs/development-log.md b/docs/development-log.md index cce5b77..143c22a 100644 --- a/docs/development-log.md +++ b/docs/development-log.md @@ -426,3 +426,29 @@ This is the append-only implementation and handoff record shared by Codex and Cl - Known limitations: Whether the trembling is now fully gone (as opposed to just less likely to occur) has not been separately re-confirmed live by the human product owner as of this commit — the scripted-swipe verification confirms the gesture-ownership conflict is structurally resolved (only one consumer is ever active at a time now), but a live human touch test is the only way to confirm the felt result. - Next recommended task: Get the human product owner's live confirmation of the whole TASK-025 batch plus this fix and the TASK-026 Custom Tabs login flow, together in one pass, then continue to the two remaining queued items (image viewer, Issues/PR numbered pagination). - Next owner: Claude, continuing per the standing direction. + +## 2026-08-27 — TASK-027: image viewer with swipe gallery navigation, and CSV table viewer + +- Agent and role: Claude, implementing two features requested together in one message: an image viewer (asked for immediately after the swipe-conflict fix above shipped) and, mid-implementation, a CSV-as-table viewer (asked for as a follow-on message while this task was already in progress). +- Commit/diff: continued on `claude/task-015-notifications`; `feature/codebrowser/content/ContentClassification.kt`, `feature/codebrowser/content/SourceContentClassifier.kt`, `feature/codebrowser/CodeBrowserViewModel.kt`, `feature/codebrowser/CodeBrowserScreen.kt`, `feature/branchdetail/BranchDetailScreen.kt`, `MainActivity.kt`, plus matching tests and `tasks/027-image-and-csv-viewer.md`. +- Features implemented: opening an image file (PNG/JPEG/GIF/BMP/WebP, detected by magic bytes not extension) now decodes and shows it, with left/right swipe moving to the previous/next image among the same folder's other images — explicitly required to never trigger the Files/History tab underneath it to change, per the human product owner's direct instruction ("탭 전환은 발생하면 안돼"). Opening a `.csv` file now shows a scrollable table (rows/columns, first row styled as a header) instead of raw comma-separated text. +- How it was implemented: image detection runs before the pre-existing NUL-byte-means-Binary check in `SourceContentClassifier`, since every one of these formats' bytes contain NULs and would otherwise always be caught as generic Binary first. The sibling-image gallery is built for free from the directory listing already fetched to reach the file (`CodeBrowserViewModel.openEntry` captures the current `Directory` state's image-extension file entries into the new `File` state's `siblingImagePaths`), rather than a second network round-trip. `navigateImage(delta)` looks up the current path's index in that list and loads the neighbor. The image swipe itself is a discrete drag-distance threshold (not a continuous scroll position, unlike the diff view), while the CSV table reuses the diff view's manually-driven shared-offset horizontal-scroll pattern verbatim for consistency, even though CSV's fixed-width cells mean the specific bound-thrashing bug that pattern exists to avoid couldn't actually occur here. The tab-swipe-vs-content-scroll gating fix from the previous follow-up was widened (not duplicated) to also cover Image and Csv content. +- Files changed: see `tasks/027-image-and-csv-viewer.md`'s Scope section for the full list. +- Checks and results: `./gradlew compileDebugKotlin compileDebugUnitTestKotlin` BUILD SUCCESSFUL; `./gradlew testDebugUnitTest` BUILD SUCCESSFUL, all unit tests passing (new coverage: magic-byte detection for all five formats, an image signature's embedded NUL not short-circuiting to Binary, oversized-before-image-detection ordering, CSV path-extension gating, quoted-field/escaped-quote parsing, row-truncation, gallery capture in listing order excluding a non-image sibling, `navigateImage` moving and stopping at both ends, `navigateImage` no-op outside a directory listing); `./gradlew assembleDebug lintDebug` BUILD SUCCESSFUL, 0 lint errors. Installed on the human product owner's device over wireless `adb`. +- Review findings and resolutions: None from a separate review pass. +- Known limitations: Not yet confirmed against a real image or CSV file on the live device — every repository browsed this session for a suitable test file (see TASK-028's log entry below for the same search, done for the same reason) didn't have one readily reachable. Relying on the unit test coverage above until a live file turns up. CSV column width is a fixed 120dp per cell, not sized to content. +- Next recommended task: Get live confirmation of both this and TASK-028 (or supply/point to a repo with an actual image/CSV file and some issues/PRs), otherwise continue per the standing direction. +- Next owner: Claude, continuing per the standing direction. + +## 2026-08-27 — TASK-028: numbered, jump-to-page pagination for the Issues/PR list + +- Agent and role: Claude, implementing the second half of the same combined request as TASK-027 above. +- Commit/diff: continued on `claude/task-015-notifications`; `domain/model/RepositoryModels.kt`, `data/workitems/GiteaWorkItemsApi.kt`, `feature/workitems/WorkItemsViewModel.kt`, `feature/workitems/WorkItemsScreen.kt`, `MainActivity.kt`, plus matching tests and `tasks/028-issues-prs-numbered-pagination.md`. +- Feature implemented: the Issues list and the PR list each now show up to 5 numbered page buttons (tap to jump directly), previous/next, and a `[ page / total ]`-style text field to type an exact page number and jump straight there — replacing plain previous/next-only, on this screen alone. Matches the earlier AskUserQuestion resolution from this session (apply numbered pagination only where a total count is knowable) — repository/history/notification lists are unchanged, still plain previous/next, since Gitea's endpoints for those don't report a total. +- How it was implemented: verified directly against Gitea's own source (`routers/api/v1/repo/issue.go`'s swagger doc comment) that both the issue-list and pull-request-list endpoints report a total match count via `X-Total-Count`, and that a documented `type=issues`/`type=pulls` query parameter exists — rather than assume either. `listIssues` now sends `type=issues` so both the returned items and the total count exclude pull requests server-side (previously, filtering was client-side only, which the class doc comment had explicitly defended as deliberate — that reasoning was rewritten now that a verified server-side option exists). `Page` gained an optional, defaulted `totalCount`, so none of the app's other 9+ `Page.of(...)` call sites needed to change. The numbered footer falls back to the original plain previous/next row whenever `totalPages` is null, rather than ever showing a broken or missing count. +- Files changed: see `tasks/028-issues-prs-numbered-pagination.md`'s Scope section for the full list. +- Checks and results: `./gradlew compileDebugKotlin compileDebugUnitTestKotlin` BUILD SUCCESSFUL; `./gradlew testDebugUnitTest` BUILD SUCCESSFUL, all unit tests passing (new coverage: `type=issues` sent, `X-Total-Count` parsed for both issues and pull requests, absent header leaves `totalCount` null, a fractional total rounds up to the correct page count, `goToPage` jumps directly, `Page.totalCount` defaults/validates correctly); `./gradlew assembleDebug lintDebug` BUILD SUCCESSFUL, 0 lint errors. Installed on the human product owner's device over wireless `adb`. +- Review findings and resolutions: None from a separate review pass. +- Known limitations: Checked the Issues and Pull Request tabs on four of the human product owner's actual repositories live; every one had zero issues and zero pull requests, so the numbered-pagination UI was never actually populated with real data this session — confirmed instead that the screen doesn't crash and correctly falls back to the existing empty state on all four. Relying on the unit test coverage above (header parsing, total-page-count math, jump behavior) until a repository with real issues/PRs is available to confirm the rendered UI itself. +- Next recommended task: Get live confirmation of both this and TASK-027 (or supply/point to a repository with real issues/PRs and an image/CSV file), otherwise continue per the standing direction — no other items remain on the queued list from this session. +- Next owner: Claude, continuing per the standing direction. diff --git a/tasks/027-image-and-csv-viewer.md b/tasks/027-image-and-csv-viewer.md new file mode 100644 index 0000000..974d001 --- /dev/null +++ b/tasks/027-image-and-csv-viewer.md @@ -0,0 +1,143 @@ +# TASK-027: Image viewer (with swipe gallery navigation) and CSV table viewer in the code browser + +Status: implemented; all required checks pass; live device verification pending (no image/CSV +files were found in the repositories browsed live this session — see Known limitations) +Owner: Claude +Reviewer: Codex + +## Outcome + +Two live requests, delivered together: opening an image file in the code browser now shows the +image (not a generic "binary file" fallback), with left/right swipe to move to the previous/next +image in the same folder; opening a `.csv` file shows a scrollable table (rows/columns, first row +styled as a header) instead of raw comma-separated text. Both reuse the tab-swipe/content-scroll +gesture-conflict fix from TASK-025's last follow-up: swiping either an image or a CSV table never +triggers the Files/History tab to change underneath it. + +## Scope + +- Included: `feature/codebrowser/content/ContentClassification.kt` (new `Image`/`Csv` cases, new + `ImageFormat` enum), `feature/codebrowser/content/SourceContentClassifier.kt` (magic-byte image + detection, `.csv`-extension table parsing), `feature/codebrowser/CodeBrowserViewModel.kt` + (`siblingImagePaths` on `File` state, `navigateImage`, `type=issues`-adjacent-but-unrelated — + see TASK-028 for that; this task only touches the image gallery/path plumbing), + `feature/codebrowser/CodeBrowserScreen.kt` (`ImageContent`, `CsvTable`/`CsvRow`/`CsvCell`), + `feature/branchdetail/BranchDetailScreen.kt` (pager-gating condition widened to Image/Csv), + `MainActivity.kt` (`onNavigateImage` wiring), `domain/model/RepositoryModels.kt` is untouched by + this task (see TASK-028 for its `Page.totalCount` addition, done in the same session but a + separate concern). +- Excluded: No new image/CSV editing, no zoom/pan on images (only swipe-to-navigate was asked + for), no header-row auto-detection for CSV (the first row is always styled as a header — see + Design notes), no thumbnail/preview generation, no change to how non-image/non-CSV files render. + +## Design notes + +- **Image detection is by magic bytes, not file extension** (`detectImageFormat` checks the PNG/ + JPEG/GIF/BMP/WebP signatures directly): a mislabeled or extensionless file still renders + correctly if it genuinely is one of these formats, and a renamed non-image file isn't + misclassified just because of its name. It runs *before* the existing NUL-byte-means-Binary + check, since every one of these formats' bytes contain NULs and would otherwise always be caught + as generic Binary first. +- **The sibling-image gallery is built from the directory listing already in hand**, not a second + fetch: `CodeBrowserViewModel.openEntry` captures the current `Directory` state's file entries + (filtered to image extensions — extension-only, since building the gallery list can't afford to + fetch and classify every sibling's bytes just to find out which are images) into the new `File` + state's `siblingImagePaths` at the moment a file is opened. `navigateImage(delta)` then looks up + the current path's index in that list and loads the neighbor, with no bound past either end. + Opening a file *outside* a directory listing (currently only possible via `retry()` after an + error) leaves `siblingImagePaths` empty — swipe navigation is simply unavailable there, a known, + accepted edge case rather than extra plumbing through `Loading`/`Error` states for it. +- **The image swipe gesture is a discrete drag-distance threshold** (`detectHorizontalDragGestures` + + a 120px total-drag check on release), not a continuous scroll position — unlike the diff + view's horizontal scroll, there's no partial-scroll state to render mid-drag, only "did the user + swipe far enough to go to the next/previous image." +- **The CSV table reuses the diff view's manually-driven shared-offset pattern** for its horizontal + scroll (`rememberScrollableState` + `Modifier.offset`, one offset shared across all rows via + `Modifier.scrollable` per row) rather than a plain shared `Modifier.horizontalScroll`. Every CSV + cell here is the same fixed width, so the specific bound-thrashing bug that pattern was built to + avoid (TASK-025's "글자가 춤을 추네") couldn't actually occur with uniform-width cells — but + reusing the already-verified-correct pattern costs nothing and keeps the codebase's one way of + doing synced horizontal scroll, rather than two. +- **No header-row detection for CSV**: the first row is always rendered with a highlighted + background, matching how most spreadsheet tools default a first-row-as-header assumption, since + actually detecting whether a CSV has a header (vs. all-data) isn't reliably possible from the + file alone. +- **Pager-swipe gating (from TASK-025's last follow-up) was widened, not duplicated**: the same + `currentPageShowsScrollableCode` check in `BranchDetailScreen.kt` now also covers + `ContentClassification.Image` and `ContentClassification.Csv`, so the human product owner's + explicit requirement — "탭 전환은 발생하면 안돼" (tab switching must not happen) while viewing + either — is met by the existing mechanism rather than a new one. + +## Acceptance criteria + +- [x] Opening a PNG/JPEG/GIF/BMP/WebP file (detected by magic bytes) shows the decoded image. +- [x] Swiping left/right on an image with siblings in the same folder moves to the next/previous + image in that folder's listing order; swiping past either end does nothing. +- [x] Swiping (or any horizontal drag) on an image or a CSV table never changes the Files/History + tab underneath it. +- [x] Opening a `.csv` file shows a scrollable table with the first row styled as a header; a + non-`.csv` file with comma-separated content is unaffected (still plain text). +- [x] Quoted CSV fields containing commas and escaped (`""`) quotes parse correctly. +- [x] Build, unit tests, and lint pass. +- [ ] Live device confirmation with a real image file and a real `.csv` file — not yet possible + this session; see Known limitations. + +## Required checks + +```text +./gradlew assembleDebug +./gradlew testDebugUnitTest +./gradlew lintDebug +``` + +## Evidence + +- `ContentClassification.kt`: new `Image(sizeBytes, content: SourceContent, format: ImageFormat)` + and `Csv(sizeBytes, rows: List>, isTruncated: Boolean)` cases; `content` reuses + `SourceContent`'s already-correct content-based `equals()`/`hashCode()` instead of holding a raw + `ByteArray` (which would default to reference equality on a `data class`). +- `SourceContentClassifier.kt`: `classify(content, path: RepositoryPath? = null)` — `path` is + optional/defaulted so every pre-existing call site keeps compiling unchanged; `detectImageFormat` + checks PNG/JPEG/GIF/BMP/WebP signatures before the NUL-byte Binary check; CSV parsing + (`renderCsv`/`parseCsv`, a small RFC-4180-ish hand-written parser) only runs when `path` ends in + `.csv`, reusing the same line/character truncation budgets as `renderText`. +- `CodeBrowserViewModel.kt`: `CodeBrowserUiState.File` gained `siblingImagePaths: List = emptyList()`; + `openEntry` captures it from the current `Directory` state's file entries (via new private + `RepositoryPath.hasImageExtension()`); new `navigateImage(delta: Int)`. +- `CodeBrowserScreen.kt`: new `ImageContent` (BitmapFactory decode + Compose `Image` + + drag-threshold swipe), `CsvTable`/`CsvRow`/`CsvCell` (shared-offset horizontal scroll, matching + the diff view's pattern); `FileContent`'s signature grew `path`/`siblingImagePaths`/ + `onNavigateImage` to support both. +- `BranchDetailScreen.kt`: `currentPageShowsScrollableCode`'s page-0 branch now also checks + `ContentClassification.Image`/`ContentClassification.Csv`, not just `Text`. +- New tests: `SourceContentClassifierTest.kt` (PNG/JPEG/GIF/BMP/WebP signature detection, an image + signature's embedded NUL byte not short-circuiting to Binary, oversized-before-image-detection, + `.csv` vs non-`.csv` path handling, no-path-given fallback to Text, quoted-field/escaped-quote + parsing, row-count truncation); `CodeBrowserViewModelTest.kt` (gallery capture in listing order + excluding a non-image sibling, `navigateImage` moving and stopping at both ends, `navigateImage` + no-op when opened outside a directory listing). +- `./gradlew compileDebugKotlin compileDebugUnitTestKotlin`: BUILD SUCCESSFUL. +- `./gradlew testDebugUnitTest`: BUILD SUCCESSFUL, all unit tests passing (including the new ones + above). +- `./gradlew assembleDebug lintDebug`: BUILD SUCCESSFUL, 0 lint errors. +- Live device: build installed over wireless `adb`. Navigated several of the human product owner's + actual repositories looking for an existing image or `.csv` file to confirm the rendering + end-to-end; none were found among the repositories browsed this session (see Known limitations). + +## Known limitations + +- **Not yet confirmed against a real image or CSV file on the live device.** Every repository + browsed this session for a suitable test file didn't have one readily reachable. The + classification and rendering logic is covered thoroughly by unit tests instead (magic-byte + detection, gallery navigation, CSV parsing), but a live human confirmation of the actual pixels + on screen — decoded image quality, table cell alignment/scroll feel — is still outstanding. +- The CSV table's column width is a fixed 120dp per cell, not sized to content — a column with + very short values wastes horizontal space, and a column with very long values still clips at + 120dp (readable via horizontal scroll, but not auto-fit). Not raised as a concern by the human + product owner; flagged here in case it's worth revisiting once seen live. +- `navigateImage` after a `retry()`-recovered error loses gallery position (see Design notes) — + accepted, not treated as a defect. + +## Review findings + +None yet — pending Codex/human review. diff --git a/tasks/028-issues-prs-numbered-pagination.md b/tasks/028-issues-prs-numbered-pagination.md new file mode 100644 index 0000000..27611c7 --- /dev/null +++ b/tasks/028-issues-prs-numbered-pagination.md @@ -0,0 +1,124 @@ +# TASK-028: Numbered, jump-to-page pagination for the Issues/PR list + +Status: implemented; all required checks pass; live device verification pending (no repository +with existing issues/PRs was found this session — see Known limitations) +Owner: Claude +Reviewer: Codex + +## Outcome + +The Issues/PR list now shows up to 5 numbered page buttons (tap to jump directly) plus previous/ +next, and a `[ page / total ]`-style input field to type an exact page number and jump straight +there — replacing the plain previous/next-only footer, on this screen only. Per the earlier +AskUserQuestion resolution in this session ("이 기능은 전체 커밋/이슈 수를 알 수 있는 화면에만 +적용"), every other paginated list in the app (repositories, history, notifications) keeps plain +previous/next, since Gitea's endpoints for those don't report a total count — Issues/PRs do. + +## Scope + +- Included: `domain/model/RepositoryModels.kt` (`Page.totalCount: Int? = null`), + `data/workitems/GiteaWorkItemsApi.kt` (`X-Total-Count` header parsing; `listIssues` now sends + `type=issues` so both the returned items and that header exclude pull requests server-side), + `feature/workitems/WorkItemsViewModel.kt` (`totalPages` on `IssueList`/`PullRequestList`, + `goToPage`), `feature/workitems/WorkItemsScreen.kt` (`NumberedPageFooter`, page-window logic, + the `[ input / total ]` jump field), `MainActivity.kt` (`onGoToPage` wiring). +- Excluded: No change to repository/history/notification pagination (deliberately kept as plain + previous/next — see Outcome). No change to how issue/PR *detail* screens work, only the list. + +## Design notes + +- **`type=issues` is a real, documented Gitea query parameter, not a guess**: verified directly + against Gitea's own source (`routers/api/v1/repo/issue.go`'s `ListIssues` swagger doc comment + lists `type` with `enum: [issues, pulls]`) before relying on it — this project's established + convention for anything not obviously provable from the client alone. The class-level doc + comment on `GiteaWorkItemsApi` previously explained *not* using this parameter as a deliberate + choice against "a guessed, unverified" one; that reasoning no longer applies now that it's + verified, so the comment was rewritten rather than left stale. The existing client-side + `pullRequest == null` filter is kept anyway, as defense in depth. +- **Why `type=issues` specifically matters for this task**: without it, Gitea's issue-list endpoint + returns issues *and* pull requests mixed together (a PR is internally an issue with a non-null + `pull_request` field), and its `X-Total-Count` header would count both — producing a total-page + count that overstates how many pages of genuine issues actually exist. Sending `type=issues` + makes the count (and the returned items) issues-only at the source. +- **`Page.totalCount` is optional and defaulted** (`totalCount: Int? = null` on both the class + and `Page.of(...)`), specifically so none of the other 9+ existing `Page.of(...)` call sites + (repositories, branches, tags, commits, notifications, comments, reviews — none of which have a + total count available) needed to change. +- **The numbered-pagination UI falls back to the plain previous/next footer whenever `totalPages` + is null** (e.g. a hypothetical Gitea instance old enough to not send the header) — a defensive + fallback, not the expected path, but one that avoids ever showing a broken or missing page count + instead of just quietly degrading to what every other list already does. +- **The 5-button page window slides, centered on the current page**, clamped to stay within + `[1, totalPages]` — page 1 of a large list shows pages 1–5 (matching "1페이지에선 5까지 + 보여줌" exactly), and the window keeps recentering as the user navigates rather than freezing + once it nears either end. +- **No icon library dependency was added** for the "🔍"-style jump control asked for — this + project has never used `androidx.compose.material.icons` (every other screen uses plain text + labels, e.g. "다시 시도" instead of a retry icon), so the jump button uses a plain text label + ("이동") consistent with that, rather than pulling in `material-icons-core` for one button. + +## Acceptance criteria + +- [x] The Issues list and the PR list each show up to 5 numbered page buttons, previous/next, and + a page-jump input field, when the server reports a total count. +- [x] Tapping a numbered page button jumps directly to that page. +- [x] Typing a page number and submitting (IME "Go" or the "이동" button) jumps to that page, + clamped to `[1, totalPages]`. +- [x] `X-Total-Count` is correctly excluded of pull requests for the Issues list (via `type=issues`) + and reflects genuine issue count. +- [x] A fractional last page rounds up (45 items at page size 20 is 3 pages, not 2). +- [x] Every other paginated list (repositories, history, notifications) is unchanged — still plain + previous/next. +- [x] Build, unit tests, and lint pass. +- [ ] Live device confirmation against a repository with enough real issues/PRs to see the numbered + UI populated — not yet possible this session; see Known limitations. + +## Required checks + +```text +./gradlew assembleDebug +./gradlew testDebugUnitTest +./gradlew lintDebug +``` + +## Evidence + +- `RepositoryModels.kt`: `Page` gained `totalCount: Int? = null`; `Page.of(...)` validates it's + non-negative when present. +- `GiteaWorkItemsApi.kt`: `listIssues` now requests `type=issues`; `toPage` parses `X-Total-Count` + (case-insensitive header lookup, matching the existing `X-HasMore` pattern) into `Page.totalCount`. +- `WorkItemsViewModel.kt`: `IssueList`/`PullRequestList` gained `totalPages: Int? = null`, computed + via a new private `Page.totalPages(pageSize)` extension (`ceil(total / pageSize)`, at least + 1); new `goToPage(page: Int)`. +- `WorkItemsScreen.kt`: `PageFooter` now branches on `totalPages` — null falls back to the original + previous/next row unchanged; non-null renders the new `NumberedPageFooter` (page-window buttons + + `[ input / total ]` jump row) instead. New private `pageWindow(current, totalPages, windowSize=5)`. +- New tests: `GiteaWorkItemsApiTest.kt` (`type=issues` sent, `X-Total-Count` parsed for both issues + and pull requests, absent header leaves `totalCount` null), `WorkItemsViewModelTest.kt` (45 + items/page size 20 rounds up to 3 pages, missing total count leaves `totalPages` null, `goToPage` + jumps directly to the requested page), `RepositoryModelsTest.kt` (`totalCount` defaults to null, + is exposed when given, rejects a negative value). +- `./gradlew compileDebugKotlin compileDebugUnitTestKotlin`: BUILD SUCCESSFUL. +- `./gradlew testDebugUnitTest`: BUILD SUCCESSFUL, all unit tests passing (including the new ones + above). +- `./gradlew assembleDebug lintDebug`: BUILD SUCCESSFUL, 0 lint errors. +- Live device: build installed over wireless `adb`. Checked the Issues and Pull Request tabs on + four of the human product owner's actual repositories; every one had zero issues and zero pull + requests, so the numbered-pagination UI itself was never actually populated with real data this + session (see Known limitations) — confirmed instead that the screen doesn't crash and correctly + shows the existing empty state on all four. + +## Known limitations + +- **Not yet confirmed against a real repository with enough issues/PRs to populate the numbered UI + with real data.** Every repository checked live this session had zero issues and zero pull + requests. The header-parsing, total-page-count, and jump-to-page logic are covered by unit tests + instead, but the actual rendered page-button row and jump field haven't been seen with real + multi-page data yet. +- The page-jump text field accepts any positive integer typed, including one exceeding + `totalPages` — it's clamped before being submitted (`coerceIn(1, totalPages)`), so this can't + request an out-of-range page, but the field itself doesn't visually reject the keystroke. + +## Review findings + +None yet — pending Codex/human review.