diff --git a/.claude/skills/verify-t3term/features/pull-request-code.md b/.claude/skills/verify-t3term/features/pull-request-code.md index 96c80b8..67f6306 100644 --- a/.claude/skills/verify-t3term/features/pull-request-code.md +++ b/.claude/skills/verify-t3term/features/pull-request-code.md @@ -5,12 +5,13 @@ - **The tab.** Where the detail's `capabilities.diff` is true, the row above a link's details reads `← List Summary Timeline Code`. → or `l` on the Timeline, or a click on `Code`, shows it, and ← or `h` goes back to the Timeline. The status line reads `→ code · ← summary` on the Timeline and `← timeline` on the Code. A detail read again without `diff` takes the tab away, and a reader on the Code lands on the Summary. - **Reading the diff.** The first time the Code shows after the details open, t3term POSTs `/api/pull-requests/diff` with the link's project, host, repository and number and no cursor. `Loading pull request diff…` shows until the first slice comes. Once a frame shows the end of the files within two pages of the view, t3term reads the next slice with the cursor the last one gave. One slice is read at a time. Nothing reads the diff before the Code shows. Going to another tab and back within the minute reads nothing, and coming back a minute or more after the first slice came reads it again. - **The count and the note.** The count reads `5 files+` while more slices are left, with `Loading more files…` under the files while the next one is read, then `8 files`. `The host withheld part of this diff, such as a binary file or a change too large to show.` shows in yellow under it when a slice says the host left something out. +- **Commits.** Where the change has commits, `› All commits` sits above the count. Enter or a click opens the list under it: `✓ All commits`, then each commit's headline with the first seven characters of its id at the end, newest first, and `Show more (13 left)` while more than 10 are left. ✓ marks the diff shown. Picking a commit closes the list, reads that commit's own diff from its first slice with every file folded, and the row reads `› Reuse the parsed config on reload · 9dc565a`. Under a commit's diff every review thread goes to the list off the diff. `All commits` reads the whole change again. A read of the conversation that no longer lists the commit goes back to the whole change. - **Files.** Each file starts folded: `› M src/config/reader.rs +7 -5`, with `A` green, `D` red, `R` blue and `M` yellow, and the counts in green and red. A rename shows both paths, `› R src/config/source.rs → src/config/sources.rs`, with the counts on the next row where they don't fit. A file whose hunks the host left out takes the host's counts, as in `› M Cargo.lock +40 -12`. - **Order.** Each slice's files show in the desktop's reading order, not the patch's. Source files come first, each after the changed files it imports and otherwise by path. Tests come next, each after the source file whose name it has, then lockfiles, snapshots and files under `dist`, `build`, `vendor`, `__generated__` or `__snapshots__`, by path. A later slice never moves the files above it. So the first slice reads `assets/config-flow.png`, `src/config/reader.rs`, `src/config/schema.rs`, the rename and `tests/config_reload.rs`, and the second `docs/config.md`, `src/legacy/reader_v1.rs` and `Cargo.lock`. - **Unfolding.** Enter on the chosen file, or a click on it, turns `›` into `⌄` and shows its lines. Each line has its number in grey, the new one or the old for a removed line, then `-` and the line in red, `+` and the line in green, or a space and the line. `⋯ 14 unmodified lines` comes before a hunk that doesn't start at line 1. A line that wraps leaves the number and the mark blank on its later rows, and a tab stops every 4 columns. A binary file reads `A binary file, whose changes have no lines to show.` and a file the host withheld `The host withheld this file's changes.` Enter or a click again folds the file. - **Review threads.** A thread on a line of an unfolded file shows under that line, behind a grey bar, where the line has that number on the thread's side. Its first row reads `⌄ Open · 4 comments`, with `Resolved` in green and `· outdated` where the host says so. A thread starts open unless it is resolved. Open, it shows each comment with its author and age. Where the host has another page, `Load more comments` follows. It reads the page through `pullRequests.threadComments`, reads `Loading more comments…` until the page comes, then shows the page's comments after the thread's, each once, and goes once the thread is whole. A failed page shows `t3term couldn't read more comments.` and the reason above the button, which reads the same page again. Where the host counts comments it has no page for, `2 more comments on the host.` shows instead. The pages stay with the thread when the conversation is read again. On a row with both numbers, a thread on the old side comes before one on the new. Enter or a click on the first row opens or closes it, and it stays so across reads. A folded file shows none of its threads. - **Off the diff.** Threads on a line no file read so far shows go to a list above the files that starts closed: `› 1 conversation not on the diff read so far` while slices are left, then `› 1 conversation not on the current diff`. Open, it lists each path once, then `Line 12` and the thread. -- **Choosing.** Once a file, a thread, the list or `↻ Retry` is in view, ↓ chooses the next one and ↑ the one before, and the status line leads with what Enter does: `Enter expand file`, `Enter collapse file`, `Enter show conversation`, `Enter hide conversation`, `Enter load more comments`, `Enter show conversations off the diff` or `Enter retry`. A click or Enter on `Load more comments` or `↻ Retry` lets go of the pick, since the row turns into a loading one. Where none is in view, they scroll. +- **Choosing.** Once a file, a thread, the list or `↻ Retry` is in view, ↓ chooses the next one and ↑ the one before, and the status line leads with what Enter does: `Enter expand file`, `Enter collapse file`, `Enter choose commit`, `Enter hide commits`, `Enter show all commits`, `Enter show commit 9dc565a`, `Enter show more commits`, `Enter show conversation`, `Enter hide conversation`, `Enter load more comments`, `Enter show conversations off the diff` or `Enter retry`. A click or Enter on `Load more comments` or `↻ Retry` lets go of the pick, since the row turns into a loading one. Where none is in view, they scroll. - **Reading again.** A new refresh number, `r` and a detail whose `updatedAt` changed each read the first slice again. The files read before stay, each with its fold, until the new slice replaces them, and the count goes back to `5 files+` until the end of the files comes near again. After `r` the diff waits for the refresh number `pullRequests.invalidate` raises, or for the end of the read that sent it, so T3 has forgotten the diff it held. - **Failures.** A first slice that fails shows `Couldn't read this diff: GitHub took too long to answer.` alone. Showing the Code again, or `r`, reads it again, and the failure stays until the files come. A later slice that fails shows `t3term couldn't read the rest of this diff.`, the reason and `↻ Retry` under the files, and Retry reads that slice again. A read again of the first slice that fails keeps the files under `May be out of date: t3term couldn't read the diff again.` and the reason. @@ -20,7 +21,7 @@ Open a thread, open its details (Alt+B), press → for `Pull requests`, choose a ## Driving it with t3v -Scenario `links` at 100x30. The diff route answers at once with two slices. The first holds `src/config/reader.rs`, `src/config/schema.rs`, the rename `src/config/source.rs → src/config/sources.rs`, the binary `assets/config-flow.png` and the new `tests/config_reload.rs`, with `truncated` and the cursor `s2`. The second holds `docs/config.md`, the deleted `src/legacy/reader_v1.rs` and a `Cargo.lock` with a header and no hunks, which the host counted at +40 -12. Files in the run directory change the reads: `diff-slow` holds each slice 4 s, `diff-fail` fails the first slice and `diff-fail-rest` the second with `GitHub took too long to answer.`, `diff-off` makes the detail say the host gives no diff, `diff-ts` serves a TypeScript diff in one slice, and `diff-threads` gives #7's conversation four review threads: a resolved one on removed line 26 of `src/config/reader.rs`, an open one on added line 28 whose host counts 4 comments where T3 sent 2 and has one more page, an outdated one on added line 45 of `src/config/schema.rs`, and one on line 12 of `src/config/watch.rs`, which the diff doesn't touch. The page comes 1.5 s late with the last of the 2 comments again and 2 more, and `comments-fail` fails it. The TypeScript diff's patch has `src/settings/panel.tsx`, which imports `./store` and `../lib/format`, then `src/settings/panel.test.tsx`, `src/settings/store.ts`, which imports `../lib/format`, `pnpm-lock.yaml` and the new `src/lib/format.ts`. The server logs each read of the diff in events.jsonl as `pr.diff` with its cursor, and `reads` lists those cursors, `null` for the first slice. +Scenario `links` at 100x30. The diff route answers at once with two slices. The first holds `src/config/reader.rs`, `src/config/schema.rs`, the rename `src/config/source.rs → src/config/sources.rs`, the binary `assets/config-flow.png` and the new `tests/config_reload.rs`, with `truncated` and the cursor `s2`. The second holds `docs/config.md`, the deleted `src/legacy/reader_v1.rs` and a `Cargo.lock` with a header and no hunks, which the host counted at +40 -12. Files in the run directory change the reads: `diff-slow` holds each slice 4 s, `diff-fail` fails the first slice and `diff-fail-rest` the second with `GitHub took too long to answer.`, `diff-off` makes the detail say the host gives no diff, `diff-ts` serves a TypeScript diff in one slice, and `diff-threads` gives #7's conversation four review threads: a resolved one on removed line 26 of `src/config/reader.rs`, an open one on added line 28 whose host counts 4 comments where T3 sent 2 and has one more page, an outdated one on added line 45 of `src/config/schema.rs`, and one on line 12 of `src/config/watch.rs`, which the diff doesn't touch. The page comes 1.5 s late with the last of the 2 comments again and 2 more, and `comments-fail` fails it. The TypeScript diff's patch has `src/settings/panel.tsx`, which imports `./store` and `../lib/format`, then `src/settings/panel.test.tsx`, `src/settings/store.ts`, which imports `../lib/format`, `pnpm-lock.yaml` and the new `src/lib/format.ts`. #7 has two commits, `Split the config reader` (1f3e2a90) and `Reuse the parsed config on reload` (9dc565a0) an hour old, and the route answers each with its own one slice: schema.rs for the first, reader.rs and the new test for the second. The server logs each read of the diff in events.jsonl as `pr.diff` with its cursor and commit, and `reads` lists those cursors, `null` for the first slice. ```bash $T3V start --cols 100 --rows 30 @@ -41,30 +42,32 @@ $T3V keys Right && $T3V wait --text "Loading pull request diff…" --text "← t $T3V capture loading-100x30 $T3V wait --text "5 files+" --text "The host withheld part of this diff, such as a" --text "› A tests/config_reload.rs +8 -0" --text "Loading more files…" $T3V capture more-100x30 -$T3V wait --text "8 files" --text "› M docs/config.md +2 -1" --text "› D src/legacy/reader_v1.rs +0 -4" --gone "Loading more files…" --idle 300 +$T3V wait --text "› All commits" --text "8 files" --text "› M docs/config.md +2 -1" --gone "Loading more files…" --idle 300 rm "$RUN/diff-slow" [ "$(reads)" = '[null,"s2"]' ] C=$($T3V capture files-100x30); S=$(echo "$C" | grep 'ans$'); T=$(echo "$C" | grep 'txt$') -# Each slice in reading order: source by path, as none imports another, then the test. Cargo.lock is below the view. -[ "$(grep -o '› [AMDR] [^ ]*' "$T" | paste -sd' ' -)" = '› M assets/config-flow.png › M src/config/reader.rs › M src/config/schema.rs › R src/config/source.rs › A tests/config_reload.rs › M docs/config.md › D src/legacy/reader_v1.rs' ] -grep -q '38;2;255;185;0mM.\[38;2;245;245;245m src/config/reader.rs .\[38;2;0;212;146m+7.\[38;2;245;245;245m .\[38;2;255;100;103m-5' "$S" && grep -q '38;2;43;127;255mR.\[38;2;245;245;245m src/config/source.rs → src/config/sources.rs' "$S" && grep -q '38;2;0;212;146mA.\[38;2;245;245;245m tests/config_reload.rs' "$S" && grep -q '38;2;255;100;103mD.\[38;2;245;245;245m src/legacy/reader_v1.rs' "$S" && grep -q '38;2;255;185;0mThe host withheld part' "$S" +# Each slice in reading order: source by path, as none imports another, then the test, then the second slice. +[ "$(grep -o '› [AMDR] [^ ]*' "$T" | head -6 | paste -sd' ' -)" = '› M assets/config-flow.png › M src/config/reader.rs › M src/config/schema.rs › R src/config/source.rs › A tests/config_reload.rs › M docs/config.md' ] +grep -q '38;2;255;185;0mM.\[38;2;245;245;245m src/config/reader.rs .\[38;2;0;212;146m+7.\[38;2;245;245;245m .\[38;2;255;100;103m-5' "$S" && grep -q '38;2;43;127;255mR.\[38;2;245;245;245m src/config/source.rs → src/config/sources.rs' "$S" && grep -q '38;2;0;212;146mA.\[38;2;245;245;245m tests/config_reload.rs' "$S" && grep -q '38;2;255;185;0mThe host withheld part' "$S" -# ↓ chooses the first file, the binary, and ↓ again the reader. Enter unfolds it: numbers in grey, removed lines red, added green. -$T3V keys Down Down && $T3V wait --text "Enter expand file" +# ↓ chooses the commits row, ↓ again the first file, the binary, and ↓ once more the reader. Enter unfolds it: numbers in grey, removed lines red, added green. +$T3V keys Down Down Down && $T3V wait --text "Enter expand file" $T3V keys Enter && $T3V wait --text "⌄ M src/config/reader.rs +7 -5" --text " 4 - use crate::config::Source;" --text " 4 + use crate::config::schema::Schema;" --text "Enter collapse file" S=$($T3V capture reader-100x30 | grep 'ans$') grep -q '38;2;129;129;129m 4 .\[38;2;255;100;103m- use crate::config::Source;' "$S" && grep -q '38;2;129;129;129m 4 .\[38;2;0;212;146m+ use crate::config::schema::Schema;' "$S" && grep -q '38;2;129;129;129m 1 .\[38;2;245;245;245m use std::fs;' "$S" # The unmodified lines before its second hunk, a tab stopping at 4 columns, and a wrapped line leaving the number and the mark blank. $T3V keys PgDn && $T3V wait --text "⋯ 14 unmodified lines" --text "28 - Ok(config)" --text "33 + // A reload reads and checks the file a" --text " s a load does, so a file edited on disk since t" -$T3V keys Home && $T3V keys Down Down && $T3V wait --text "Enter collapse file" +$T3V keys Home && $T3V keys Down Down Down && $T3V wait --text "Enter collapse file" $T3V keys Enter && $T3V wait --text "› M src/config/reader.rs +7 -5" --gone "⋯ 14 unmodified lines" # A click unfolds the others: the binary, the rename and the new file with its tabs. -$T3V click --text "tests/config_reload.rs" && $T3V wait --text "⌄ A tests/config_reload.rs +8 -0" --text "4 + fn a_reload_checks_the_schema() {" +$T3V click --text "tests/config_reload.rs" && $T3V wait --text "⌄ A tests/config_reload.rs +8 -0" --text "3 + #[test]" $T3V click --text "src/config/source.rs" && $T3V wait --text "⌄ R src/config/source.rs → src/config/sources.rs" --text "1 - /// Where a config comes from." --text "1 + /// Where a config comes from: a file, the envir" $T3V click --text "assets/config-flow.png" && $T3V wait --text "⌄ M assets/config-flow.png" --text "A binary file, whose changes have no lines to show." $T3V capture kinds-100x30 -$T3V keys End && $T3V wait --text "› M Cargo.lock +40 -12" --text "5 + let reader = Reader::new(\"tests/newer.toml\")" +$T3V keys End && $T3V wait --text "› M Cargo.lock +40 -12" --text "› D src/legacy/reader_v1.rs +0 -4" --text "5 + let reader = Reader::new(\"tests/newer.toml\")" +S=$($T3V capture end-100x30 | grep 'ans$') +grep -q '38;2;255;100;103mD.\[38;2;245;245;245m src/legacy/reader_v1.rs' "$S" $T3V click --text "Cargo.lock" && $T3V wait --text "⌄ M Cargo.lock +40 -12" $T3V keys End && $T3V wait --text "The host withheld this file's changes." @@ -164,16 +167,38 @@ mark $T3V refresh && settle '[null,null]' $T3V wait --text "⌄ Resolved · 1 comment" --text "› Open · 4 comments" --idle 300 # The page stays with the thread. -$T3V click --text "Open · 4 comments" && $T3V wait --text "⌄ Open · 4 comments" --text "Should reload check the schema too?" +$T3V click --text "Open · 4 comments" && $T3V wait --text "⌄ Open · 4 comments" --text "fixture-reviewer" $T3V keys PgDn && $T3V wait --text "Added a_reload_checks_the_schema." --gone "Load more comments" --idle 300 -# ↓ from the top chooses the list, and Enter opens it: each path once, then the line and the thread. -$T3V keys Home Down && $T3V wait --text "Enter show conversations off the diff" +# ↓ from the top chooses the commits row, and ↓ again the list. Enter opens it: each path once, then the line and the thread. +$T3V keys Home Down && $T3V wait --text "Enter choose commit" +$T3V keys Down && $T3V wait --text "Enter show conversations off the diff" $T3V keys Enter && $T3V wait --text "⌄ 1 conversation not on the diff read so far" --text "src/config/watch.rs" --text "Line 12" --text "The watcher still calls load_unchecked." --text "Enter hide conversations off the diff" $T3V capture off-diff-100x30 $T3V keys Enter && $T3V wait --text "› 1 conversation not on the diff read so far" --gone "Line 12" # A folded file's threads don't show. Below, schema.rs has an outdated thread on added line 45. $T3V click --text "src/config/reader.rs" && $T3V wait --text "› M src/config/reader.rs +7 -5" --gone "Resolved · 1 comment" --gone "Open · 4 comments" $T3V keys PgDn && $T3V wait --text "⌄ Open · 1 comment · outdated" --text "A config with no version reads as 0 here." + +# The commits row opens the list, newest first. Picking a commit reads its own diff from the first slice, with its files folded and every thread off the diff. +$T3V keys Home && $T3V wait --text "› All commits" +$T3V click --text "› All commits" && $T3V wait --text "⌄ All commits" --text "✓ All commits" --text "Reuse the parsed config on reload" --text "Split the config reader" --text "Enter hide commits" +C=$($T3V capture commits-100x30); T=$(echo "$C" | grep 'txt$') +[ "$(grep -o -e '✓ All commits' -e 'Reuse the parsed config on reload *9dc565a' -e 'Split the config reader *1f3e2a9' "$T" | sed 's/ */ /g' | paste -sd'|' -)" = '✓ All commits|Reuse the parsed config on reload 9dc565a|Split the config reader 1f3e2a9' ] +$T3V keys Down && $T3V wait --text "Enter show all commits" +$T3V keys Down && $T3V wait --text "Enter show commit 9dc565a" +mark +$T3V keys Enter && $T3V wait --text "› Reuse the parsed config on reload · 9dc565a" --text "› M src/config/reader.rs" --text "› A tests/config_reload.rs" --text "› 4 conversations not on the current diff" --gone "Split the config reader" --gone "src/config/schema.rs" --idle 300 +[ "$(jq -s -c '[.[] | select(.event == "pr.diff") | [.cursor, .commit]] | .['"$M"':]' "$E/events.jsonl")" = '[[null,"9dc565a0"]]' ] +$T3V capture commit-100x30 +$T3V click --text "Reuse the parsed config on reload" && $T3V wait --text "⌄ Reuse the parsed config on reload · 9dc565a" --text "Split the config reader" +mark +$T3V click --text "Split the config reader" && $T3V wait --text "› Split the config reader · 1f3e2a9" --text "src/config/schema.rs" --gone "tests/config_reload.rs" --idle 300 +[ "$(jq -s -c '[.[] | select(.event == "pr.diff") | [.cursor, .commit]] | .['"$M"':]' "$E/events.jsonl")" = '[[null,"1f3e2a90"]]' ] +# All commits reads the whole change again from its first slice, with every file folded, so the end is near and the second slice comes at once. Only the thread on watch.rs stays off the diff. +$T3V click --text "Split the config reader" && $T3V wait --text "✓ Split the config reader" +mark +$T3V click --text " All commits" && $T3V wait --text "› All commits" --text "8 files" --text "› 1 conversation not on the current diff" --text "› M src/config/reader.rs +7 -5" --idle 300 +[ "$(jq -s -c '[.[] | select(.event == "pr.diff") | [.cursor, .commit]] | .['"$M"':]' "$E/events.jsonl")" = '[[null,null],["s2",null]]' ] rm "$RUN/diff-threads" # Another thread's link, whose first slice fails: the failure alone. Showing the Code again reads it again, and so does opening the details again. @@ -197,7 +222,7 @@ $T3V keys Right Right && $T3V wait --text "8 files" --text "› M src/config/rea # At 80x24 every row still fits the pane. $T3V resize 80 24 && $T3V wait --text "8 files" --idle 300 -$T3V keys Down Down Enter && $T3V wait --text "⌄ M src/config/reader.rs +7 -5" --text " 4 + use crate::config::schema::Schema;" --text "Enter collapse file" +$T3V keys Down Down Down Enter && $T3V wait --text "⌄ M src/config/reader.rs +7 -5" --text " 4 + use crate::config::schema::Schema;" --text "Enter collapse file" $T3V capture reader-80x24 $T3V status --json | jq -e '.scopeViolations == 0 and .unknownRpcs == 0 and .fixtureErrors == 0' ``` @@ -212,5 +237,6 @@ $T3V status --json | jq -e '.scopeViolations == 0 and .unknownRpcs == 0 and .fix - `r` does nothing until the read of the detail and the conversation is done, 3 s after it starts. Wait for `r refresh` in the status line before each press. - A first slice that failed shows its failure, not `Loading pull request diff…`, while it is read again. - Switching threads drops the read and its Code, so coming back reads the diff from the start with every file folded. Closing the dialog with Esc and opening it again keeps them. +- The fixture's links have at most three commits, so `Show more` never shows, and the route answers an unknown commit with an empty patch the TUI never asks for. The force-push that drops a picked commit isn't served either. - The recipe doesn't wait out the minute after which showing the Code reads it again, though it checks that showing it again reads a first slice that failed, and the fixture can't serve an empty diff (`This pull request has no file changes.`) or one the parser can't split. - Clicks land on the rows the last frame drew. Click the lowest file first when unfolding several, so the rows above don't move. diff --git a/README.md b/README.md index 43096ee..262d84f 100644 --- a/README.md +++ b/README.md @@ -99,9 +99,9 @@ The TUI follows the desktop app closely, because muscle memory is worth more tha | Sidebar | ↑/↓ or j/k select (↓ past the last thread scrolls on to the headings under it), Enter open, w, d and e open or close the Working, Done and Settled shelves, [ and ] narrow or widen the sidebar, 0 back to its usual width, q quit | | Composer | Enter send (queues if the thread is busy), Alt+Enter or Ctrl+J newline, Ctrl+R swap in an unsent message, Esc to transcript | | Transcript | ↑/↓ scroll, g/G top/bottom, t open or close every row of tool calls, p expand or collapse the first long plan in view, w open or close the worktree setup's details while its card is in view, Enter compose, Esc, ← or h sidebar, showing it if hidden | -| Thread details | → or l the Pull requests tab, ← or h back a step, Esc close the dialog or leave the pane. On the Thread tab ↑/↓ or j/k choose a Lineage row, or scroll when there is none to choose, Enter open the chosen row, PgUp/PgDn page, Home/g top, End/G bottom. On the Pull requests tab the same keys select a link, and Enter shows its details in the list's place, where they scroll and Enter or ← shows the list again. Once the host has answered, → or l shows the details' Timeline, then their Code where the host gives a diff, and ← or h goes back a tab. In the details ↑/↓ also choose a button or group of the Comments section or the Timeline, or a file, a review thread, a thread's Load more comments, the list of conversations off the diff or Retry on the Code, once it is in view, and Enter does what the chosen one does. r reads the pull request from its host again | +| Thread details | → or l the Pull requests tab, ← or h back a step, Esc close the dialog or leave the pane. On the Thread tab ↑/↓ or j/k choose a Lineage row, or scroll when there is none to choose, Enter open the chosen row, PgUp/PgDn page, Home/g top, End/G bottom. On the Pull requests tab the same keys select a link, and Enter shows its details in the list's place, where they scroll and Enter or ← shows the list again. Once the host has answered, → or l shows the details' Timeline, then their Code where the host gives a diff, and ← or h goes back a tab. In the details ↑/↓ also choose a button or group of the Comments section or the Timeline, or the commit list's row or entries, a file, a review thread, a thread's Load more comments, the list of conversations off the diff or Retry on the Code, once it is in view, and Enter does what the chosen one does. r reads the pull request from its host again | -The wheel scrolls the transcript and the sidebar. Clicking ◧ at the top left of the conversation hides or shows the sidebar, clicking ◨ at the top right opens or closes the thread's details, and dragging the sidebar's right edge resizes it. Clicking a thread opens it, clicking the Working, Done or Settled heading opens or closes that shelf, clicking a chip under the composer opens its menu, clicking the top row of the tasks drawer opens or closes its list, clicking × on a provider or error banner dismisses it, clicking elsewhere on a long notice or error opens or closes it, and clicking a row of tool calls opens that row. Clicking the top edge of a long plan, or its Expand plan or Collapse plan button, expands or collapses that plan. Clicking a worktree setup card's Details row opens or closes its details. Clicking a tab of the thread's details shows that tab. On the Thread tab, clicking a Lineage row opens that thread, shows more rows or opens or closes Previous agents. On the Pull requests tab, clicking a linked pull request selects it, or shows its details when it is selected already, and clicking `← List` above those details shows the list again. Clicking Summary, Timeline or Code beside it shows that tab. On the Code, clicking a file unfolds or folds it, clicking a review thread's first row or the list of conversations off the diff opens or closes it, clicking a thread's Load more comments reads its next page of comments, and clicking Retry reads the rest of the diff again. +The wheel scrolls the transcript and the sidebar. Clicking ◧ at the top left of the conversation hides or shows the sidebar, clicking ◨ at the top right opens or closes the thread's details, and dragging the sidebar's right edge resizes it. Clicking a thread opens it, clicking the Working, Done or Settled heading opens or closes that shelf, clicking a chip under the composer opens its menu, clicking the top row of the tasks drawer opens or closes its list, clicking × on a provider or error banner dismisses it, clicking elsewhere on a long notice or error opens or closes it, and clicking a row of tool calls opens that row. Clicking the top edge of a long plan, or its Expand plan or Collapse plan button, expands or collapses that plan. Clicking a worktree setup card's Details row opens or closes its details. Clicking a tab of the thread's details shows that tab. On the Thread tab, clicking a Lineage row opens that thread, shows more rows or opens or closes Previous agents. On the Pull requests tab, clicking a linked pull request selects it, or shows its details when it is selected already, and clicking `← List` above those details shows the list again. Clicking Summary, Timeline or Code beside it shows that tab. On the Code, clicking the row above the files opens or closes the list of commits, clicking a commit or All commits in it shows that diff, clicking Show more lists more commits, clicking a file unfolds or folds it, clicking a review thread's first row or the list of conversations off the diff opens or closes it, clicking a thread's Load more comments reads its next page of comments, and clicking Retry reads the rest of the diff again. On macOS the Alt keys need Option to send Meta: "Use Option as Meta key" in Terminal, "Esc+" for the Option key in iTerm2, or `macos-option-as-alt = true` in Ghostty. diff --git a/docs/pull-request-panel.md b/docs/pull-request-panel.md index fb8147b..8f91bb3 100644 --- a/docs/pull-request-panel.md +++ b/docs/pull-request-panel.md @@ -101,6 +101,7 @@ Where the detail's `capabilities.diff` is true, the details have a third tab, Co | --- | --- | --- | | Reading | The tab mounts the first time it shows and stays mounted while the panel shows the pull request (PullRequestDetailPanel.tsx:2784-2798). Its query reads the first slice, and reads it again when the tab mounts with an answer 60 seconds old. A sentinel 240 pixels under the files reads the next slice once it comes into view (PullRequestCodeTab.tsx:607-629). | The first slice is read the first time the Code shows after the details open, and read again each time the Code shows where it came a minute or more ago or failed. The next slice is read once a frame shows the end of the files within two pages of the view. One slice is read at a time. | | Reading again | `codeRefreshToken` (PullRequestDetailPanel.tsx:832) moves with each refresh number, a person's Refresh and a detail whose `updatedAt` changed (836-847), and the tab reads the first slice again | The same three read the first slice again. After `r`, the read waits until T3's refresh number moves or the read that sent `pullRequests.invalidate` is done, so T3 has forgotten the diff it held. The files read before show until the new ones come, and a file keeps its fold. A read again that fails keeps them under `May be out of date: t3term couldn't read the diff again.` and the reason. | +| Commits | Where the change has commits, the toolbar's menu says `All commits` or the picked commit's headline. It lists `All commits`, then the commits newest first by when each was committed, 10 at a time with `Show more (13 left)`, each with its id's first seven characters (PullRequestCodeTab.tsx:683-692, 1063-1119). Picking one reads its own diff from its first slice, folds the files and closes the list off the diff (268-277), and places no thread on its lines, since a commit counts its lines apart from the change (454-458). A commit the change no longer lists goes back to All commits (1050-1062). The query of a diff picked before stays alive for 5 minutes once nothing shows it, so a read of it under way runs on and its answer is kept (packages/client-runtime/src/state/runtime.ts:534 and 590) | `› All commits` above the count, or `› Read the schema first · abc1234`. Enter or a click opens the list under it: `✓ All commits`, then each commit's headline cut to its row with the seven characters at the end, ✓ before the diff shown, and `Show more (13 left)`. ↑ and ↓ choose an entry, and Enter or a click shows its diff, reading it from its first slice, and closes the list with its row chosen. Commits at the same instant keep T3's order, and one whose time t3term can't read goes last. An empty headline reads `Untitled commit`, as on the Timeline. While a commit's diff shows, every thread is in the list off the diff. A read of the conversation that no longer lists the commit reads the whole change again. Picking another diff stops the read of the one before, whose answer t3term would drop. | | Count | `12 files`, with `+` while more slices come (PullRequestCodeTab.tsx:1124-1125), and a note when the host withheld part of the diff (1183-1195) | `12 files+`, then `The host withheld part of this diff, such as a binary file or a change too large to show.` in yellow where a slice says the host left something out or has text the parser found no file in. | | Files | Folded at first, as the `diffFilesCollapsed` setting is true until changed (pullRequestDiff.logic.ts:36-43, PullRequestCodeTab.tsx:230-232). The header has a chevron, an icon for how the file changed, the path, both paths of a rename, and `+3 -1` unless both are 0, from the host's own counts where the hunks it sent count none (822-823) | `› M src/lib.rs +3 -1`, with A, D, R or M in green, red, blue or yellow for an added, deleted, renamed or changed file. The path wraps under itself. Enter or a click unfolds it, `⌄`, and folds it again. A path one slice holds twice, as git writes a file whose type changed, shows once as its last part, as the desktop's `orderDiffFiles` keeps one file for each path (pullRequestFileOrder.logic.ts:152-153). | | Order | `orderDiffFiles` (pullRequestFileOrder.logic.ts:145-197) orders each slice on its own, so a later slice never moves the files above it (PullRequestCodeTab.tsx:404-411). Source comes first, each file after the changed files it imports. A quoted specifier after `from`, `import` or `require(` in any line of a file's hunks names a changed file by its path, with `./` and `../` resolved and a script's extension dropped, or by a name only one source file has. Where a cycle leaves no file ready, the lowest path goes next. Then the tests (`.test.`, `.spec.`, or a `test`, `tests` or `__tests__` directory), each after the source file whose name it has, then lockfiles, snapshots, minified files and files under `dist`, `build`, `vendor`, `__generated__` or `__snapshots__`, by path. Paths compare with `localeCompare` | The same order, worked out once when a slice comes. The diff library the desktop uses puts each unchanged line of a hunk on both sides, so an import in an unchanged line counts too (@pierre/diffs 1.5.2, dist/utils/parsePatchFiles.js:208-222), and t3term counts it. Paths compare as the root collation does for ASCII: punctuation, then digits, then letters with case ignored, then lowercase before uppercase. | @@ -108,7 +109,7 @@ Where the detail's `capabilities.diff` is true, the details have a third tab, Co | Review threads | Each thread on a file the tab shows, with a line inside one of its hunks, shows under that line (PullRequestCodeTab.tsx:452-472, pullRequestDiff.logic.ts:12-22). On a row with both numbers, a thread on the old side comes before one on the new (@pierre/diffs 1.5.2, dist/renderers/DiffHunksRenderer.js:1051-1075). `ReviewThreadCard` starts open unless resolved. Its header says `Open` or `Resolved`, the count of comments and `outdated`, and under it come the comments, `Load more comments` and buttons to resolve, reply, edit and react (PullRequestReviewAnnotation.tsx:89-358). `Load more comments` reads the next page through `pullRequests.threadComments`, reads `Loading...` until it comes, and adds its comments, keeping the first of each id (139-207, 305-313, pullRequestDetail.logic.ts:170-184). A failure shows a toast (PullRequestCodeTab.tsx:928-940) | `│ ⌄ Open · 3 comments · outdated` behind a bar, with `Resolved` in green. It starts open unless resolved, and Enter or a click on its first row opens and closes it. Open, it shows each comment as the Summary does, with a blank row between them, then `Load more comments` where the host has another page. Enter or a click reads that page, as [pull request reads](pull-request-reads.md#a-review-threads-remarks) says, and the row reads `Loading more comments…` until it comes. Its comments follow the thread's, each id once. A failed read shows `t3term couldn't read more comments.` and the reason above the button, which reads the same page again. Where the host has no page to offer but counts more, `2 more comments on the host.` shows instead. Only the threads of unfolded files show, and a thread stays open or closed across reads, by its id. t3term places a thread only on a line it read, so one on a line of a hunk shorter than its header counts goes to the list off the diff. The desktop checks the hunk's range alone. | | Off the diff | Above the code, closed at first: `Conversations not on the current diff`, or `on the diff loaded so far` while slices are left, and the count. Open, it lists the threads by path, each path once, with `Line 42` before each thread, at most 256 pixels high (1376-1440) | `› 2 conversations not on the current diff`, or `on the diff read so far`, above the files. Enter or a click opens it. Open, it lists each path once, then `Line 42` and the thread, for each thread on it. | | Waiting and failing | `Loading pull request diff...`, `Loading more files...` after the files, the first slice's error alone, and `The rest of this diff could not be loaded.` with Retry for a later slice (760-776) | `Loading pull request diff…` and `Loading more files…`. The first slice's failure shows alone, and `r` or showing the Code after the details open again reads it again. A later slice's failure shows `t3term couldn't read the rest of this diff.`, the reason and `↻ Retry`, which ↑ and ↓ choose. | -| Empty | `This pull request has no file changes.` (1366-1372) | The same, or `t3term couldn't split this patch into files.` where the patch has text but no file the parser could read. | +| Empty | `This pull request has no file changes.`, or `This commit has no file changes.` for a commit's own diff (1366-1372), returned before the list off the diff (1376), so no review thread shows | The same, or `t3term couldn't split this patch into files.` where the patch has text but no file the parser could read. No review thread shows either, until a diff with files does. | ## Next slices @@ -156,7 +157,7 @@ The desktop opens a link in its own pull request view only when the environment' 4. **Refreshes.** Done: while the details hold a read, t3term follows `pullRequests.subscribeRefreshes` and reads the link again on each new number, and `r` sends `pullRequests.invalidate` with the link's reference before it reads again, as above. The invalidate spends host requests and makes every reader read again, so t3term sends it only for that key. The details also read the detail again every 5 minutes while you use t3term, and when you arrive at them: you come back after 6 minutes away, open the link's details again or the terminal gains focus, as the desktop's live refresh does (apps/web/src/hooks/useLiveRefresh.ts). 5. **The stack.** Done in part: where the host keeps stacks, a link's details read `pullRequests.stack` and list its layers and their states on the Summary, as above. The stack saved on the link shows until the first read of it came. Opening a layer's details from the stack is left. 6. **The project's list.** `pullRequests.list` and `pullRequests.listStats` list a project's pull requests outside any thread. That is a separate view, after the page. -7. **The diff.** Done in part: the Code tab reads the diff a slice at a time and shows its files, as above. Each slice's files show in the desktop's reading order, and the review threads under their lines, with the rest of a thread's comments a page at a time. Next come the commit picker, more lines around a hunk from `pullRequests.diffFileContents`, viewed files, and the toolbar's toggles. +7. **The diff.** Done in part: the Code tab reads the diff a slice at a time and shows its files, as above. Each slice's files show in the desktop's reading order, and the review threads under their lines, with the rest of a thread's comments a page at a time. The list of commits shows one commit's own diff. Next come opening a commit from the Timeline, more lines around a hunk from `pullRequests.diffFileContents`, viewed files, and the toolbar's toggles. Linking, unlinking, watching, reviewing, commenting, merging and every other write stay out until a person can confirm each one in t3term. @@ -173,7 +174,7 @@ Linking, unlinking, watching, reviewing, commenting, merging and every other wri - The desktop's linking from a URL, unlinking, watch toggle, row menu and pull request page aren't here. A link's details show the header's stack, the Summary tab's reviewers, labels, checks, description and comments, the Timeline and the Code tab's files and review threads, and not its actions. - The groups show no avatars, and their authors are only counted. - Reactions only show. t3term can't add or remove one, and doesn't name who reacted, where the desktop names up to three when the pointer rests on a pill. Neither t3term nor the desktop shows the description's reactions. -- A commit on the Timeline doesn't open, as the Code tab has no commit picker yet, and shows its first author's login where the desktop shows every author's avatar. +- A commit on the Timeline doesn't open its diff on the Code tab, where the desktop's does (PullRequestDetailPanel.tsx:516-519), and shows its first author's login where the desktop shows every author's avatar. - A run on the Timeline counts its authors by lowercased login and counts a deleted account as `ghost`, as the Summary's groups count them. The desktop's run counts logins as the host sent them and leaves out remarks with no author. - A run on the Timeline that is open stays open when the order switches. The desktop's runs close, because the switch makes each one anew. - A verdict on the Timeline names its file and line, with `Outdated`, where the desktop names its file. @@ -196,10 +197,11 @@ Linking, unlinking, watching, reviewing, commenting, merging and every other wri - The saved stack comes from the open thread's links. The desktop looks through the links of every thread on the environment and takes the newest by `syncedAt`, so a layer linked only from another thread shows its branch in t3term where the desktop shows its title. Of two links to one pull request on a thread, t3term keeps the first, where the desktop takes the newest. - t3term can't merge or rebase a stack, and doesn't keep the layers' head commits that the desktop's stack merge sends. - The Code tab compares paths as the desktop's `localeCompare` does for ASCII. A character outside ASCII comes after the letters, where the desktop's collation puts an accented letter with its base letter, so `é.ts` comes after `z.ts` rather than before it. -- The Code tab has no commit picker, more lines around a hunk, viewed files, file tree, raw patch, or the desktop's whitespace, split view, wrap and fold-all toggles. Its lines don't wrap at word breaks or show syntax colors. +- The Code tab has no more lines around a hunk, viewed files, file tree, raw patch, or the desktop's whitespace, split view, wrap and fold-all toggles. Its lines don't wrap at word breaks or show syntax colors. - A thread's pages of comments stay with the details' read, so they show again after the conversation is read again and when the same link's details open again. The desktop's card keeps them only while it is mounted. Once a thread's pages run out, a read again that counts more comments than it holds offers `Load more comments` again, from the page after the comments the read sent, and the comments it holds show once. The desktop's card reopens its pages only after the reader's own reply. A failed page says so in the thread, where the desktop shows a toast. - A thread holds at most 1,000 comments, and offers no page past them. Its id and cursors are cut at 1,024 bytes, as t3term cuts every id, so a longer cursor would read the wrong page or fail. GitHub's are far shorter. - Nothing resolves, replies to, edits or reacts to a thread, and a thread has no `Fix in thread`. - The list of conversations off the diff scrolls with the files. The desktop's stays above them, at most 256 pixels high. A thread shows its state as a word after the chevron, where the desktop draws a circle, or a circle with a check once resolved. - The Code tab reads the diff only while it shows. The desktop's stays mounted and reads on a refresh while another tab shows, so its files are ready when the reader comes back. t3term instead reads the first slice again when the Code shows where it came a minute or more ago, so the files show the old diff until it comes. - The files read before show while the first slice is read again. The desktop drops them and shows `Loading pull request diff...`. +- The list of commits opens in the Code tab's rows and pushes the files down, where the desktop's menu opens over them. Going back to a diff read before reads its first slice from T3 again, which keeps a whole change's slices for a minute and a commit's for ten. diff --git a/docs/pull-request-reads.md b/docs/pull-request-reads.md index 9f23e8c..d1ba06c 100644 --- a/docs/pull-request-reads.md +++ b/docs/pull-request-reads.md @@ -42,10 +42,12 @@ A `Stack` has the host's id, its own number, its page, the branch its bottom lay ## A diff -`diff` reads the changes over HTTP, as the desktop does. It POSTs a `PullRequestDiffInput`, which is a `PullRequestRef` and an optional `cursor`, to `/api/pull-requests/diff` with the session's token (packages/contracts/src/environmentHttp.ts:650-663). T3 sends the patch over HTTP because it is often the largest answer, and the route checks `orchestration:read` as the RPC methods do (apps/server/src/pullRequest/http.ts). It takes the `Api` a `Client` holds, where the other reads take the RPC client. +`diff` reads the changes over HTTP, as the desktop does. It POSTs a `PullRequestDiffInput`, which is a `PullRequestRef`, an optional `cursor` and an optional `commit`, to `/api/pull-requests/diff` with the session's token (packages/contracts/src/environmentHttp.ts:650-663). T3 sends the patch over HTTP because it is often the largest answer, and the route checks `orchestration:read` as the RPC methods do (apps/server/src/pullRequest/http.ts). It takes the `Api` a `Client` holds, where the other reads take the RPC client. A diff comes a slice at a time, and a slice holds whole files. `next_cursor` is where the next slice starts, or `None` once the diff is whole, and the caller passes it back as `cursor` to read on. `truncated` says the host left something in the slice out, such as a binary file or a change too large to inline. `omitted` gives the host's own counts for files whose hunks it left out. T3's GitHub provider asks GitHub for the whole diff, up to 8 MiB, in one slice. Past that, or past the 300 files GitHub will diff, it reads 100 files a slice from GitHub's files API, and writes a file GitHub sends no patch for as its header alone (apps/server/src/pullRequest/GitHubPullRequestCli.ts:2206-2259, gitHubPullRequestJson.ts:3219-3266). T3 keeps a slice for 60 seconds under the `updatedAt` its last detail read gave, so once a detail read finds the pull request changed, the next slice comes from the host (apps/server/src/pullRequest/PullRequestService.ts:164, 3175-3215). +With a `commit`, the diff is that commit's own changes rather than the whole change's, as the desktop's Code tab reads it when a commit is picked (apps/web/src/components/pullRequest/PullRequestCodeTab.tsx:262-290). Its cursors are its own, so a cursor from the whole change's slices doesn't read on in a commit's. GitHub's provider reads a commit's files from the files API from the first slice, since `gh pr diff` can't name a commit, and fails where `commit` isn't a commit's SHA (GitHubPullRequestCli.ts:2216-2230). T3 keeps a commit's slices for 10 minutes, under the commit rather than the `updatedAt`, since a commit's changes can't change (PullRequestService.ts:165-166, 3189, 3195-3201). `diff` sends the id as the activity gave it. + `patch::Patch::read` splits a slice's patch into files, hunks and lines. A file has its paths before and after, its status (added, deleted, renamed or modified), whether git says it is binary, and the lines it adds and removes. A line has its kind, its numbers before and after the change, and its text as a range of the patch, so nothing is copied. The parser reads git's quoted paths with their escapes, the tab git puts after a path with a space, and a plain unified diff without `diff --git`. A line it can't place, such as a hunk header that isn't one, marks the file `unreadable`. So does a hunk with fewer or more lines on a side than its header counts, including one the patch ends inside, and a header whose lines would be numbered past 18,446,744,073,709,551,615, the highest number t3term keeps. The rest of the patch still reads. ## A review thread's remarks @@ -112,7 +114,7 @@ The unit tests read a detail with every field filled in, then without each optio - The TUI calls `detail`, `activity`, `stack`, `invalidate`, `diff` and `thread_comments`, and follows `pullRequests.subscribeRefreshes` through `client::spawn_pull_request_refresh_watch`. It shows the activity's remarks, verdicts, commits, reactions and review threads. It never reads `checks`. - There is no CLI command for a pull request. -- The list of pull requests, the diff of one commit, a file's whole contents, viewed files, review thread replies, and reviewer and label candidates aren't read. +- The list of pull requests, a file's whole contents, viewed files, review thread replies, and reviewer and label candidates aren't read. - `diff` doesn't ask for compression, as T3 runs on the same machine. - Nothing writes: no comment, review, reaction, resolution, label, reviewer or action. Those need `source-control:write`. - A project whose repository matches the link isn't looked for. The TUI passes the thread's project and the link's host, and T3 routes the ref by host. diff --git a/src/pull_request.rs b/src/pull_request.rs index 5e12511..148536a 100644 --- a/src/pull_request.rs +++ b/src/pull_request.rs @@ -374,15 +374,18 @@ pub async fn thread_comments( CommentPage::read(&answer) } -/// One slice of the pull request's diff, from the slice `cursor` names or from the first. -/// T3 sends the whole diff in one slice where the host gives it whole, and a page of files at a -/// time otherwise, with the cursor of the next in `next_cursor`. +/// One slice of the pull request's diff, from the slice `cursor` names or from the first, of +/// the whole change or of the commit `commit` names alone, as the desktop's Code tab scopes it +/// (PullRequestCodeTab.tsx:262-290 at f570bd2). T3 sends the whole diff in one slice where the +/// host gives it whole, and a page of files at a time otherwise, with the cursor of the next in +/// `next_cursor`. A commit's cursors are its own. pub async fn diff( api: &Api, reference: &Reference, cursor: Option<&str>, + commit: Option<&str>, ) -> Result { - read_diff(api, reference, cursor, MOST_DIFF).await + read_diff(api, reference, cursor, commit, MOST_DIFF).await } /// `diff`, reading at most `most` bytes of the answer. @@ -390,12 +393,16 @@ async fn read_diff( api: &Api, reference: &Reference, cursor: Option<&str>, + commit: Option<&str>, most: usize, ) -> Result { let mut payload = reference.payload(); if let Some(cursor) = cursor { payload["cursor"] = json!(cursor); } + if let Some(commit) = commit { + payload["commit"] = json!(commit); + } let (status, body) = api .post_bounded(DIFF, &payload, DIFF_TIMEOUT, most) .await diff --git a/src/pull_request/tests.rs b/src/pull_request/tests.rs index f21ce65..2ca61f5 100644 --- a/src/pull_request/tests.rs +++ b/src/pull_request/tests.rs @@ -1156,7 +1156,7 @@ mod diff { let answer = fake::response(status, &["location: http://127.0.0.1:1/"], body); let t3 = fake::Server::start(move |_| answer.clone()); let api = Api::without_proxy(&t3.origin); - diff(&api, &reference(), None).await.unwrap_err() + diff(&api, &reference(), None, None).await.unwrap_err() } fn http(error: PullRequestError) -> String { @@ -1180,7 +1180,7 @@ mod diff { let answer = fake::response("200 OK", &[], &body.to_string()); let t3 = fake::Server::start(move |_| answer.clone()); let api = Api::without_proxy(&t3.origin); - let first = diff(&api, &reference(), None).await.unwrap(); + let first = diff(&api, &reference(), None, None).await.unwrap(); assert_eq!( first, Diff { @@ -1202,10 +1202,17 @@ mod diff { omitted_cut: false, } ); - diff(&api, &reference(), first.next_cursor.as_deref()) + diff(&api, &reference(), first.next_cursor.as_deref(), None) .await .unwrap(); - assert_eq!(t3.paths(), [DIFF, DIFF]); + // A commit's own diff names it, beside the cursor of its own next slice. + diff(&api, &reference(), None, Some("abc1234def")) + .await + .unwrap(); + diff(&api, &reference(), Some("3"), Some("abc1234def")) + .await + .unwrap(); + assert_eq!(t3.paths(), [DIFF, DIFF, DIFF, DIFF]); let bodies: Vec = t3 .bodies() .iter() @@ -1213,7 +1220,11 @@ mod diff { .collect(); let mut next = reference().payload(); next["cursor"] = json!("2"); - assert_eq!(bodies, [reference().payload(), next]); + let mut commit = reference().payload(); + commit["commit"] = json!("abc1234def"); + let mut commit_next = commit.clone(); + commit_next["cursor"] = json!("3"); + assert_eq!(bodies, [reference().payload(), next, commit, commit_next]); } #[test] @@ -1337,7 +1348,7 @@ mod diff { let t3 = fake::Server::start(|_| String::new()); let api = Api::without_proxy(&t3.origin); - let error = http(diff(&api, &reference(), None).await.unwrap_err()); + let error = http(diff(&api, &reference(), None, None).await.unwrap_err()); assert!( error.starts_with("t3term couldn't read the diff from T3: "), "{error}" @@ -1355,9 +1366,9 @@ mod diff { for answer in [sized, streamed] { let t3 = fake::Server::start(move |_| answer.clone()); let api = Api::without_proxy(&t3.origin); - let read = read_diff(&api, &reference(), None, body.len()).await; + let read = read_diff(&api, &reference(), None, None, body.len()).await; assert_eq!(read.unwrap().patch, "0123456789"); - let error = read_diff(&api, &reference(), None, body.len() - 1).await; + let error = read_diff(&api, &reference(), None, None, body.len() - 1).await; assert_eq!( http(error.unwrap_err()), "The diff is longer than the 16 MiB t3term reads." diff --git a/src/tui/mod.rs b/src/tui/mod.rs index 32b4b3d..1add2fb 100644 --- a/src/tui/mod.rs +++ b/src/tui/mod.rs @@ -104,7 +104,8 @@ type WatchSetup = Box mpsc::UnboundedReceiver>; /// as an action with the ask's ticket. type ReadPull = Box; /// Reads a slice of a pull request's diff in the background as `DiffAsk` says, and sends the -/// answer as an action with the ask's ticket. +/// answer as an action with the ask's ticket. The details want only the answer to their newest +/// ask, so a read stops the one under way before it. type ReadDiff = Box; /// Reads the next page of a review thread's remarks in the background as `CommentsAsk` says, /// and sends the answer as an action with the ask's ticket. @@ -771,14 +772,16 @@ async fn read_pull_request(client: &Client, reference: &Reference, ask: &Ask, se } /// Reads the slice of the diff of the pull request `reference` names that `cursor` names, or the -/// first, through T3's HTTP route, as the desktop's Code tab does. A sentence of T3's own, such -/// as a host the server has no credentials for, reads as T3 wrote it, as for the detail. +/// first, of the whole change or of the commit `commit` names, through T3's HTTP route, as the +/// desktop's Code tab does. A sentence of T3's own, such as a host the server has no +/// credentials for, reads as T3 wrote it, as for the detail. async fn read_diff( client: &Client, reference: &Reference, cursor: Option<&str>, + commit: Option<&str>, ) -> Result, String> { - match pull_request::diff(&client.api, reference, cursor).await { + match pull_request::diff(&client.api, reference, cursor, commit).await { Ok(diff) => Ok(Box::new(diff)), Err(e @ PullRequestError::Rpc(_)) => Err(format!("Couldn't read this diff: {e}")), Err(e) => Err(e.to_string()), @@ -1155,14 +1158,23 @@ impl App { read_pull_request(&client, &reference, &ask, send).await; }); }), - read_diff: Box::new(move |reference, ask: DiffAsk| { - let (client, answers) = (differ.clone(), diffs.clone()); - tokio::spawn(async move { - let answer = read_diff(&client, &reference, ask.cursor.as_deref()).await; - let ticket = ask.ticket; - let _ = answers.send(ActionResult::PullRequestDiff { ticket, answer }); - }); - }), + read_diff: { + let under_way = std::cell::RefCell::new(None::); + Box::new(move |reference, ask: DiffAsk| { + let (client, answers) = (differ.clone(), diffs.clone()); + let read = tokio::spawn(async move { + let (cursor, commit) = (ask.cursor.as_deref(), ask.commit.as_deref()); + let answer = read_diff(&client, &reference, cursor, commit).await; + let ticket = ask.ticket; + let _ = answers.send(ActionResult::PullRequestDiff { ticket, answer }); + }); + // An ask comes only once the details have the answer to the last one or no + // longer want it, as when another commit was picked, so that read stops. + if let Some(before) = under_way.replace(Some(read.abort_handle())) { + before.abort(); + } + }) + }, read_comments: Box::new(move |reference, ask: CommentsAsk| { let (client, answers) = (pager.clone(), pages.clone()); tokio::spawn(async move { @@ -13979,8 +13991,16 @@ mod tests { } /// What a read of a pull request's diff named: the project, the host, the repository, - /// the number, the ticket and the cursor of the slice. - type NotedDiff = (String, String, String, u64, u64, Option); + /// the number, the ticket, the cursor of the slice and the commit. + type NotedDiff = ( + String, + String, + String, + u64, + u64, + Option, + Option, + ); /// Replaces how `app` reads a pull request's diff, so a read only notes what it named. fn note_diffs(app: &mut App) -> std::rc::Rc>> { @@ -13994,6 +14014,7 @@ mod tests { reference.number(), ask.ticket, ask.cursor, + ask.commit, )); }); asked @@ -14193,6 +14214,7 @@ mod tests { named.3, named.4, named.5, + None, ); assert_eq!(*diffs.borrow(), std::slice::from_ref(&named)); let shown = status(&mut app); @@ -14234,6 +14256,7 @@ mod tests { 31, 4, None, + None, ); assert_eq!(*diffs.borrow(), [named.clone(), again.clone()]); let rows = body_after(&mut app, 200, 50); @@ -14329,6 +14352,96 @@ mod tests { ); } + #[test] + fn a_commit_picked_on_the_code_tab_reads_its_own_diff_through_the_links_project() { + let mut app = readable_pulls_tui(vec![linked(31, "Bound titles")]); + let _asked = note_reads(&mut app); + let diffs = note_diffs(&mut app); + press(&mut app, KeyCode::Enter); + let mut activity = host_activity_sent(); + activity["commits"] = json!([ + {"oid": "1111111aaaa", "messageHeadline": "Read the schema first", + "committedDate": "2026-10-13T08:00:00Z"}, + {"oid": "2222222bbbb", "messageHeadline": "Check the schema on reload", + "committedDate": "2026-10-13T09:00:00Z"}, + ]); + let activity = pull_request::Activity::read(&activity).unwrap(); + let parts = vec![ + Part::Detail(Ok(Box::new(host_detail()))), + Part::Activity(Ok(Box::new(activity))), + Part::Done, + ]; + hear(&mut app, 1, parts); + press(&mut app, KeyCode::Right); + press(&mut app, KeyCode::Right); + let diff = pull_request::Diff { + patch: "diff --git a/a b/a\n--- a/a\n+++ b/a\n@@ -1 +1 @@\n-a\n+b\n".into(), + truncated: false, + next_cursor: None, + omitted: Vec::new(), + omitted_cut: false, + }; + let answer = Ok(Box::new(diff.clone())); + app.on_action_result(ActionResult::PullRequestDiff { ticket: 2, answer }); + let status = |app: &mut App| { + let buffer = cells(app, 200, 50); + text_in(&buffer, Rect::new(0, 49, 200, 1)) + }; + let rows = body_after(&mut app, 200, 50); + let text: Vec<&str> = rows.iter().map(|row| row.trim()).collect(); + assert_eq!(text[4..6], ["› All commits", "1 file"]); + + // ↓ chooses the row, Enter opens the list, and Enter on a commit reads its diff. + press(&mut app, KeyCode::Down); + assert!(status(&mut app).contains("Enter choose commit · ")); + press(&mut app, KeyCode::Enter); + let rows = body_after(&mut app, 200, 50); + let text: Vec<&str> = rows.iter().map(|row| row.trim()).collect(); + assert_eq!(text[4..6], ["⌄ All commits", "✓ All commits"]); + // The pane is narrow, so the headline is cut to keep the id. + assert_eq!(text[6], "Check the schema on… 2222222"); + press(&mut app, KeyCode::Down); + press(&mut app, KeyCode::Down); + let shown = status(&mut app); + assert!(shown.contains("Enter show commit 2222222 · "), "{shown}"); + press(&mut app, KeyCode::Enter); + let named = |ticket: u64, commit: Option<&str>| { + ( + "p".to_string(), + "github.com".to_string(), + "krishhgg/t3term".to_string(), + 31, + ticket, + None, + commit.map(str::to_string), + ) + }; + assert_eq!(diffs.borrow()[1..], [named(3, Some("2222222bbbb"))]); + let rows = body_after(&mut app, 200, 50); + let text: Vec<&str> = rows.iter().map(|row| row.trim()).collect(); + // The row wraps the commit's id under its headline. + assert_eq!( + text[4..7], + [ + "› Check the schema on reload", + "2222222", + "Loading pull request diff…" + ] + ); + + // A click on either of its rows opens the list, and one on All commits reads the + // whole change again. + let answer = Ok(Box::new(diff)); + app.on_action_result(ActionResult::PullRequestDiff { ticket: 3, answer }); + cells(&mut app, 200, 50); + let body = app.details.drawn.expect("the pane").body; + assert!(mouse(&mut app, CLICK, body.x + 2, body.y + 5)); + let rows = body_after(&mut app, 200, 50); + assert_eq!(rows[6].trim(), "All commits"); + assert!(mouse(&mut app, CLICK, body.x + 4, body.y + 6)); + assert_eq!(diffs.borrow()[2..], [named(4, None)]); + } + #[test] fn load_more_comments_reads_a_threads_next_page_through_the_links_project() { let mut app = readable_pulls_tui(vec![linked(31, "Bound titles")]); @@ -15878,6 +15991,98 @@ mod tests { assert_eq!(reads(&runtime), 0); } + /// The app's reader sends T3 the slice's cursor and the commit a diff read names, and its + /// answer comes back with the ticket. + #[tokio::test] + async fn the_app_reads_a_slice_of_a_commits_diff_from_t3() { + use crate::http::{Api, fake}; + + let body = json!({"patch": "", "truncated": false, "nextCursor": null}); + let answer = fake::response("200 OK", &[], &body.to_string()); + let t3 = fake::Server::start(move |_| answer.clone()); + let mut client = Client::offline(); + client.api = Api::without_proxy(&t3.origin); + let (sender, mut results) = mpsc::unbounded_channel(); + let app = App::new( + Arc::new(client), + Theme::new(Depth::TrueColor), + &Settings::default(), + sender, + ); + let reference = Reference::new("p", Some("github.com"), "krishhgg/t3term", 31).unwrap(); + let ask = DiffAsk { + ticket: 7, + host: "github.com".into(), + repository: "krishhgg/t3term".into(), + number: 31, + cursor: Some("2".into()), + commit: Some("abc1234def".into()), + }; + (app.read_diff)(reference, ask); + let result = tokio::time::timeout(Duration::from_secs(5), results.recv()) + .await + .expect("an answer within five seconds") + .expect("the answer"); + let ActionResult::PullRequestDiff { ticket, answer } = result else { + panic!("an answer of another kind"); + }; + assert_eq!(ticket, 7); + assert_eq!(answer.map(|diff| diff.patch), Ok(String::new())); + let sent: Value = serde_json::from_str(&t3.bodies()[0]).unwrap(); + assert_eq!(sent["cursor"], "2"); + assert_eq!(sent["commit"], "abc1234def"); + } + + #[tokio::test] + async fn the_apps_next_read_of_a_diff_stops_the_one_under_way() { + use crate::http::{Api, fake}; + use std::sync::atomic::{AtomicUsize, Ordering}; + + // T3 answers one read at a time and holds the first back for a second, so the second + // read's answer comes after the first's unless the first stopped. + let body = json!({"patch": "", "truncated": false, "nextCursor": null}); + let answer = fake::response("200 OK", &[], &body.to_string()); + let reads = AtomicUsize::new(0); + let t3 = fake::Server::start(move |_| { + if reads.fetch_add(1, Ordering::SeqCst) == 0 { + std::thread::sleep(Duration::from_secs(1)); + } + answer.clone() + }); + let mut client = Client::offline(); + client.api = Api::without_proxy(&t3.origin); + let (sender, mut results) = mpsc::unbounded_channel(); + let app = App::new( + Arc::new(client), + Theme::new(Depth::TrueColor), + &Settings::default(), + sender, + ); + let reference = Reference::new("p", Some("github.com"), "krishhgg/t3term", 31).unwrap(); + let ask = |ticket, commit: &str| DiffAsk { + ticket, + host: "github.com".into(), + repository: "krishhgg/t3term".into(), + number: 31, + cursor: None, + commit: Some(commit.into()), + }; + (app.read_diff)(reference.clone(), ask(7, "1111111aaaa")); + tokio::time::sleep(Duration::from_millis(200)).await; + (app.read_diff)(reference, ask(8, "2222222bbbb")); + let result = tokio::time::timeout(Duration::from_secs(5), results.recv()) + .await + .expect("an answer within five seconds") + .expect("the answer"); + let ActionResult::PullRequestDiff { ticket, .. } = result else { + panic!("an answer of another kind"); + }; + assert_eq!(ticket, 8); + assert!(results.try_recv().is_err()); + let sent: Value = serde_json::from_str(&t3.bodies()[1]).unwrap(); + assert_eq!(sent["commit"], "2222222bbbb"); + } + // ---- the Environments dialog ---- /// The TUI with a stand-in for the check of the saved environments, which counts the diff --git a/src/tui/pull_requests.rs b/src/tui/pull_requests.rs index 8a60c8c..1b767d5 100644 --- a/src/tui/pull_requests.rs +++ b/src/tui/pull_requests.rs @@ -926,8 +926,8 @@ pub(super) struct Ask { } /// What reading a slice of a link's pull request's diff names: the link's host, repository and -/// number as T3 sent them, the cursor of the slice, or None for the first, and the ticket its -/// answer comes back with. +/// number as T3 sent them, the cursor of the slice, or None for the first, the commit whose own +/// diff it reads, or None for the whole change's, and the ticket its answer comes back with. #[derive(Debug, Clone, PartialEq, Eq)] pub(super) struct DiffAsk { pub ticket: u64, @@ -935,6 +935,7 @@ pub(super) struct DiffAsk { pub repository: String, pub number: u64, pub cursor: Option, + pub commit: Option, } /// What reading the next page of a review thread's remarks names: the link's host, repository @@ -1695,13 +1696,32 @@ impl PullRequests { { return; } - if let Control::File(_) | Control::Retry = control { + if control.on_code() { let read = self.read.as_mut().expect("the shown read"); - let Some(code) = read.code.as_mut().filter(|code| code.has(control)) else { - return; + let commits = match &read.conversation { + Answer::Read(conversation) => conversation.commits(), + _ => Vec::new(), }; - code.act(control); - self.pick = Some(control).filter(|control| code.has(*control)); + let code = read.code.get_or_insert_with(Code::new); + if !code.has(control, &commits) { + return; + } + let listed = code.listed(&commits); + if code.act(control, &commits) { + self.shown.close_off_diff(); + } + self.pick = match control { + // The list closes on the row that opened it, as a menu gives its button the + // focus back. + Control::AllCommits | Control::Commit(_) => Some(Control::Commits), + // The first of the commits it listed takes the place of Show more once none + // are left. + Control::MoreCommits if !code.has(control, &commits) => { + Some(Control::Commit(listed)) + } + _ => Some(control), + } + .filter(|control| code.has(*control, &commits)); self.relay(); return; } @@ -1763,8 +1783,12 @@ impl PullRequests { .read .as_ref() .filter(|read| self.reads_selected(read))?; - if let Control::File(_) | Control::Retry = pick { - return read.code.as_ref().map(|code| code.action(pick)); + if pick.on_code() { + let commits = match &read.conversation { + Answer::Read(conversation) => conversation.commits(), + _ => Vec::new(), + }; + return read.code.as_ref().map(|code| code.action(pick, &commits)); } match &read.conversation { Answer::Read(conversation) => Some(conversation.action(pick, &self.shown)), @@ -2197,6 +2221,12 @@ impl PullRequests { conversation.follow(&summary.lifecycle); self.shown.prune(conversation); } + // A commit a rebase took out of the change takes the Code tab back to the whole of it. + if let (Some(code), Answer::Read(conversation)) = (&mut read.code, &read.conversation) + && code.follow(&conversation.commits()) + { + self.shown.close_off_diff(); + } // A detail that says the host gives no diff takes the Code tab away, and the Summary // shows in its place, as the desktop's panel moves the reader (1444-1455). if self.tabs() == Some(Tab::Code) && !self.offers_code() { @@ -2228,7 +2258,8 @@ impl PullRequests { if read.invalidate || read.echo { return None; } - let cursor = read.code.as_mut().expect("made above").ask(ticket)?; + let code = read.code.as_mut().expect("made above"); + let cursor = code.ask(ticket)?; self.tickets = ticket; Some(DiffAsk { ticket, @@ -2236,6 +2267,7 @@ impl PullRequests { repository: read.repository.clone(), number: read.number, cursor, + commit: code.commit().map(str::to_string), }) } @@ -2286,7 +2318,9 @@ impl PullRequests { /// Takes the answer to read `ticket` of a slice of the diff, which came at `now`, or why it /// failed. Says whether what the details show changed. A first slice replaces the files - /// shown, so no control stays chosen. An answer to another read is dropped. + /// shown, so no control on them stays chosen. The list of commits above them keeps its + /// own, as the desktop's menu gives its button the focus back after a pick. An answer to + /// another read is dropped. pub fn answer_diff( &mut self, ticket: u64, @@ -2297,7 +2331,7 @@ impl PullRequests { let Some(first) = code.and_then(|code| code.take(ticket, answer, now)) else { return false; }; - if first { + if first && !self.pick.is_some_and(Control::on_commits) { self.pick = None; } self.relay(); @@ -3060,15 +3094,16 @@ fn detail_lines( if let (Tab::Code, Some(read)) = (tab, read) { lines.push(Line::default()); let at = lines.len(); - // The review threads come with the conversation, and none show until it came. - let threads = match &read.conversation { - Answer::Read(conversation) => conversation.threads(), - _ => &[], + // The review threads and the commits come with the conversation, and none show until + // it came. + let (threads, commits) = match &read.conversation { + Answer::Read(conversation) => (conversation.threads(), conversation.commits()), + _ => (&[][..], Vec::new()), }; // Until the tab asks for its first slice, it shows as it does while that read runs. let (code, controls) = match &read.code { - Some(code) => code.lay_out(threads, shown, width, now, theme), - None => Code::new().lay_out(threads, shown, width, now, theme), + Some(code) => code.lay_out(threads, &commits, shown, width, now, theme), + None => Code::new().lay_out(threads, &commits, shown, width, now, theme), }; lines.extend(code); let moved = |rows: Range| rows.start + at..rows.end + at; @@ -6350,6 +6385,7 @@ mod tests { repository: "krishhgg/t3term".into(), number: 31, cursor: cursor.map(str::to_string), + commit: None, } } @@ -6739,6 +6775,127 @@ mod tests { assert!(rows.contains(&"› A f1.rs +1 -0".to_string()), "{rows:#?}"); } + #[test] + fn the_code_tab_reads_the_commit_picked_from_its_list() { + let theme = theme(); + let commit = |n: u32| { + json!({"oid": format!("{n:07}abc"), "messageHeadline": format!("Commit {n}"), + "committedDate": format!("2026-10-13T00:{n:02}:00Z")}) + }; + let mut activity = remarks(1); + activity["commits"] = (0..21).map(commit).collect(); + activity["reviewThreads"] = json!([ + {"id": "t0", "path": "f0.rs", "line": 1, "side": "right", "isResolved": false, + "isOutdated": false, "comments": [{"id": "r0", + "author": {"login": "mona", "name": null}, "body": "On line 1.", + "createdAt": "2026-10-13T00:30:00Z", "url": null}]}, + ]); + let scoped = |ticket: u64, commit: Option<&str>| DiffAsk { + commit: commit.map(str::to_string), + ..diff_ask(ticket, None) + }; + let mut pulls = conversing(activity.clone()); + assert!(!pulls.answer(1, Part::Done, now())); + assert!(pulls.forward() && pulls.forward()); + assert_eq!(pulls.take_diff_ask(true, now()), Some(scoped(2, None))); + assert!(pulls.answer_diff(2, Ok(added(0, 1, None)), now())); + pulls.page(60, 40, now(), true, &theme); + let rows = detail_rows(&pulls); + let at = rows + .iter() + .position(|row| row == "› All commits") + .expect("the row"); + assert_eq!(rows[at + 1], "1 file"); + + // The row that picks the diff comes first, and opens the list. + pulls.step(true); + assert_eq!(pulls.pick, Some(Control::Commits)); + assert_eq!(pulls.action().as_deref(), Some("choose commit")); + pulls.enter(); + assert_eq!(pulls.pick, Some(Control::Commits)); + assert_eq!(pulls.action().as_deref(), Some("hide commits")); + pulls.page(60, 40, now(), true, &theme); + pulls.step(true); + assert_eq!(pulls.pick, Some(Control::AllCommits)); + assert_eq!(pulls.action().as_deref(), Some("show all commits")); + pulls.step(true); + assert_eq!(pulls.pick, Some(Control::Commit(0))); + assert_eq!(pulls.action().as_deref(), Some("show commit 0000020")); + + // Show more stays chosen while more are left, and the first commit it listed takes + // its place once none are. + pulls.pick = Some(Control::MoreCommits); + assert_eq!(pulls.action().as_deref(), Some("show more commits")); + pulls.enter(); + assert_eq!(pulls.pick, Some(Control::MoreCommits)); + pulls.enter(); + assert_eq!(pulls.pick, Some(Control::Commit(20))); + pulls.enter(); + assert_eq!(pulls.pick, Some(Control::Commits)); + assert_eq!( + pulls.take_diff_ask(true, now()), + Some(scoped(3, Some("0000000abc"))) + ); + // An entry of the list closed picks nothing. + pulls.pick = Some(Control::Commit(1)); + pulls.enter(); + assert_eq!(pulls.pick, Some(Control::Commit(1))); + assert_eq!(pulls.take_diff_ask(true, now()), None); + pulls.pick = Some(Control::Commits); + + // A commit's diff places no thread, and picking another closes the list of those + // off the diff. + // The diff that comes keeps the row that picked it chosen, where it chooses nothing + // on the files it replaced. + assert!(pulls.answer_diff(3, Ok(added(0, 1, None)), now())); + assert_eq!(pulls.pick, Some(Control::Commits)); + pulls.page(60, 40, now(), true, &theme); + let rows = detail_rows(&pulls); + assert_eq!( + rows[at..at + 4], + [ + "› Commit 0 · 0000000", + "1 file", + "", + "› 1 conversation not on the current diff" + ] + ); + pulls.pick = Some(Control::OffDiff); + pulls.enter(); + assert!(pulls.shown.off_diff_open()); + pulls.pick = Some(Control::Commits); + pulls.enter(); + pulls.pick = Some(Control::Commit(0)); + pulls.enter(); + assert!(!pulls.shown.off_diff_open()); + assert_eq!( + pulls.take_diff_ask(true, now()), + Some(scoped(4, Some("0000020abc"))) + ); + pulls.pick = Some(Control::File(0)); + assert!(pulls.answer_diff(4, Ok(added(0, 1, None)), now())); + assert_eq!(pulls.pick, None); + pulls.pick = Some(Control::OffDiff); + pulls.enter(); + assert!(pulls.shown.off_diff_open()); + + // A read of the conversation that no longer lists the commit, as after a force-push, + // takes the tab back to the whole change, the list of threads off it closed. + activity["commits"] = (0..20).map(commit).collect(); + assert!(pulls.reload(now())); + let ask = pulls.take_ask(true, now()).expect("the read again"); + let detail = Part::Detail(Ok(read_detail(&bare_detail()))); + pulls.answer(ask.ticket, detail, now()); + let again = pull_request::Activity::read(&activity).unwrap(); + pulls.answer(ask.ticket, Part::Activity(Ok(Box::new(again))), now()); + assert!(pulls.shown.off_diff_open()); + pulls.answer(ask.ticket, Part::Done, now()); + assert!(!pulls.shown.off_diff_open()); + let ask = pulls.take_diff_ask(true, now()).expect("the whole change"); + assert_eq!(ask.commit, None); + assert_eq!(ask.cursor, None); + } + #[test] fn a_thread_reads_more_comments_a_page_at_a_time() { let theme = theme(); diff --git a/src/tui/pull_requests/code.rs b/src/tui/pull_requests/code.rs index e0437de..ac0a51f 100644 --- a/src/tui/pull_requests/code.rs +++ b/src/tui/pull_requests/code.rs @@ -17,11 +17,21 @@ //! or before it for a removed line, as the desktop's stacked view has, and the unchanged lines //! between two hunks are counted on a row of their own. //! +//! Where the conversation read lists commits, a row above the files says which diff the tab +//! shows, `All commits` or a commit's headline, and opens a list of the commits, newest first, +//! 10 at a time, as the desktop's scope menu does (PullRequestCodeTab.tsx:1063-1119). Picking +//! one reads that commit's own diff in place of the whole change's, from its first slice, with +//! the files folded and the list of conversations off the diff closed. A read of the +//! conversation that no longer lists the commit picked goes back to the whole change, as after +//! a force-push (1050-1062). +//! //! The review threads the conversation read show under the lines they are on, in the files //! unfolded, as the desktop's annotations do. A thread shows under a line only where a line of //! the file's hunks has its number on its side, as `isLineInFileDiff` asks //! (pullRequestDiff.logic.ts:12-22). The rest, on a line the diff doesn't show, on a file or on -//! a file not read yet, show above the files in a list that starts closed, by file. +//! a file not read yet, show above the files in a list that starts closed, by file. Under a +//! commit's own diff, where a line's number may mean other code than the thread's, every thread +//! shows in that list (PullRequestCodeTab.tsx:454-458). //! //! Only the files unfolded are laid out, each once for each width. Each line of them is cleaned //! of control characters, its tabs set every `TAB` columns, and cut at `LINE` characters as it @@ -31,13 +41,13 @@ use std::collections::{HashMap, HashSet}; use ratatui::style::{Color, Style}; use ratatui::text::{Line, Span}; -use unicode_width::UnicodeWidthChar; +use unicode_width::{UnicodeWidthChar, UnicodeWidthStr}; -use super::super::clip; use super::super::setup::bounded; use super::super::theme::Theme; -use super::conversation::{Control, Shown, Targets, Thread}; -use super::{FAILURE, FRESH, grouped, whole}; +use super::super::{clip, row}; +use super::conversation::{Commit, Control, Shown, Targets, Thread}; +use super::{FAILURE, FRESH, flowed, grouped, whole}; use crate::pull_request::{self, Side, patch}; use crate::transcript::clusters; @@ -52,6 +62,9 @@ const PATH: usize = 1024; const LOADING: &str = "Loading pull request diff…"; const LOADING_MORE: &str = "Loading more files…"; const EMPTY: &str = "This pull request has no file changes."; +const EMPTY_COMMIT: &str = "This commit has no file changes."; +/// How many more commits the list shows at a time, as the desktop's `COMMIT_PAGE_SIZE`. +const COMMIT_PAGE: usize = 10; const WITHHELD: &str = "The host withheld part of this diff, such as a binary file or a change too large to show."; const REST_FAILED: &str = "t3term couldn't read the rest of this diff."; @@ -85,6 +98,12 @@ pub(super) struct Code { /// The rows of each unfolded file's lines, by its slice's place and its place in that /// slice, so a file is laid out again only when its slice, the width or its fold changes. bodies: HashMap<(usize, usize), Body>, + /// The id of the commit whose own diff the tab shows, or None for the whole change. + commit: Option, + /// Whether the list of commits is open. + commits_open: bool, + /// How many commits the list shows. + commits_shown: usize, /// How many times a file's rows were laid out for `bodies`. #[cfg(test)] pub(super) builds: usize, @@ -147,6 +166,9 @@ impl Code { due: false, open: HashSet::new(), bodies: HashMap::new(), + commit: None, + commits_open: false, + commits_shown: COMMIT_PAGE, #[cfg(test)] builds: 0, } @@ -269,15 +291,31 @@ impl Code { self.due && self.asked.is_none() } - /// Whether `control` is one of the tab's and shows. - pub(super) fn has(&self, control: Control) -> bool { + /// The id of the commit whose own diff the tab shows, or None for the whole change. + pub(super) fn commit(&self) -> Option<&str> { + self.commit.as_deref() + } + + /// Whether `control` is one of the tab's and shows, with `commits` newest first in its + /// list. + pub(super) fn has(&self, control: Control, commits: &[&Commit]) -> bool { + let open = self.commits_open && !commits.is_empty(); match control { Control::File(at) => at < self.files.len(), Control::Retry => self.retries(), + Control::Commits => !commits.is_empty(), + Control::AllCommits => open, + Control::Commit(at) => open && at < self.listed(commits), + Control::MoreCommits => open && commits.len() > self.listed(commits), _ => false, } } + /// How many of `commits` the list shows while it is open. + pub(super) fn listed(&self, commits: &[&Commit]) -> usize { + commits.len().min(self.commits_shown) + } + /// Whether the end of the files offers Retry: the read of a later slice failed, and slices /// show above it with more after them. fn retries(&self) -> bool { @@ -286,13 +324,22 @@ impl Code { && self.next_cursor().is_some() } - /// Does what `control` does: folds or unfolds a file, or reads again the slice whose read - /// failed. - pub(super) fn act(&mut self, control: Control) { + /// Does what `control` does, with `commits` newest first in the list: folds or unfolds a + /// file, reads again the slice whose read failed, opens or closes the list of commits, + /// lists more of them, or picks the diff the tab shows. Says whether that diff changed. + pub(super) fn act(&mut self, control: Control, commits: &[&Commit]) -> bool { match control { + Control::Commits => self.commits_open = !self.commits_open, + Control::MoreCommits => self.commits_shown += COMMIT_PAGE, + Control::AllCommits => return self.scope(None), + Control::Commit(at) => { + if let Some(commit) = commits.get(at) { + return self.scope(Some(commit.oid.clone())); + } + } Control::File(at) => { let Some(path) = self.file(at).map(|(_, file)| file.path().to_string()) else { - return; + return false; }; if !self.open.remove(&path) { self.open.insert(path); @@ -306,10 +353,47 @@ impl Code { } _ => {} } + false + } + + /// Shows the diff of the commit `commit` names, or of the whole change, and closes the list + /// of commits. Another diff than the one shown is read from its first slice at once, with + /// its files folded and the list back to its first 10 commits, as the desktop's scope key + /// starts over (PullRequestCodeTab.tsx:268-277). A read under way for the diff before is + /// dropped when it comes. Says whether the diff changed. + fn scope(&mut self, commit: Option) -> bool { + self.commits_open = false; + if self.commit == commit { + return false; + } + self.commit = commit; + self.slices.clear(); + self.files.clear(); + self.omitted.clear(); + self.bodies.clear(); + self.open.clear(); + self.asked = None; + self.failed = None; + self.again = true; + self.due = false; + self.commits_shown = COMMIT_PAGE; + true + } + + /// Shows the whole change's diff again where `commits`, from a read of the conversation, + /// no longer list the commit shown, as after a rebase or a force-push. Says whether the + /// diff changed. + pub(super) fn follow(&mut self, commits: &[&Commit]) -> bool { + let gone = self + .commit + .as_deref() + .is_some_and(|oid| !commits.iter().any(|commit| commit.oid == oid)); + gone && self.scope(None) } - /// What Enter does with `control`, for the status line. - pub(super) fn action(&self, control: Control) -> String { + /// What Enter does with `control`, for the status line, with `commits` newest first in + /// the list. + pub(super) fn action(&self, control: Control, commits: &[&Commit]) -> String { match control { Control::File(at) => { let open = self @@ -317,6 +401,14 @@ impl Code { .is_some_and(|(_, file)| self.open.contains(file.path())); if open { "collapse file" } else { "expand file" }.to_string() } + Control::Commits if self.commits_open => "hide commits".to_string(), + Control::Commits => "choose commit".to_string(), + Control::AllCommits => "show all commits".to_string(), + Control::Commit(at) => match commits.get(at) { + Some(commit) => format!("show commit {}", commit.short), + None => "show commit".to_string(), + }, + Control::MoreCommits => "show more commits".to_string(), _ => "retry".to_string(), } } @@ -336,8 +428,15 @@ impl Code { /// Where each of `threads` shows: under a line of each file shown with its path whose /// hunks have a line with its number on its side, or else in the list of those off the - /// diff. + /// diff, as every thread does under a commit's own diff. fn place(&self, threads: &[Thread]) -> Placed { + if self.commit.is_some() { + let off = (0..threads.len()).collect(); + return Placed { + on: HashMap::new(), + off, + }; + } let mut by_path: HashMap<&str, Vec> = HashMap::new(); for at in 0..self.files.len() { let (_, file) = self.file(at).expect("a file shown"); @@ -449,16 +548,18 @@ impl Code { self.bodies = bodies; } - /// The tab's rows at `width`, with the rows of each file's header, of each review thread's - /// first row, of the list of those off the diff, and of Retry. Until the first slice came, - /// or when its read failed, it says so. Then the count of files, `+` while more are to be - /// read, and a note when the host withheld part of the diff. Then the list of `threads` off - /// the diff, then each file with the threads on its lines, and at the end `Loading more - /// files…`, or Retry once that failed. `shown` says which threads and whether the list are - /// open. + /// The tab's rows at `width`, with the rows of each control: the list of `commits`, newest + /// first, each file's header, each review thread's first row, the list of those off the + /// diff and Retry. First the row that picks the diff from `commits`, and its list while it + /// is open. Until the first slice came, or when its read failed, it says so. Then the count + /// of files, `+` while more are to be read, and a note when the host withheld part of the + /// diff. Then the list of `threads` off the diff, then each file with the threads on its + /// lines, and at the end `Loading more files…`, or Retry once that failed. `shown` says + /// which threads and whether the list are open. pub(super) fn lay_out( &self, threads: &[Thread], + commits: &[&Commit], shown: &Shown, width: usize, now: i64, @@ -470,12 +571,16 @@ impl Code { if width == 0 { return (lines, targets); } + // Above the diff while it loads too, so the list that picked a commit stays where it + // was while that commit's diff comes (PullRequestCodeTab.tsx:1322-1335). + self.scope_lines(commits, &mut lines, &mut targets, width, theme); if self.slices.is_empty() { let text = match &self.failed { Some((failure, _)) => failure.as_str(), None => LOADING, }; - return (whole(text, width, muted), targets); + lines.extend(whole(text, width, muted)); + return (lines, targets); } let count = self.files.len(); let noun = if count == 1 { "file" } else { "files" }; @@ -508,7 +613,11 @@ impl Code { } if !self.lists_files() { lines.push(Line::default()); - let text = if unread { UNREAD } else { EMPTY }; + let text = match (unread, &self.commit) { + (true, _) => UNREAD, + (false, Some(_)) => EMPTY_COMMIT, + (false, None) => EMPTY, + }; lines.extend(whole(text, width, muted)); return (lines, targets); } @@ -639,6 +748,97 @@ impl Code { } (lines, targets) } + + /// The row that says which diff the tab shows, `All commits` or the commit's headline and + /// the first seven characters of its id, and opens the list of `commits`, newest first. + /// While open, the list follows: `All commits`, then each commit's headline with those + /// seven characters at the end, ✓ before the diff shown, and `Show more (N left)` while + /// more are left, then a blank row. Nothing shows where there are no commits, as the + /// desktop draws no menu with nothing to pick (PullRequestCodeTab.tsx:1067-1069). + fn scope_lines( + &self, + commits: &[&Commit], + lines: &mut Vec>, + targets: &mut Targets, + width: usize, + theme: &Theme, + ) { + if commits.is_empty() { + return; + } + let fg = Style::new().fg(theme.fg); + let muted = Style::new().fg(theme.muted); + let chevron = if self.commits_open { "⌄" } else { "›" }; + let shown = self.commit.as_deref(); + let start = lines.len(); + match commits + .iter() + .find(|commit| Some(commit.oid.as_str()) == shown) + { + Some(commit) => { + let parts = vec![ + Span::styled(format!("{chevron} {}", commit.headline()), fg), + Span::styled(commit.short.clone(), muted), + ]; + lines.extend(flowed(parts, " · ", width, theme)); + } + None => lines.extend(whole(&format!("{chevron} All commits"), width, fg)), + } + targets.push((start..lines.len(), Control::Commits)); + if !self.commits_open { + return; + } + targets.push((lines.len()..lines.len() + 1, Control::AllCommits)); + lines.push(entry(shown.is_none(), "All commits", "", fg, width, theme)); + let listed = self.listed(commits); + for (at, commit) in commits[..listed].iter().enumerate() { + let picked = shown == Some(commit.oid.as_str()); + let (headline, short) = (commit.headline(), commit.short.as_str()); + targets.push((lines.len()..lines.len() + 1, Control::Commit(at))); + lines.push(entry(picked, headline, short, fg, width, theme)); + } + if commits.len() > listed { + let left = grouped((commits.len() - listed) as u64); + let text = format!("Show more ({left} left)"); + targets.push((lines.len()..lines.len() + 1, Control::MoreCommits)); + lines.push(entry(false, &text, "", muted, width, theme)); + } + lines.push(Line::default()); + } +} + +/// An entry of the list of commits on one row of `width`: ✓ where it is `picked`, its `text` +/// in `style` cut to fit, and `short`, a commit's id's first seven characters, at the end +/// beside at least four columns of the text. +fn entry( + picked: bool, + text: &str, + short: &str, + style: Style, + width: usize, + theme: &Theme, +) -> Line<'static> { + let lead = if picked { " ✓ " } else { " " }; + let indent = lead.width(); + if width <= indent { + return Line::from(Span::styled(clip(text, width), style)); + } + let id = short.width(); + let (right, room) = match id > 0 && indent + 4 + 1 + id <= width { + true => ( + vec![Span::styled( + short.to_string(), + Style::new().fg(theme.muted), + )], + width - indent - 1 - id, + ), + false => (Vec::new(), width - indent), + }; + let left = vec![ + Span::styled(lead, Style::new().fg(theme.fg)), + Span::styled(clip(text, room), style), + ]; + row(left, right, width) } /// A file's header: a chevron that says whether it is unfolded, a letter for how it changed, @@ -914,7 +1114,7 @@ fn broken(text: &str, width: usize) -> Vec { let mut rows = vec![String::new()]; let mut used = 0; for piece in clusters(text) { - let columns = unicode_width::UnicodeWidthStr::width(piece); + let columns = piece.width(); if used > 0 && used + columns > width { rows.push(String::new()); used = 0; diff --git a/src/tui/pull_requests/code/tests.rs b/src/tui/pull_requests/code/tests.rs index 31df2e3..9a9699c 100644 --- a/src/tui/pull_requests/code/tests.rs +++ b/src/tui/pull_requests/code/tests.rs @@ -21,7 +21,7 @@ fn rows_of(lines: &[Line]) -> Vec { /// The tab's rows and targets at `width`, with no review threads. fn laid(code: &Code, width: usize) -> (Vec>, Targets) { - code.lay_out(&[], &Shown::default(), width, 0, &theme()) + code.lay_out(&[], &[], &Shown::default(), width, 0, &theme()) } /// The tab's rows at `width`. @@ -117,14 +117,14 @@ fn files_show_folded_and_each_unfolds_into_its_hunks() { .zip(files) .collect::>() ); - assert_eq!(code.action(Control::File(0)), "expand file"); + assert_eq!(code.action(Control::File(0), &[]), "expand file"); // Each line has the number it has after the change, or before it where it was removed, // and the unchanged lines between two hunks are counted. for at in 0..5 { - code.act(Control::File(at)); + code.act(Control::File(at), &[]); } - assert_eq!(code.action(Control::File(0)), "collapse file"); + assert_eq!(code.action(Control::File(0), &[]), "collapse file"); assert_eq!( rows(&code, 40), [ @@ -158,7 +158,7 @@ fn files_show_folded_and_each_unfolds_into_its_hunks() { ); let (_, targets) = laid(&code, 40); assert_eq!(targets[1], (7..8, Control::File(1))); - code.act(Control::File(0)); + code.act(Control::File(0), &[]); assert_eq!(rows(&code, 40)[2..4], ["› A docs/new.md +2 -0", ""]); } @@ -178,7 +178,7 @@ fn the_unchanged_lines_before_a_hunk_count_from_the_line_git_names() { +y "; let mut code = read(slice(patch, None), 0); - code.act(Control::File(0)); + code.act(Control::File(0), &[]); assert_eq!( rows(&code, 40), [ @@ -202,7 +202,7 @@ fn the_unchanged_lines_before_a_hunk_count_from_the_line_git_names() { +y "; let mut code = read(slice(patch, None), 0); - code.act(Control::File(0)); + code.act(Control::File(0), &[]); assert_eq!(rows(&code, 40)[3], "⋯ 1 unmodified line"); } @@ -218,7 +218,7 @@ fn a_hunk_that_removes_nothing_after_the_highest_line_unfolds() { +added "; let mut code = read(slice(patch, None), 0); - code.act(Control::File(0)); + code.act(Control::File(0), &[]); let rows = rows(&code, 60); assert_eq!( rows[2..4], @@ -297,7 +297,7 @@ fn a_failure_says_why_and_a_later_slice_s_offers_retry() { ["Couldn't read this diff: [31mGitHub said", "no"] ); assert!(laid(&code, 40).1.is_empty()); - assert!(!code.has(Control::Retry)); + assert!(!code.has(Control::Retry, &[])); assert_eq!(code.ask(2), None, "a failure isn't read again by itself"); assert!(code.stale(0), "showing the tab again reads it"); @@ -325,10 +325,10 @@ fn a_failure_says_why_and_a_later_slice_s_offers_retry() { ); let (_, targets) = laid(&code, 40); assert_eq!(targets[1], (7..8, Control::Retry)); - assert!(code.has(Control::Retry)); - assert_eq!(code.action(Control::Retry), "retry"); - code.act(Control::Retry); - assert!(!code.has(Control::Retry)); + assert!(code.has(Control::Retry, &[])); + assert_eq!(code.action(Control::Retry, &[]), "retry"); + code.act(Control::Retry, &[]); + assert!(!code.has(Control::Retry, &[])); assert_eq!(code.ask(3), Some(Some("2".to_string()))); // A read of the first slice again that failed leaves the slices shown under a note that @@ -347,7 +347,7 @@ fn a_failure_says_why_and_a_later_slice_s_offers_retry() { "› M one +1 -1", ]; assert_eq!(rows(&code, 40), shown); - assert!(!code.has(Control::Retry)); + assert!(!code.has(Control::Retry, &[])); code.near_end(); assert_eq!(code.ask(3), None); assert!(code.stale(0)); @@ -381,7 +381,7 @@ diff --git a/two b/two code.near_end(); assert_eq!(code.ask(2), Some(Some("2".to_string()))); assert_eq!(code.take(2, Ok(slice(first, None)), 0), Some(false)); - code.act(Control::File(1)); + code.act(Control::File(1), &[]); // A read under way when the diff is wanted again is dropped when it comes. code.near_end(); @@ -428,8 +428,8 @@ diff --git a/two b/two +f "; let mut code = read(slice(first, Some("2")), 0); - code.act(Control::File(0)); - code.act(Control::File(1)); + code.act(Control::File(0), &[]); + code.act(Control::File(1), &[]); let fresh = rows(&code, 40); code.prepare(40, &theme()); // The rows the tab keeps are those it lays out afresh, and it shows those it keeps. @@ -445,8 +445,8 @@ diff --git a/two b/two // Unfolding another file, or the minute changing, lays out that file alone, and folding // one forgets its rows. - code.act(Control::File(2)); - code.act(Control::File(0)); + code.act(Control::File(2), &[]); + code.act(Control::File(0), &[]); code.prepare(40, &theme()); assert_eq!(code.bodies[&(0, 1)].lines.as_ptr(), one); let mut kept: Vec<_> = code.bodies.keys().copied().collect(); @@ -457,7 +457,7 @@ diff --git a/two b/two // A later slice keeps the rows of the files before it, unless it says the host withheld // one of them. - code.act(Control::File(0)); + code.act(Control::File(0), &[]); code.prepare(41, &theme()); assert_eq!(rows_of(&code.bodies[&(0, 0)].lines), ["No lines changed."]); let two = code.bodies[&(0, 2)].lines.as_ptr(); @@ -514,7 +514,7 @@ diff --git a/small b/small }, ]; let mut code = read(diff, 0); - code.act(Control::File(0)); + code.act(Control::File(0), &[]); assert_eq!( rows(&code, 60), [ @@ -556,8 +556,8 @@ fn an_empty_or_unreadable_diff_says_so() { let text = "diff --git a/a b/a\n@@ -x +1 @@\n+lost\ndiff --git a/b b/b\n@@ -1,2 +1,2 @@\n kept\n"; let mut code = read(slice(text, None), 0); - code.act(Control::File(0)); - code.act(Control::File(1)); + code.act(Control::File(0), &[]); + code.act(Control::File(1), &[]); let note = "t3term couldn't read some lines of this file's patch."; assert_eq!( rows(&code, 60), @@ -572,7 +572,7 @@ fn an_empty_or_unreadable_diff_says_so() { text.push_str(&"+x\n".repeat(patch::MOST_LINES)); text.push_str("diff --git a/next b/next\n@@ -1 +1 @@\n-a\n+b\n"); let mut code = read(slice(&text, None), 0); - code.act(Control::File(1)); + code.act(Control::File(1), &[]); let rows = rows(&code, 100); assert_eq!( rows[rows.len() - 2..], @@ -610,8 +610,8 @@ diff --git a/other b/other rows(&code, 40), ["2 files", "", "› A link +1 -0", "", "› M other +1 -1"] ); - assert!(code.has(Control::File(1))); - assert!(!code.has(Control::File(2))); + assert!(code.has(Control::File(1), &[])); + assert!(!code.has(Control::File(2), &[])); } #[test] @@ -643,7 +643,7 @@ fn every_row_fits_its_width_and_hostile_text_is_cleaned() { +new line that is rather long and has to wrap at narrow widths "; let mut code = read(slice(patch, Some("2")), 0); - code.act(Control::File(0)); + code.act(Control::File(0), &[]); for width in 1..=80 { let (lines, targets) = laid(&code, width); for line in &lines { @@ -715,7 +715,7 @@ fn with_threads( ) -> (Vec, Targets) { let showing = code.threads_shown(conversation.threads(), shown); conversation.lay_out_threads(&showing, width, &theme()); - let (lines, targets) = code.lay_out(conversation.threads(), shown, width, NOW, &theme()); + let (lines, targets) = code.lay_out(conversation.threads(), &[], shown, width, NOW, &theme()); (rows_of(&lines), targets) } @@ -763,7 +763,7 @@ fn review_threads_show_under_their_lines() { let lib = (0..code.files.len()) .find(|&at| code.file(at).unwrap().1.path() == "src/lib.rs") .unwrap(); - code.act(Control::File(lib)); + code.act(Control::File(lib), &[]); let mut shown = Shown::default(); let (rows, targets) = with_threads(&code, &mut conversation, &shown, 40); let at = rows @@ -870,7 +870,7 @@ fn review_threads_show_under_their_lines() { ); // Folding the file takes its threads away, with their controls. - code.act(Control::File(lib)); + code.act(Control::File(lib), &[]); assert!(!code.shows(Control::Thread(0), conversation.threads(), &shown)); assert_eq!( code.threads_shown(conversation.threads(), &shown), @@ -906,8 +906,8 @@ diff --git a/b b/b thread("short", "b", Some(3), "right", false, &["Short."]), ]); let mut code = read(slice(patch, None), 0); - code.act(Control::File(0)); - code.act(Control::File(1)); + code.act(Control::File(0), &[]); + code.act(Control::File(1), &[]); let (rows, _) = with_threads(&code, &mut conversation, &Shown::default(), 40); let card = |text: &str| { [ @@ -997,7 +997,7 @@ fn a_thread_reads_its_remarks_a_page_at_a_time() { first["nextCommentsCursor"] = json!("p2"); let mut conversation = conversation(vec![first]); let mut code = read(slice(ONE, None), 0); - code.act(Control::File(0)); + code.act(Control::File(0), &[]); let mut shown = Shown::default(); let (rows, targets) = with_threads(&code, &mut conversation, &shown, 40); let at = rows @@ -1076,9 +1076,9 @@ fn a_thread_reads_its_remarks_a_page_at_a_time() { shown.act(Control::Thread(0), &conversation); // Nor under a folded file. assert!(code.shows(Control::More(0), conversation.threads(), &shown)); - code.act(Control::File(0)); + code.act(Control::File(0), &[]); assert!(!code.shows(Control::More(0), conversation.threads(), &shown)); - code.act(Control::File(0)); + code.act(Control::File(0), &[]); assert!(conversation.want_more(0)); assert_eq!( conversation.take_wanted(10), @@ -1110,7 +1110,7 @@ fn a_thread_reads_no_page_past_what_t3term_keeps() { cut["nextCommentsCursor"] = json!("p2"); let mut conversation = conversation(vec![nearly, full, cut]); let mut code = read(slice(ONE, None), 0); - code.act(Control::File(0)); + code.act(Control::File(0), &[]); let shown = Shown::default(); // A thread that came with as many remarks as t3term keeps reads no page after them. @@ -1171,7 +1171,7 @@ fn pages_go_with_their_thread_when_the_conversation_is_read_again() { let mut new = conversation(threads(&["First.", "Second."], "fresh")); new.carry(&mut old); let mut code = read(slice(ONE, None), 0); - code.act(Control::File(0)); + code.act(Control::File(0), &[]); let shown = Shown::default(); let (rows, _) = with_threads(&code, &mut new, &shown, 40); for body in ["│ First.", "│ Second.", "│ Remark t-2."] { @@ -1269,14 +1269,14 @@ fn a_new_theme_restyles_the_threads() { let lib = (0..code.files.len()) .find(|&at| code.file(at).unwrap().1.path() == "src/lib.rs") .unwrap(); - code.act(Control::File(lib)); + code.act(Control::File(lib), &[]); let shown = Shown::default(); // The color of a remark's `text`, laid out at one width as the panel lays it out: the // thread's own and one from a page read after. let color = |conversation: &mut Conversation, theme: &Theme, text: &str| { let showing = code.threads_shown(conversation.threads(), &shown); conversation.lay_out_threads(&showing, 40, theme); - let (lines, _) = code.lay_out(conversation.threads(), &shown, 40, NOW, theme); + let (lines, _) = code.lay_out(conversation.threads(), &[], &shown, 40, NOW, theme); lines.iter().find_map(|line| { let span = line.spans.iter().find(|span| span.content.contains(text))?; line.style.patch(span.style).fg @@ -1339,14 +1339,15 @@ fn threads_fit_every_width_and_hostile_text_is_cleaned() { ); assert!(conversation.take_page(1, Err(hostile))); let mut code = read(slice(&patch, None), 0); - code.act(Control::File(0)); + code.act(Control::File(0), &[]); let mut shown = Shown::default(); shown.act(Control::OffDiff, &conversation); for width in 1..=80 { let showing = code.threads_shown(conversation.threads(), &shown); assert_eq!(showing, [0, 1]); conversation.lay_out_threads(&showing, width, &theme()); - let (lines, targets) = code.lay_out(conversation.threads(), &shown, width, NOW, &theme()); + let (lines, targets) = + code.lay_out(conversation.threads(), &[], &shown, width, NOW, &theme()); for line in &lines { let text = line.to_string(); // The Markdown renderer keeps 8 columns for text beside the bar's 2, and the @@ -1404,3 +1405,340 @@ fn threads_fit_every_width_and_hostile_text_is_cleaned() { assert_eq!(rows[at - 2], "│"); assert!(rows[at - 1].starts_with("│ mona")); } + +/// A conversation with `threads` and a commit for each of `commits`, as its id, headline and +/// time, in the order T3 sent them, and no remarks. +fn committed(threads: Vec, commits: &[(&str, &str, &str)]) -> Conversation { + let commits: Vec = commits + .iter() + .map(|(oid, headline, at)| { + json!({"oid": oid, "messageHeadline": headline, "committedDate": at}) + }) + .collect(); + let activity = json!({"comments": [], "commentCount": 0, "commentsTruncated": false, + "reviewThreads": threads, "commits": commits}); + Conversation::of(&pull_request::Activity::read(&activity).unwrap()) +} + +/// The tab's rows and targets at `width` with `conversation`'s threads and commits. +fn scoped( + code: &Code, + conversation: &Conversation, + shown: &Shown, + width: usize, +) -> (Vec, Targets) { + let commits = conversation.commits(); + let threads = conversation.threads(); + let (lines, targets) = code.lay_out(threads, &commits, shown, width, NOW, &theme()); + (rows_of(&lines), targets) +} + +/// An entry of the list of commits on a row of `width`: `lead`, then `text`, then `short` at +/// the end. +fn entry_row(lead: &str, text: &str, short: &str, width: usize) -> String { + let gap = width - lead.width() - text.width() - short.width(); + format!("{lead}{text}{}{short}", " ".repeat(gap)) +} + +#[test] +fn the_list_of_commits_picks_the_diff_the_tab_reads() { + // In the order T3 sent them: one in another zone, two at the same instant and one with + // no time t3term reads. + let on_line = thread( + "t1", + "src/lib.rs", + Some(2), + "right", + false, + &["On the new line."], + ); + let conversation = committed( + vec![on_line], + &[ + ("aaaaaaa1111", "First", "2026-10-15T08:00:00Z"), + ("bbbbbbb2222", "Zone", "2026-10-15T11:30:00+02:00"), + ("ccccccc3333", "Newest", "2026-10-15T10:00:00Z"), + ("ddddddd4444", "", "not a date"), + ("eeeeeee5555", "Same time", "2026-10-15T08:00:00Z"), + ], + ); + let commits = conversation.commits(); + let shown = Shown::default(); + let mut code = read(slice(PATCH, None), 0); + let (rows, targets) = scoped(&code, &conversation, &shown, 40); + assert_eq!(rows[..2], ["› All commits", "5 files"]); + assert_eq!(targets[0], (0..1, Control::Commits)); + assert_eq!(code.action(Control::Commits, &commits), "choose commit"); + for control in [ + Control::AllCommits, + Control::Commit(0), + Control::MoreCommits, + ] { + assert!(!code.has(control, &commits), "{control:?}"); + } + // Under the whole change, the thread is under its line. + assert!( + !rows.iter().any(|row| row.contains("conversation")), + "{rows:#?}" + ); + + // Newest first by when each was committed, ties in T3's order, the unreadable time last. + assert!(!code.act(Control::Commits, &commits)); + assert_eq!(code.action(Control::Commits, &commits), "hide commits"); + let (rows, targets) = scoped(&code, &conversation, &shown, 40); + assert_eq!( + rows[..9], + [ + "⌄ All commits".to_string(), + " ✓ All commits".into(), + entry_row(" ", "Newest", "ccccccc", 40), + entry_row(" ", "Zone", "bbbbbbb", 40), + entry_row(" ", "First", "aaaaaaa", 40), + entry_row(" ", "Same time", "eeeeeee", 40), + entry_row(" ", "Untitled commit", "ddddddd", 40), + String::new(), + "5 files".into(), + ] + ); + let entries = [Control::AllCommits] + .into_iter() + .chain((0..5).map(Control::Commit)); + let expected: Vec<_> = (1..7).map(|row| row..row + 1).zip(entries).collect(); + assert_eq!(targets[1..7], expected); + assert!(code.has(Control::Commit(4), &commits)); + assert!(!code.has(Control::Commit(5), &commits)); + assert!(!code.has(Control::MoreCommits, &commits)); + assert_eq!( + code.action(Control::AllCommits, &commits), + "show all commits" + ); + assert_eq!( + code.action(Control::Commit(1), &commits), + "show commit bbbbbbb" + ); + assert_eq!(code.action(Control::Commit(9), &commits), "show commit"); + + // Picking the diff shown closes the list and reads nothing. + code.act(Control::File(4), &commits); + assert!(!code.act(Control::AllCommits, &commits)); + assert!(!code.has(Control::AllCommits, &commits)); + assert_eq!(code.ask(2), None); + + // Another commit's diff reads at once from its first slice, and a read for the whole + // change under way is dropped when it comes. + code.again(); + assert_eq!(code.ask(2), Some(None)); + code.act(Control::Commits, &commits); + assert!(code.act(Control::Commit(1), &commits)); + assert_eq!(code.commit(), Some("bbbbbbb2222")); + assert!(!code.has(Control::Commit(1), &commits)); + assert_eq!(code.ask(3), Some(None)); + assert_eq!(code.take(2, Ok(slice(PATCH, None)), 10), None); + let (rows, targets) = scoped(&code, &conversation, &shown, 40); + assert_eq!(rows, ["› Zone · bbbbbbb", "Loading pull request diff…"]); + assert_eq!(targets, [(0..1, Control::Commits)]); + + // Its files show folded, and the thread goes off its diff though its line is in it. + assert_eq!(code.take(3, Ok(slice(PATCH, None)), 10), Some(true)); + let (rows, _) = scoped(&code, &conversation, &shown, 40); + assert_eq!( + rows[..4], + [ + "› Zone · bbbbbbb", + "5 files", + "", + "› 1 conversation not on the current diff" + ] + ); + assert!( + rows.contains(&"› R from.rs → to.rs".to_string()), + "{rows:#?}" + ); + code.act(Control::File(3), &commits); + assert!( + code.threads_shown(conversation.threads(), &shown) + .is_empty() + ); + + // A commit with no files says so. + code.act(Control::Commits, &commits); + assert!(code.act(Control::Commit(0), &commits)); + assert_eq!(code.ask(4), Some(None)); + assert_eq!(code.take(4, Ok(slice("", None)), 20), Some(true)); + let (rows, _) = scoped(&code, &conversation, &shown, 40); + assert_eq!( + rows, + [ + "› Newest · ccccccc", + "0 files", + "", + "This commit has no file changes." + ] + ); + + // All commits reads the whole change again, and a failure shows under the row. + code.act(Control::Commits, &commits); + assert!(code.act(Control::AllCommits, &commits)); + assert_eq!(code.commit(), None); + assert_eq!(code.ask(5), Some(None)); + let failure = Err("T3 couldn't read the diff.".to_string()); + assert_eq!(code.take(5, failure, 30), Some(true)); + let (rows, _) = scoped(&code, &conversation, &shown, 40); + assert_eq!(rows, ["› All commits", "T3 couldn't read the diff."]); + + // A commit picked after that reads its own diff, and the failure goes with the diff it + // was of. + code.act(Control::Commits, &commits); + assert!(code.act(Control::Commit(0), &commits)); + let (rows, _) = scoped(&code, &conversation, &shown, 40); + assert_eq!(rows, ["› Newest · ccccccc", "Loading pull request diff…"]); +} + +#[test] +fn the_list_shows_10_commits_at_a_time_and_a_rebase_takes_it_back_to_all() { + let oids: Vec = (0..23).map(|n| format!("{n:07}abc")).collect(); + let headlines: Vec = (0..23).map(|n| format!("Commit {n}")).collect(); + let times: Vec = (0..23) + .map(|n| format!("2026-10-15T08:{n:02}:00Z")) + .collect(); + let all: Vec<(&str, &str, &str)> = (0..23) + .map(|n| (oids[n].as_str(), headlines[n].as_str(), times[n].as_str())) + .collect(); + let conversation = committed(Vec::new(), &all); + let commits = conversation.commits(); + let shown = Shown::default(); + let mut code = read(slice(ONE, None), 0); + code.act(Control::Commits, &commits); + let more = |code: &Code| { + let (rows, targets) = scoped(code, &conversation, &shown, 40); + let at = targets + .iter() + .find(|(_, control)| *control == Control::MoreCommits) + .map(|(rows, _)| rows.start); + (at.map(|at| rows[at].clone()), rows) + }; + let (row, rows) = more(&code); + assert_eq!(row.as_deref(), Some(" Show more (13 left)")); + assert_eq!(rows[2], entry_row(" ", "Commit 22", "0000022", 40)); + assert_eq!(rows[11], entry_row(" ", "Commit 13", "0000013", 40)); + assert_eq!(rows[12..15], [" Show more (13 left)", "", "1 file"]); + assert!(code.has(Control::Commit(9), &commits)); + assert!(!code.has(Control::Commit(10), &commits)); + assert_eq!( + code.action(Control::MoreCommits, &commits), + "show more commits" + ); + + assert!(!code.act(Control::MoreCommits, &commits)); + assert_eq!(more(&code).0.as_deref(), Some(" Show more (3 left)")); + code.act(Control::MoreCommits, &commits); + let (row, rows) = more(&code); + assert_eq!(row, None); + assert_eq!(rows[24], entry_row(" ", "Commit 0", "0000000", 40)); + assert!(code.has(Control::Commit(22), &commits)); + assert!(!code.has(Control::MoreCommits, &commits)); + + // Another commit lists 10 again when the list opens. + assert!(code.act(Control::Commit(15), &commits)); + assert_eq!(code.commit(), Some("0000007abc")); + code.act(Control::Commits, &commits); + let (row, rows) = more(&code); + assert_eq!(row.as_deref(), Some(" Show more (13 left)")); + assert_eq!(rows[..2], ["⌄ Commit 7 · 0000007", " All commits"]); + + // A read of the conversation that still lists the commit keeps it. One that doesn't, as + // after a force-push, reads the whole change again, the list closed. + assert!(!code.follow(&commits)); + assert_eq!(code.ask(2), Some(None)); + let kept = committed(Vec::new(), &all[..5]); + assert!(code.follow(&kept.commits())); + assert_eq!(code.commit(), None); + assert!(!code.has(Control::AllCommits, &kept.commits())); + assert_eq!(code.ask(3), Some(None)); + assert!(!code.follow(&kept.commits())); + + // A conversation with no commits has no row to pick one. + let none = committed(Vec::new(), &[]); + assert!(!code.has(Control::Commits, &none.commits())); + assert_eq!(code.take(3, Ok(slice(ONE, None)), 10), Some(true)); + let (rows, targets) = scoped(&code, &none, &shown, 40); + assert_eq!(rows[..3], ["1 file", "", "› M a +1 -1"]); + assert_eq!(targets, [(2..3, Control::File(0))]); +} + +#[test] +fn the_list_of_commits_fits_every_width_and_cleans_its_headlines() { + let hostile = "Fix \u{1b}[2J the \u{7}bell, 漢字漢字, and a headline long enough to wrap"; + let conversation = committed( + Vec::new(), + &[ + ("f00d\u{1b}]0;x", hostile, "2026-10-15T08:00:00Z"), + ("beadbeef", "Two", "2026-10-15T09:00:00Z"), + ], + ); + let commits = conversation.commits(); + let shown = Shown::default(); + let mut code = read(slice(ONE, None), 0); + code.act(Control::Commits, &commits); + assert!(code.act(Control::Commit(1), &commits)); + // T3 gets the id as it sent it. + assert_eq!(code.commit(), Some("f00d\u{1b}]0;x")); + code.act(Control::Commits, &commits); + let (rows, _) = scoped(&code, &conversation, &shown, 80); + let cleaned = "Fix [2J the bell, 漢字漢字, and a headline long enough to wrap"; + assert_eq!( + rows[..5], + [ + format!("⌄ {cleaned} · f00d]0;"), + " All commits".into(), + entry_row(" ", "Two", "beadbee", 80), + entry_row(" ✓ ", cleaned, "f00d]0;", 80), + String::new(), + ] + ); + for width in 1..=80 { + let threads = conversation.threads(); + let (lines, targets) = code.lay_out(threads, &commits, &shown, width, NOW, &theme()); + for line in &lines { + let text = line.to_string(); + assert!(text.width() <= width, "{width}: {text:?}"); + assert!( + !text.contains(|c: char| c.is_control()), + "{width}: {text:?}" + ); + } + assert!(targets.iter().all(|(rows, _)| rows.end <= lines.len())); + // Each entry is one row, and the row that opens the list wraps under itself. + let entries: Vec<_> = targets + .iter() + .filter(|(_, control)| matches!(control, Control::AllCommits | Control::Commit(_))) + .collect(); + assert_eq!(entries.len(), 3, "{width}"); + assert!(entries.iter().all(|(rows, _)| rows.len() == 1), "{width}"); + let rows = rows_of(&lines); + // The id shows beside at least four columns of the headline. + let two = &rows[targets[2].0.start]; + assert_eq!( + two.ends_with("beadbee"), + width >= 4 + 4 + 1 + 7, + "{width}: {two:?}" + ); + // Where the width leaves no room beside the lead, the headline shows without it. + if (2..=4).contains(&width) { + assert!(two.starts_with('T'), "{width}: {two:?}"); + } + } + + // The row closes the list as it opened it, and an open list has nothing to pick once the + // conversation lists no commits. + code.act(Control::Commits, &commits); + assert!(!code.has(Control::AllCommits, &commits)); + let (rows, _) = scoped(&code, &conversation, &shown, 80); + assert_eq!( + rows[..2], + [format!("› {cleaned} · f00d]0;"), LOADING.into()] + ); + code.act(Control::Commits, &commits); + assert!(code.has(Control::AllCommits, &commits)); + assert!(!code.has(Control::AllCommits, &[])); +} diff --git a/src/tui/pull_requests/conversation.rs b/src/tui/pull_requests/conversation.rs index 1f24a71..445fee3 100644 --- a/src/tui/pull_requests/conversation.rs +++ b/src/tui/pull_requests/conversation.rs @@ -40,8 +40,7 @@ use super::super::theme::{Theme, age, parse_iso_ms}; use super::super::{clip, row}; use super::{Answer, FAILURE, NAME, flowed, grouped, whole}; use crate::pull_request::{ - self, Actor, Comment, CommentKind, Commit, Reaction, ReactionContent, ReviewThread, Side, - ThreadComment, + self, Actor, Comment, CommentKind, Reaction, ReactionContent, ReviewThread, Side, ThreadComment, }; use crate::transcript::without_controls; @@ -49,7 +48,7 @@ mod thread; mod timeline; use timeline::Timeline; -pub(super) use timeline::{Lifecycle, section as timeline_section}; +pub(super) use timeline::{Commit, Lifecycle, section as timeline_section}; /// How many active remarks show, as the desktop's first page of the conversation holds /// (`COMMENT_PAGE`). @@ -140,10 +139,22 @@ impl Shown { } } Control::OffDiff => self.off_diff_open = !self.off_diff_open, - Control::File(_) | Control::Retry | Control::More(_) => {} + Control::File(_) + | Control::Retry + | Control::More(_) + | Control::Commits + | Control::AllCommits + | Control::Commit(_) + | Control::MoreCommits => {} } } + /// Closes the Code tab's list of the conversations off the diff, as the desktop does when + /// the reader picks another commit's diff (PullRequestCodeTab.tsx:268-277). + pub(super) fn close_off_diff(&mut self) { + self.off_diff_open = false; + } + /// Whether `thread` shows its remarks: as the reader left it, or else unless resolved. pub(super) fn thread_open(&self, thread: &Thread) -> bool { self.threads @@ -221,9 +232,32 @@ pub(super) enum Control { /// Reads the next page of a review thread's remarks, by the thread's place as for /// `Thread`. More(usize), + /// Opens or closes the Code tab's list of commits, which picks the diff it shows. + Commits, + /// Shows the diff of the whole change on the Code tab, from its list of commits. + AllCommits, + /// Shows a commit's own diff on the Code tab, by its place among the commits newest + /// first. + Commit(usize), + /// Lists up to 10 more commits in the Code tab's list. + MoreCommits, } impl Control { + /// Whether the Code tab's own state, rather than the conversation's, says what it does: + /// a file, Retry and the list of commits. + pub(super) fn on_code(self) -> bool { + matches!(self, Control::File(_) | Control::Retry) || self.on_commits() + } + + /// Whether it is the list of commits, which stays above the files when a diff comes. + pub(super) fn on_commits(self) -> bool { + matches!( + self, + Control::Commits | Control::AllCommits | Control::Commit(_) | Control::MoreCommits + ) + } + /// The control that takes this one's place when it goes: Show only 10 recent once every /// older remark shows, and Show older once only 10 do. pub(super) fn partner(self) -> Option { @@ -521,6 +555,8 @@ pub(super) struct Conversation { places: Vec, /// The commits, in the order T3 sent them. commits: Vec, + /// The places of the commits, newest first, as the Code tab lists them. + newest: Vec, /// The author's login, where the host gave it with the conversation. It takes the place of /// the detail's. author: Option, @@ -574,6 +610,13 @@ impl Conversation { .reviewers .as_ref() .map(|reviewers| reviewers.iter().map(Reviewer::of).collect()); + let commits: Vec = + activity.commits.iter().map(timeline::Commit::of).collect(); + // By when each was committed, not by its text, as the desktop sorts them + // (PullRequestCodeTab.tsx:683-692). Commits at the same instant keep the order T3 sent + // them in, and one with no time T3 could read goes last. + let mut newest: Vec = (0..commits.len()).collect(); + newest.sort_by_key(|&at| std::cmp::Reverse(commits[at].at)); Conversation { count: activity.comment_count, truncated: activity.comments_truncated, @@ -588,7 +631,8 @@ impl Conversation { cut: activity.cut, verdicts_cut: activity.verdicts_cut, places, - commits: activity.commits.iter().map(timeline::Commit::of).collect(), + commits, + newest, author: activity .author .as_ref() @@ -603,6 +647,11 @@ impl Conversation { &self.threads } + /// The commits, newest first, as the Code tab's list of commits shows them. + pub(super) fn commits(&self) -> Vec<&Commit> { + self.newest.iter().map(|&at| &self.commits[at]).collect() + } + /// Lays the text of each remark of the threads at `at` out at `width`, unless it already /// is. The Code tab says which threads show. pub(super) fn lay_out_threads(&mut self, at: &[usize], width: usize, theme: &Theme) { @@ -786,7 +835,11 @@ impl Conversation { | Control::Retry | Control::Thread(_) | Control::OffDiff - | Control::More(_) => false, + | Control::More(_) + | Control::Commits + | Control::AllCommits + | Control::Commit(_) + | Control::MoreCommits => false, } } @@ -830,7 +883,12 @@ impl Conversation { } Control::OffDiff => toggle(shown.off_diff_open, "conversations off the diff"), Control::More(_) => "load more comments".to_string(), - Control::File(_) | Control::Retry => String::new(), + Control::File(_) + | Control::Retry + | Control::Commits + | Control::AllCommits + | Control::Commit(_) + | Control::MoreCommits => String::new(), } } } @@ -1063,7 +1121,7 @@ fn state_label(state: &str) -> String { /// for each login, whatever order the host sent them in, with dismissed ones left out. Two /// reviews with no author are two reviewers. A verdict given before the newest commit is /// stale. A time that doesn't parse can't be said to come before or after another. -fn verdicts(comments: &[Comment], commits: &[Commit]) -> Vec { +fn verdicts(comments: &[Comment], commits: &[pull_request::Commit]) -> Vec { let newest = commits .iter() .filter_map(|commit| instant(&commit.committed_at)) diff --git a/src/tui/pull_requests/conversation/timeline.rs b/src/tui/pull_requests/conversation/timeline.rs index c945b8c..4784aa6 100644 --- a/src/tui/pull_requests/conversation/timeline.rs +++ b/src/tui/pull_requests/conversation/timeline.rs @@ -69,12 +69,14 @@ impl Lifecycle { } /// A commit on the branch, cleaned and bounded once, when the conversation arrives. -pub(super) struct Commit { +pub(in super::super) struct Commit { + /// Its id as T3 sent it, which a read of its own diff names. + pub(in super::super) oid: String, /// The first seven characters of its id, as the desktop shows them. - short: String, + pub(in super::super) short: String, /// Its headline, which may be empty. headline: String, - at: Option, + pub(super) at: Option, /// The lines it adds and removes, where the host counted both. counts: Option<(u64, u64)>, /// Its first author's login, which stands in for the desktop's avatar. @@ -85,6 +87,7 @@ impl Commit { pub(super) fn of(commit: &pull_request::Commit) -> Commit { let author = commit.authors.as_deref().and_then(<[_]>::first); Commit { + oid: commit.oid.clone(), short: bounded(&commit.oid, NAME).chars().take(7).collect(), headline: bounded(&commit.headline, TITLE), at: instant(&commit.committed_at), @@ -92,6 +95,15 @@ impl Commit { author: author.map(|actor| bounded(&actor.login, NAME)), } } + + /// Its headline, or `Untitled commit` where it has none, as the desktop's `CommitEvent` + /// says. + pub(in super::super) fn headline(&self) -> &str { + match self.headline.is_empty() { + true => "Untitled commit", + false => &self.headline, + } + } } /// What an event on the Timeline is. @@ -460,10 +472,7 @@ fn plural(count: u64, what: &str) -> String { fn commit_lines(commit: &Commit, width: usize, now: i64, theme: &Theme) -> Vec> { let gutter = gutter(width); let strong = Style::new().fg(theme.fg).add_modifier(Modifier::BOLD); - let headline = match commit.headline.is_empty() { - true => "Untitled commit", - false => &commit.headline, - }; + let headline = commit.headline(); // No counts show where the host counted none, as the desktop's `PullRequestDiffStat` // leaves `+0 -0` out. let counts = match commit.counts {