chadtmiller.com/knotless

Switch repository

Loading…

What changes

A diff's files come in the order an editor's file tree lists them. In each directory, its directories come before its files, each by name, ignoring case and comparing runs of digits as numbers (v2 before v10): crates/, docs/, web/, then Cargo.lock, justfile, README.md. The server sorts them (diff::sort, called in web.rs's files), so a pull's files page, a commit's page, getThread's changes, getCommit and getPullDiff all answer in that order. The lexicons now describe it. diff::File::path gives the path a file goes by, which file_diff now uses as well.

The file tree lists each directory's entries in the diffs' order (FileTree.svelte), instead of putting directories before files on its own, so reading the tree from top to bottom always matches reading the diffs.

A file reached from the tree stays where it was jumped to. PullDiff.svelte holds the clicked file, or the one a #diff-N link opened at, at the top of the screen. A ResizeObserver on the diffs scrolls it back each time a file above it is drawn or gets its rows. The hold ends when the reader scrolls, touches, presses a key or clicks.

Why

The tree put directories first, but the diffs came in git's byte order, which puts Cargo.lock before crates/. Files at the bottom of the tree showed at the top of the diffs. Sorting on the server, rather than reordering in the browser, keeps firstRows useful: the first files to arrive with their rows are the first on the page.

In WebKit (Safari, and every browser on an iPhone), clicking a file in the tree often landed on the wrong one, and a second click fixed it. Files away from the screen are drawn at a guessed height (content-visibility: auto, --diff-file-guess), and the files near the target fetch their rows after the jump. Both grow the content above the target. Chromium adjusts the scroll for content that grows above it; WebKit does not, so the target moved down.

How it was checked

On knotless.dev's 275-file pull 3myr3azoen3az, in Playwright, I clicked tree links and measured where the clicked diff's top landed (16px is right):

The "after" column ran on this branch's pages, built locally against knotless.dev's XRPC. Wheel-scrolling after a jump moves the page as it should. svelte-check passes with no warnings, and just lex passes.

diff::tests::sorts_as_a_file_tree covers directories before files, case, numbers, numbers written with a leading zero, a name that is a prefix of another, and a deleted file a beside an added a/b. No recorded answer in harness/shots/fixtures.json has its files out of this order, so no shots should move. The rest is left to the checks.

Shortcomings

In WebKit, dragging the scrollbar may not end the hold: if a file above finishes loading during the drag, the page can jump back to the clicked file. The order follows what VS Code's explorer shows, not every editor's.

The sort, the tree, the lexicon text and the docs all look right. The pull also adds a change file about a different feature, and that needs to go before it lands.

• Sort: I walked through in_tree and by_name. They give a consistent total order, and the leading-zero, prefix and deleted-file cases behave as the test says. I did not run anything.
• Tree: FileTree.svelte now relies on the server's order, and every path that feeds it is sorted server-side.
• Change file: .changeset/owner-watches-private.md is about knotless-agent --owner and repo share. Nothing else in this pull touches that. It would put a wrong line in CHANGELOG and trigger a minor bump. This pull changes only knotless-serve and web, so it needs no change file under AGENTS.md. Remove it, probably left over from another branch.
• Minor, PullDiff.svelte: hold() reads the diffs state, and the mount $effect calls it. The effect therefore tracks diffs and will re-run if that binding changes. This is harmless today. You could read it with untrack to be safe.

Both points from my earlier review are done. .changeset/owner-watches-private.md is removed, and no unrelated change file is left in .changeset. The mount $effect in PullDiff.svelte now calls hold inside untrack, so it no longer re-runs when diffs changes. The new test assertions (in_tree("a", "a/b") is Greater, and equal paths are Equal) match the order the pull describes. I only read the diff since my last review and ran nothing.