Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
39 changes: 37 additions & 2 deletions apps/hue/src/app.d
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import std.stdio : stderr, write;
import std.string : chompPrefix;

import sparkles.syntax;
import sparkles.syntax.md.model : MdDoc;
import sparkles.twoslash;
import sparkles.core_cli.args;

Expand All @@ -34,8 +35,8 @@ import sparkles.base.term_caps : isTerminal, StdStream;
import ansi_model : BackgroundMode, backgroundOptions;
import document : ContentKind, Document, DocumentPipeline, hueFenceRenderer;
import diff_commutative : CommutativeKind;
import diff_session : SessionHeader;
import forge : PullRequest;
import diff_session : AnchoredThread, SessionHeader, ThreadComment;
import forge : CommentThread, PullRequest, ThreadSide;
import diff_structural : StructuralPolicy;
import diff_view : DiffLayout, DiffViewOptions;
import sparkles.diff : WhitespaceMode;
Expand Down Expand Up @@ -391,6 +392,37 @@ private SessionHeader prHeader(ref GrammarRegistry reg, in PullRequest pr) @syst
return h;
}

/// `DPR3`: the forge's conversations as the view's own anchored threads —
/// each comment's markdown parsed here, so the renderer draws a review
/// comment the way it draws every other piece of markdown hue shows.
private AnchoredThread[] prThreads(ref GrammarRegistry reg,
in CommentThread[] threads) @system
{
import sparkles.syntax.md.model : extractMarkdown;

AnchoredThread[] out_;
foreach (ref t; threads)
{
AnchoredThread a = {
path: t.path,
line: t.line,
oldSide: t.side == ThreadSide.oldSide,
resolved: t.resolved,
outdated: t.outdated,
};
foreach (ref c; t.comments)
a.comments ~= ThreadComment(c.author, shortDate(c.createdAt),
c.body_.length ? extractMarkdown(reg, c.body_) : MdDoc.init);
out_ ~= a;
}
return out_;
}

/// An ISO-8601 timestamp trimmed to its date. A review thread wants "when,
/// roughly"; the clock time is noise beside the comment it labels.
private string shortDate(string iso) @safe pure nothrow
=> iso.length >= 10 ? iso[0 .. 10] : iso;

private StructuralPolicy parseStructural(string spelling) @safe
{
import diff_structural : parseStructuralPolicy;
Expand Down Expand Up @@ -661,6 +693,9 @@ int main(string[] args)
text(got.repo.owner, "/", got.repo.name, " #",
got.pr.number), got.patch);
doc.diffSession.header = prHeader(registry, got.pr);
doc.diffSession.threads = prThreads(registry, got.threads);
if (got.threadsNote.length)
warning(i"review threads unavailable: $(got.threadsNote)");
}
else if (cli.diff || cli.staged)
{
Expand Down
36 changes: 36 additions & 0 deletions apps/hue/src/diff_session.d
Original file line number Diff line number Diff line change
Expand Up @@ -77,12 +77,48 @@ struct SessionHeader
MdDoc description;
}

/// One message of a review conversation, ready to render.
///
/// The body is an `MdDoc` for the same reason a PR description is: a review
/// comment is markdown, and rendering it as raw text would be the one place
/// in hue where markdown is not markdown.
struct ThreadComment
{
string author;
string when;
MdDoc body_;
}

/**
`DPR3`: a review conversation anchored to a file and a line.

Forge-neutral on purpose. This is the shape the VIEW renders, and `DCM2`'s
local comments produce exactly the same value — which is what "the thread
block is one widget" means concretely: no forge type reaches the renderer, and
a locally-authored thread is not a second kind of thing.
*/
struct AnchoredThread
{
string path;
/// 1-based line on its side; zero when the forge could not place it.
uint line;
/// The thread hangs on the old text (a comment on a removed line).
bool oldSide;
bool resolved;
/// The code it was written against has changed since.
bool outdated;
ThreadComment[] comments;
}

