Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 19 additions & 1 deletion examples/corpus.html
Original file line number Diff line number Diff line change
Expand Up @@ -842,13 +842,31 @@ <h3>One unchanged root, replacement calls in context</h3>
></lf-call-diff>
</lf-tab>
</lf-tabs>
<h3 id="pr-hunk-heading">The default the picker now opens on</h3>
<p id="pr-hunk-intro">
The one hunk the rest of the change is arranged around: the picker asks for a
prepared diff instead of assembling its own <code>git diff</code> arguments.
</p>
<lf-diff id="pr-key-hunk">
<pre>
diff --git a/src/git/diff.rs b/src/git/diff.rs
--- a/src/git/diff.rs
+++ b/src/git/diff.rs
@@ -112,4 +112,3 @@ impl Repository {
pub fn picker_preview(&amp;self, worktree: &amp;Worktree) -&gt; Result&lt;Diff&gt; {
- let args = ["diff", "--patch", "--no-color", "--find-renames", "--diff-algorithm=histogram", "HEAD"];
- Diff::parse(&amp;self.git(&amp;args, worktree.path())?)
+ PreparedDiff::for_worktree(self, worktree)?.unified()
}
</pre>
</lf-diff>
<h3 id="pr-invariant-heading">Review invariants</h3>
<p id="pr-invariant-intro">
The implementation is expected to preserve these three properties.
</p>
<lf-code id="pr-review-invariants" language="rust" hi="1-3">
<pre>
assert_eq!(real_index_after, real_index_before);
assert_eq!(repo.read_index_bytes(&amp;worktree)?, index_before_preview, "preview preparation rewrote {worktree:?}");
assert!(temp_index.is_operation_scoped());
assert!(offscreen_worktree_preview.is_demand_loaded());
</pre>
Expand Down
20 changes: 19 additions & 1 deletion examples/pr-walkthrough.html
Original file line number Diff line number Diff line change
Expand Up @@ -359,13 +359,31 @@ <h3>One unchanged root, replacement calls in context</h3>
></lf-call-diff>
</lf-tab>
</lf-tabs>
<h3 id="pr-hunk-heading">The default the picker now opens on</h3>
<p id="pr-hunk-intro">
The one hunk the rest of the change is arranged around: the picker asks for a
prepared diff instead of assembling its own <code>git diff</code> arguments.
</p>
<lf-diff id="pr-key-hunk">
<pre>
diff --git a/src/git/diff.rs b/src/git/diff.rs
--- a/src/git/diff.rs
+++ b/src/git/diff.rs
@@ -112,4 +112,3 @@ impl Repository {
pub fn picker_preview(&amp;self, worktree: &amp;Worktree) -&gt; Result&lt;Diff&gt; {
- let args = ["diff", "--patch", "--no-color", "--find-renames", "--diff-algorithm=histogram", "HEAD"];
- Diff::parse(&amp;self.git(&amp;args, worktree.path())?)
+ PreparedDiff::for_worktree(self, worktree)?.unified()
}
</pre>
</lf-diff>
<h3 id="pr-invariant-heading">Review invariants</h3>
<p id="pr-invariant-intro">
The implementation is expected to preserve these three properties.
</p>
<lf-code id="pr-review-invariants" language="rust" hi="1-3">
<pre>
assert_eq!(real_index_after, real_index_before);
assert_eq!(repo.read_index_bytes(&amp;worktree)?, index_before_preview, "preview preparation rewrote {worktree:?}");
assert!(temp_index.is_operation_scoped());
assert!(offscreen_worktree_preview.is_demand_loaded());
</pre>
Expand Down
14 changes: 13 additions & 1 deletion skills/leaf/packages/default/theme.css
Original file line number Diff line number Diff line change
Expand Up @@ -1576,9 +1576,15 @@ lf-chart .lf-chart-swatch { width: 9px; height: 9px; border-radius: 2px;
font-weight: 650; }
.lf-diff-filtered { display: none !important; }
.lf-diff-next:is(:focus-visible, .lf-focus-visible),
.lf-diff-review:is(:focus-visible, .lf-focus-visible),
.lf-diff-search:is(:focus-visible, .lf-focus-visible) { outline: var(--here-ring);
--lf-here-ring: diff-tools;
outline-offset: 1px; }
/* The review press rides the file's own summary row, inside the box that clips it, so
it takes the inset the summary below takes and not the toolbar's outward gap: drawn
outward its lower run fell past the bottom the details clips at. */
.lf-diff-review:is(:focus-visible, .lf-focus-visible) { outline: var(--here-ring);
--lf-here-ring: diff-review;
outline-offset: calc(-1 * var(--here-ring-w)); }
/* The file's own row, and the box it opens clips (overflow above), so the ring it wears
is inset — theme.css's rule for a control packed into something rather than standing
on its own. Outside, its top run was drawn into the border the details draws and lost
Expand All @@ -1603,6 +1609,12 @@ lf-chart .lf-chart-swatch { width: 9px; height: 9px; border-radius: 2px;
--diffs-bg-separator: var(--code-chrome);
--diffs-fg: var(--code-ink);
--diffs-fg-number: var(--code-muted);
/* A changed row's number is drawn in that row's own base by default, which is the
ink the fill beside it is mixed from: green on green at 4.03:1 in the light
palette. The tinted-surface inks are the layer's answer to exactly that question
and keep the number the same statement, legibly. */
--diffs-fg-number-addition-override: var(--ok-ink);
--diffs-fg-number-deletion-override: var(--danger-ink);
--diffs-addition-base: var(--ok);
--diffs-deletion-base: var(--danger);
--diffs-bg-addition: color-mix(in srgb, var(--ok) 16%, var(--pre-bg));
Expand Down
20 changes: 13 additions & 7 deletions skills/leaf/packages/pr-review/theme.css
Original file line number Diff line number Diff line change
Expand Up @@ -121,10 +121,15 @@ lf-pull-request {
font: var(--t-4) var(--mono);
}

/* The card is already a tint of the accent over the page's paper, so a second
* translucent accent here stacked into a surface the layer's own --muted ink could not
* clear: the label and any emphasis inside the author's words read at 4.32:1. The
* raised-surface token separates the quoted block from the card by going lighter
* instead, which the accent rule beside it was doing the work of anyway. */
.lf-pr-description {
padding: var(--sp-3);
border-inline-start: 3px solid var(--accent);
background: color-mix(in srgb, var(--accent) 5%, transparent);
background: var(--card);
}

.lf-pr-description-label,
Expand Down Expand Up @@ -268,7 +273,10 @@ lf-call-diff {
font-size: var(--t-6);
}

.lf-call-toggle {
/* Doubled with .lf-ui, which `offer` writes on every control a widget injects: the
* runtime's chrome face and ink are stated at one class, so a package rule that names
* only its own class loses the face it asked for. */
.lf-call-toggle.lf-ui {
flex: none;
padding: 3px 9px;
border: 1px solid var(--code-border);
Expand Down Expand Up @@ -420,7 +428,9 @@ lf-call-diff {
text-underline-offset: 2px;
}

.lf-call-location:hover {
/* Only where there is somewhere to go: a row that names its location without linking it
* must not answer the pointer as though it did. */
.lf-call-location[href]:hover {
color: var(--accent);
}

Expand Down Expand Up @@ -450,10 +460,6 @@ lf-call-diff {
background: transparent;
}

.lf-call-toggle {
display: none;
}

.lf-call-group:not([open]) > .lf-call-group-body {
display: block;
}
Expand Down
61 changes: 39 additions & 22 deletions skills/leaf/packages/pr-review/widgets/lf-call-diff.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import {
announce,
navigateToDatum,
offer,
projectData,
watchData,
} from "/runtime/widget-api.js";
Expand Down Expand Up @@ -108,19 +109,24 @@ function updateDisclosureControl(owner) {

function buildToolbar(owner) {
const toolbar = make("div", "lf-call-tools");
// The counts this widget writes are an account of the tree, not words the page holds,
// so `data-lf-gen` takes them out of the version diff and makes each its own passage
// cell — the marker `lf-diff` puts on its own injected stat. It does not stop a drag
// quoting them: that is `.lf-ui`, which `lf-diff` adds beside it on its line numbers.
const summary = make("p", "lf-call-summary");
Comment thread
leaf-agent marked this conversation as resolved.
const button = make("button", "lf-call-toggle");
toolbar.dataset.lfUi = "";
button.type = "button";
summary.dataset.lfGen = "1";
// `offer`, not a bare button: the disclosure control is chrome this widget injected
// and a handler is all it ever was, so the markers it writes are what tells the
// exported copy to take the press away rather than draw a hand over a dead one.
const button = offer("button", "lf-call-toggle");
button.addEventListener("click", () => {
const groups = [...owner.querySelectorAll(":scope > .lf-call-group")];
const open = groups.some((group) => !group.open);
for (const group of groups) group.open = open;
updateDisclosureControl(owner);
announce(`${open ? "Expanded" : "Collapsed"} all call-tree roots`);
});
toolbar.append(summary);
if (!owner.preparingExport) toolbar.append(button);
toolbar.append(summary, button);
return toolbar;
}

Expand Down Expand Up @@ -180,13 +186,22 @@ function renderLine(record, prior, owner) {
setText(body, record.body);
setText(location, record.location);
location.hidden = !record.location;
if (owner.preparingExport) location.removeAttribute("href");
else location.href = `#${owner.getAttribute("diff")}`;
location.onclick = async (event) => {
event.preventDefault();
event.stopPropagation();
await travelToLine(owner, record);
};
// The header row names no location, so its anchor is hidden — and an `href` on a
// hidden anchor is a way in that leads nowhere. Worse, `reachScrollers` reads a
// candidate for a focusable descendant before granting the stop, and a hidden
// `a[href]` is one: the header's own words run off the side, and the live page
// answered "there is already a way in here" with a link nobody can reach.
if (record.location && !owner.preparingExport) {
location.href = `#${owner.getAttribute("diff")}`;
location.onclick = async (event) => {
event.preventDefault();
event.stopPropagation();
await travelToLine(owner, record);
};
} else {
location.removeAttribute("href");
location.onclick = null;
}
return line;
}

Expand Down Expand Up @@ -220,10 +235,14 @@ customElements.define(
this.stopWatching = null;
}

// In a copy the anchor can only reach the patch, never the line it names — and on a
// group's root row, which is the disclosure's own `<summary>`, it is a focusable
// descendant of a disclosure as well. So the copy keeps each location as text.
// The toggle needs nothing here: `offer` marked it, and the bake takes a marked
// press away on its own. Nor do the counts, which stay, because an account of the
// tree is something a reader still wants on paper.
lfPrepareExport() {
this.preparingExport = true;
this.querySelector(":scope > .lf-call-tools .lf-call-toggle")?.remove();
for (const count of this.querySelectorAll(".lf-call-group-count")) count.remove();
for (const location of this.querySelectorAll(".lf-call-location"))
location.removeAttribute("href");
}
Expand Down Expand Up @@ -322,15 +341,13 @@ customElements.define(
const groupRecords = records.filter((record) => record.groupKey === key);
const root = groupRecords.find((record) => record.root);
const rootNode = nodesByKey.get(root.key);
if (!this.preparingExport) {
let count = rootNode.querySelector(".lf-call-group-count");
if (!count) {
count = make("span", "lf-call-group-count");
count.dataset.lfUi = "";
rootNode.append(count);
}
setText(count, groupLabel(groupRecords));
let count = rootNode.querySelector(".lf-call-group-count");
if (!count) {
count = make("span", "lf-call-group-count");
count.dataset.lfGen = "1";
rootNode.append(count);
}
setText(count, groupLabel(groupRecords));
reconcileChildren(
parts.body,
groupRecords
Expand Down
35 changes: 33 additions & 2 deletions skills/leaf/scripts/leaf/render-checks/standalone.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,35 @@ export function coveredWords({
}
return false;
};
// What a run paints, which is not the whole of its rect. A box that clips its overflow
// shows only the part inside it, so a name ellipsised in a narrow column reads as
// covering whatever stands beside it while the reader sees the ellipsis and nothing
// else — the CallDiff root's own row, where the copy revealed a tab the live page keeps
// closed. Every clipping ancestor is intersected in, so the reading is the one the
// reader is given. The walk below stays in the light DOM, so the climb does too.
const painted = (el, drawn) => {
let box = drawn;
for (let ancestor = el; ancestor && box; ancestor = ancestor.parentElement) {
const style = getComputedStyle(ancestor);
if (style.overflowX !== "visible" || style.overflowY !== "visible") {
const bounds = ancestor.getBoundingClientRect();
const left = Math.max(box.left, bounds.left);
const right = Math.min(box.right, bounds.right);
const top = Math.max(box.top, bounds.top);
const bottom = Math.min(box.bottom, bounds.bottom);
box =
right > left && bottom > top
? new DOMRect(left, top, right - left, bottom - top)
: null;
}
// An out-of-flow box is laid out against its containing block rather than against
// the ancestry, so a hidden overflow further out need not reach it at all. Stop
// climbing there and keep the rect whole: over-reporting a cover is this reading's
// safe direction, and missing one is the fault it was written for.
if (style.position === "absolute" || style.position === "fixed") break;
}
return box;
};
const walk = document.createTreeWalker(document.body, NodeFilter.SHOW_TEXT);
for (let node = walk.nextNode(); node; node = walk.nextNode()) {
const el = node.parentElement;
Expand All @@ -41,9 +70,11 @@ export function coveredWords({
const range = document.createRange();
range.selectNodeContents(node);
const label = el.closest("text");
for (const box of range.getClientRects())
if (box.width > 1 && box.height > 1)
for (const drawn of range.getClientRects()) {
const box = painted(el, drawn);
if (box && box.width > 1 && box.height > 1)
runs.push({ el, label, box, text: node.data.trim().slice(0, 40) });
}
}
const found = [];
for (let i = 0; i < runs.length; i++)
Expand Down
15 changes: 15 additions & 0 deletions tests/interact_support.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
from concurrent.futures import ThreadPoolExecutor
from pathlib import Path

import anyio
import pytest
from click.testing import CliRunner
from conftest import LEAF_COMMAND
Expand Down Expand Up @@ -73,6 +74,20 @@
"""


def run_async(entry):
"""Run an async entry point on a thread of this test's own.

Playwright's sync API drives an asyncio loop and holds it running in the thread that
opened a browser for the whole life of `sync_playwright()`, and the `browser` fixture
is session-scoped per xdist worker. So `anyio.run` in a worker that has already run a
browser test raises "Already running asyncio in this thread", and the same call in a
worker that has not passes — leaving the scheduler to decide whether an MCP test can
start a loop at all. A thread with no loop on it answers for every schedule.
"""
with ThreadPoolExecutor(max_workers=1) as loop_thread:
return loop_thread.submit(lambda: anyio.run(entry)).result()


def spawn_probe(spawn, page_dir, body, **environment):
"""Run a deterministic race seam in an isolated Leaf application process."""
env = {name: str(value) for name, value in environment.items()}
Expand Down
12 changes: 11 additions & 1 deletion tests/render_cases_interaction.py
Original file line number Diff line number Diff line change
Expand Up @@ -882,7 +882,7 @@ def backdate_note(page_dir, version, hours):
# log holding one event apiece leaves the whole of it standing at once. Selection and
# completion share one option-group unit but occupy distinct facets, so both stand;
# accept and reject share the settlement facet, so two suggestions let both competing
# verbs stand. The floor below derives the list from the registry, so a twelfth widget's
# verbs stand. The floor below derives the list from the registry, so a newly declared
# verb fails here rather than passing unexercised.
STANDING_PAGE = leaf_page(
"standing state",
Expand Down Expand Up @@ -917,6 +917,15 @@ def backdate_note(page_dir, version, hours):
<lf-old><p id="ab-roll">Access logs roll off after 30 days.</p></lf-old>
<lf-new><p>Access logs are kept for 90 days.</p></lf-new>
</lf-suggestion>
<lf-diff id="ab-patch"><pre>
diff --git a/ab/bracket.py b/ab/bracket.py
--- a/ab/bracket.py
+++ b/ab/bracket.py
@@ -1,2 +1,2 @@
def bracket():
- return "plastic"
+ return "steel"
</pre></lf-diff>
""",
)

Expand All @@ -938,6 +947,7 @@ def backdate_note(page_dir, version, hours):
("ab-email", "edit", {"text": "The words as the reader rewrote them."}),
("ab-sug-410", "accept", {}),
("ab-sug-logs", "reject", {}),
("ab-patch", "review", {"file": "ab/bracket.py", "reviewed": True}),
]
RELATIVE_WIDGET_PAGE = leaf_page(
"relative widget",
Expand Down
Loading
Loading