chadtmiller.com/knotless
git that lives in your Atmosphere account
A pull's card counts what it changes, and cards show when, not state
MergedThe pull adds a stats row to a pull's social card: comments, reviews, files, +/− lines and five squares. The card's footer now shows the commit count. The counts come from rev-list --count and diff --shortstat over the same range as the pull's diff. The range handling, the fallback to no size, the shortstat parsing and the docs update all look right.
One bug: Size::squares doesn't do what its doc comment and the pull body say. When one side is tiny next to the other, the tiny side gets no square. Details are in the inline comment. Please fix the function and add a test case for it.
crates/knotless-serve/src/card.rsline 424Outdated
return (0, 0); | |
} | |
let red = (5 * self.removed).div_ceil(all); | |
(5 - red, red) |
The doc says each side that changed anything shows at least one square, but this doesn't hold. With 1 added and 1000 removed, red = ceil(5000/1001) = 5, so the result is (0, 5) and the added side vanishes. Clamp so that each non-zero side keeps one square. For example, set red = red.clamp(usize::from(self.removed > 0), 5 - usize::from(self.added > 0)). Add a test for size(1, 1000).
The pull is much as before and the rest of it looks right. But Size::squares is still wrong, so my earlier request isn't fully met.
red = ceil(5 * removed / all) guarantees the removed side a square. It doesn't guarantee one to the added side. With 1 added and 1000 removed, red is ceil(4.995), which is 5, so the result is (0, 5). The added line gets no square, though the doc comment says each side that changed anything shows at least one. The new test cases don't cover this direction. size(0, 3) is a case where added is 0, which is fine.
Clamp the split so each non-zero side keeps one square. For example, compute red and then, when added > 0 and removed > 0, clamp it to 1..=4. Then add size(1, 1000), expecting (1, 4), to the squares test.
crates/knotless-serve/src/card.rsline 424Outdated
return (0, 0); | |
} | |
let red = (5 * self.removed).div_ceil(all); | |
(5 - red, red) |
(5 - red, red) gives (0, 5) for 1 added and 1000 removed, so the added side shows no square. Clamp red to 1..=4 when both sides are non-zero, and add a test case for it.
Size::squares is unchanged from the last round, so my request is still open. With added = 1 and removed = 1000, red = ceil(4.995) = 5, so the result is (0, 5). The doc comment says each side that changed anything shows at least one square. Clamp red to 1..=4 when both sides are non-zero. Then add size(1, 1000), expecting (1, 4), to the squares test.
crates/knotless-serve/src/card.rsline 425Outdated
return (0, 0); | |
} | |
let red = (5 * self.removed).div_ceil(all); | |
(5 - red, red) |
With added = 1 and removed = 1000 this gives red = 5, so green gets no square. When both added and removed are > 0, clamp red to 1..=4. Add a size(1, 1000) == (1, 4) case to the test.
Size::squares is still unchanged from my last three reviews, so the request is still open. With added = 1 and removed = 1000, red = ceil(4.995) = 5, so the result is (0, 5). The doc comment says each side that changed anything shows at least one square.
The fix: when both added and removed are non-zero, clamp red to 1..=4. Then add size(1, 1000), expecting (1, 4), to the squares test. The rest of the pull looks right.
crates/knotless-serve/src/card.rsline 419
/// Each side that changed anything shows at least one. | |
fn squares(self) -> (usize, usize) { | |
let all = self.added + self.removed; | |
if all == 0 { |
red can be 5 while added > 0, which leaves green with no square. Clamp it to 1..=4 when both sides are non-zero.
This merged before the fix reached it. The clamp and the size(1, 1000) test are in A pull's card keeps a green square for a line added beside many removed.
8 passed, 2 skipped
What changes
A pull's social card now counts what GitHub's cards do. A row above the foot shows its comments, its reviews and the files it changes, then the lines it adds and removes in green and red, with five squares of their share. The foot reads the author, the day it was opened and the pull's commits.
Neither a pull's nor an issue's card shows its state any more. The Open, Merged or Closed pill is gone, and the bar along the foot takes the owner's colour, as a repository's card does. An issue's foot reads its author, the day it was opened and its comments.
A title that wraps onto three lines is set at 58px rather than 72px on a pull's card, to leave the row its room.
Why
A shared link to a pull said little beyond its title: who opened it and how many comments it had. GitHub's card shows how big the change is at a glance.
The state went because a card does not change once shared: Bluesky stores the picture with the post, and Slack and Discord cache what they unfurled. A card saying Open on a pull merged since is wrong, where the day it was opened stays true and the counts read as of then.
The counts come from
git rev-list --countandgit diff --shortstatover the range the pull's diff shows, so they match the page. Both the page (whose card URL is a hash of the SVG) and the card's PNG route count the same way. When the range cannot be read, as when the head's branch is gone, the card leaves the files, lines and commits out rather than showing zeros.How it was checked
card.rstests draw every kind of card, including a pull with a three-line title and no size, and check the squares' split.web.rstests parse--shortstatlines. The cards example (just bench-cards) draws them, looked at here: a short and a three-line pull, and an issue. CI runs the rest.Shortcomings
Each card request and each pull page now runs two more git commands on the pull's mirror. They are cheap next to the diff the page already reads, but the PNG route did not read git before.
🤖 Generated with Claude Code