struct DiffSession
{
SessionEntry[] entries;
size_t index; /// the selected entry (always < `entries.length` when non-empty)
/// `DPR2`: the header this session renders above its files, if any.
SessionHeader header;
/// `DPR3`/`DCM2`: every conversation on this session, any file. The view
/// selects each file's own by path.
AnchoredThread[] threads;

@safe pure nothrow @nogc:

Expand Down
203 changes: 201 additions & 2 deletions apps/hue/src/diff_view.d
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ module diff_view;

import std.conv : text;

import diff_session : DiffSession, FileChange, SessionEntry, SessionHeader,
statusGlyph;
import diff_session : AnchoredThread, DiffSession, FileChange, SessionEntry,
SessionHeader, statusGlyph;
import document : DiffSides;
import sparkles.diff.model : Degradation, DiffDoc, FileEntry, Hunk, Row,
RowKind, Span;
Expand Down Expand Up @@ -75,6 +75,10 @@ struct DiffViewOptions
/// badge. Demote, never hide — the badge says how many rows it stands for
/// and the reviewer can always expand.
bool foldFormattingOnly = true;
/// `DPR3`/`DCM2`: this file's review conversations. Each renders as a
/// block under the row it is anchored to; a resolved one folds to a
/// single line, because a settled argument is context, not a task.
const(AnchoredThread)[] threads;
/// `DVT1`: the per-side type payloads for this file. Each attaches only
/// when its `code` is byte-identical to that side's diff text — see
/// $(LREF anchors) — so a decoration can never land on the wrong token.
Expand Down Expand Up @@ -253,6 +257,12 @@ WidgetTree viewDiffDoc(const ref DiffDoc doc, DiffViewOptions opt = DiffViewOpti
// expanded unchanged region is read from.
if (fi < sides.length)
fopt.sideText = sides[fi].newText;
// `DPR3`: the conversations on THIS file, by path — the session holds
// them all, and a collapsed file renders none.
if (!fopt.entry.collapsed && session.threads.length)
fopt.threads = threadsFor(session.threads,
doc.pathText(doc.files[fi].newPath),
doc.pathText(doc.files[fi].oldPath));
// `DVT1`: the file's per-side type overlays, already anchored (or
// refused) by whoever attached them.
if (!fopt.entry.collapsed && fi < types.length)
Expand Down Expand Up @@ -466,6 +476,90 @@ private Slot statusSlot(FileChange c) @safe pure nothrow @nogc
}
}

/// The threads belonging to one file. Matched on either path so a rename
/// does not orphan a conversation written before it.
private const(AnchoredThread)[] threadsFor(
return scope const(AnchoredThread)[] all, scope const(char)[] newPath,
scope const(char)[] oldPath) @safe
{
const(AnchoredThread)[] mine;
foreach (ref t; all)
if (t.path == newPath || t.path == oldPath)
mine ~= t;
return mine;
}

/// Does this thread hang on this row? A thread on the old side anchors to a
/// removed or context row's old line; one on the new side to an added or
/// context row's new line.
private bool anchoredHere(in AnchoredThread t, in Row row) @safe pure nothrow @nogc
{
if (t.line == 0)
return false; // outdated: no line to hang on
return t.oldSide ? row.oldLine == t.line : row.newLine == t.line;
}

/**
`DPR3`/`DCM2`: one review conversation, rendered under its anchor line.

A resolved thread folds to a single line. That is not tidiness — an
unresolved conversation is a thing the reviewer must act on, and rendering a
settled argument at the same weight buries the live one. The badge still says
who and how many, so nothing is hidden, only demoted; the same
demote-never-hide contract `DVN2` holds for noise.

Indented to the code's column so the conversation reads as belonging to that
line rather than to the file.
*/
private uint threadBlock(ref Builder b, in AnchoredThread t, int gutterWidth,
in DiffViewOptions opt) @safe
{
import sparkles.syntax.md.render_widgets : MdViewOptions, viewMarkdownInto;
import sparkles.ui.geometry : Insets;
import sparkles.ui.style : TextStyle;

const pad = gutterWidth > 0 ? gutterWidth + 2 : 2;
auto indent = new char[](pad);
indent[] = ' ';

if (t.resolved)
{
const who = t.comments.length ? t.comments[0].author : "someone";
return b.add(Widget(kind: WidgetKind.rich, spans: [
TextSpan(indent.idup, slot: Slot.gutter),
TextSpan(text("✓ resolved — ", t.comments.length,
t.comments.length == 1 ? " comment by " : " comments, from ",
who), slot: Slot.muted),
]));
}

auto rows = new uint[](0);
foreach (i, ref c; t.comments)
{
TextSpan[] head = [TextSpan(indent.idup, slot: Slot.gutter)];
head ~= TextSpan(i == 0 ? "▌ " : "│ ", slot: Slot.chromeAccent);
head ~= TextSpan(c.author.idup, slot: Slot.chromeAccent,
textStyle: TextStyle(bold: true));
if (c.when.length)
head ~= TextSpan(" " ~ c.when.idup, slot: Slot.muted);
if (t.outdated && i == 0)
head ~= TextSpan(" (outdated)", slot: Slot.muted);
rows ~= b.add(Widget(kind: WidgetKind.rich, spans: head));

if (c.body_.root.children.length)
{
// Indented by PADDING, not by a prefix span: a comment body wraps,
// and a prefix only ever lands on the first row — leaving the rest
// hanging out at the code's own column, which reads as code.
MdViewOptions mopt;
rows ~= b.add(Widget(kind: WidgetKind.column,
children: [viewMarkdownInto(b, c.body_, mopt)],
padding: Insets(0, 0, 0, pad + 2)));
}
}
return b.container(WidgetKind.column, rows);
}

/**
`DPR2`: the session header — what this change is, then its description.

Expand All @@ -480,6 +574,7 @@ private uint sessionHeader(ref Builder b, const SessionHeader h,
SideRenderer render, in DiffViewOptions opt) @safe
{
import sparkles.syntax.md.render_widgets : MdViewOptions, viewMarkdownInto;
import sparkles.ui.geometry : Insets;
import sparkles.ui.style : TextStyle;

auto rows = new uint[](0);
Expand Down Expand Up @@ -759,7 +854,15 @@ private uint viewHunk(ref Builder b, const ref DiffDoc doc, in Hunk hunk,
rows ~= hunkHeader(b, hunk);

foreach (ref row; doc.hunkRows(hunk))
{
rows ~= viewRow(b, doc, row, gutterWidth, opt);
// `DPR3`: the conversation goes UNDER the line it is about, the way
// a reviewer reads it — not in a margin, and not in a separate pane
// where the code it refers to is no longer on screen.
foreach (ref t; opt.threads)
if (anchoredHere(t, row))
rows ~= threadBlock(b, t, gutterWidth, opt);
}

return b.container(WidgetKind.column, rows);
}
Expand Down Expand Up @@ -1477,3 +1580,99 @@ import sparkles.ui.style : Slot;
}
assert(!plainText.canFind("feat: a thing"));
}

@("diff_view.threads.anchorUnderTheirLineAndFoldWhenResolved")
@safe unittest
{
import diff_session : AnchoredThread, buildDiffSession, ThreadComment;
import sparkles.syntax.md.model : MdBlock, MdBlockKind, MdDoc, MdInline,
MdInlineKind, Span;

static MdDoc prose(string text) @safe
{
MdDoc d = {
source: text,
root: MdBlock(kind: MdBlockKind.document, children: [
MdBlock(kind: MdBlockKind.paragraph, span: Span(0, text.length),
inlines: [MdInline(kind: MdInlineKind.text,
span: Span(0, text.length))]),
]),
};
return d;
}

auto doc = diffText("one\ntwo\nthree\n", "one\nTWO\nthree\n", "f.d", "f.d");
auto session = buildDiffSession(doc);
session.threads = [
AnchoredThread(path: "f.d", line: 2, resolved: false,
comments: [ThreadComment("reviewer", "2026-08-07",
prose("this needs a why"))]),
AnchoredThread(path: "f.d", line: 3, resolved: true,
comments: [ThreadComment("reviewer", "2026-08-06",
prose("settled long ago"))]),
];

auto tree = viewDiffDoc(doc, DiffViewOptions.init, null, null, session);

const(char)[] all;
size_t threadRow = size_t.max, anchorRow = size_t.max;
foreach (i, ref n; tree.nodes)
{
const(char)[] row = n.text;
foreach (sp; n.spans)
row ~= sp.text;
all ~= row;

import std.algorithm.searching : canFind;

if (row.canFind("this needs a why"))
threadRow = i;
if (row.canFind("TWO"))
anchorRow = i;
}

import std.algorithm.searching : canFind;

assert(threadRow != size_t.max, "an unresolved thread renders its comments");
assert(anchorRow != size_t.max && anchorRow < threadRow,
"the conversation goes UNDER the line it is about");
assert(all.canFind("reviewer") && all.canFind("2026-08-07"));

// A resolved thread demotes to one line: still there, no longer shouting.
assert(!all.canFind("settled long ago"), "a resolved body folds away");
assert(all.canFind("resolved"), "but the badge says it exists");

// Threads belong to their own file: another path's conversations are not
// borrowed by this one.
session.threads[0].path = "other.d";
auto elsewhere = viewDiffDoc(doc, DiffViewOptions.init, null, null, session);
const(char)[] other;
foreach (ref n; elsewhere.nodes)
foreach (sp; n.spans)
other ~= sp.text;
assert(!other.canFind("this needs a why"));
}

@("diff_view.threads.anOutdatedThreadHasNoLineToHangOn")
@safe unittest
{
import diff_session : AnchoredThread, buildDiffSession, ThreadComment;

// GitHub reports an outdated thread with a null line, which decodes to
// zero. Zero must not match row 0 of anything — it means "nowhere".
auto doc = diffText("a\n", "b\n", "f.d", "f.d");
auto session = buildDiffSession(doc);
session.threads = [AnchoredThread(path: "f.d", line: 0, outdated: true,
comments: [ThreadComment("reviewer", "2026-08-01")])];

auto tree = viewDiffDoc(doc, DiffViewOptions.init, null, null, session);
const(char)[] all;
foreach (ref n; tree.nodes)
foreach (sp; n.spans)
all ~= sp.text;

import std.algorithm.searching : canFind;

assert(!all.canFind("reviewer"),
"a thread with no line anchors nowhere rather than at the top");
}
Loading
Loading