git-ents.gitmain
⌘K
foforge
commit ac69680
roots: interleave comments inline at their anchored lines

A blob view rendered every comment below the whole file, no matter which line it anchored to. The raw-source view is now one table row per line (a .blob-nums/.blob-code cell pair, no wrapper beyond those two cells) instead of two big <pre> panes, so a comment whose current range lands on lines a-b can render its card as a full-width row directly after line b’s own row — multiple comments ending on the same line stack in ref order. A comment with no current line range (a whole-file anchor, or an outdated projection) has nowhere to interleave and keeps the old below-the-blob placement, now titled "outdated comments" to distinguish it; a rendered document or binary view is untouched, since neither has a source line to interleave at.

Splitting arborium’s highlighted HTML (one string for the whole file) into per-line fragments is the subtle part: a highlight span can cross a newline (a multiline block comment, a triple-quoted string), so naively splitting on \n would leave an unclosed <span> dangling into the next fragment. A small tokenizer walks the span-open/span-close/text stream (arborium’s HtmlFormat::ClassNames renderer never emits any other shape) and closes every open span at each embedded newline, reopening them at the start of the next line — entities (<) are only ever split on literal \n bytes within a text run, never re-parsed or re-escaped.

roots: rework the raw-source blob view into one table row per line, replacing the two-pre-pane layout roots: add a span-aware line splitter for arborium’s highlighted HTML output roots: interleave a comment’s card after its anchored range’s last line, full width across the blob roots: title the below-the-blob section "outdated comments" and refactor the comment-card markup into a shared helper roots: give the blob table a sticky line-number gutter so the whole row scrolls together Assisted-by: Claude:claude-sonnet-5

Joseph D. Carpinelli · 1 month ago

Reviews

No reviews of this commit yet — record a verdict below.

Start a review

verdict

crates/cli/ents-web/tests/router.rs @@ -896,7 +896,7 @@ .to_bytes(); let body = String::from_utf8(body.to_vec()).expect("utf8 html"); assert!(body.contains("blob-nums")); - assert!(body.contains("<pre class=\"blob-code\"><code>")); + assert!(body.contains("<td class=\"blob-code\"><code>")); assert!(body.contains("1 &lt; 2")); } @@ -1113,11 +1113,20 @@ .expect("body") .to_bytes(); let body = String::from_utf8(body.to_vec()).expect("utf8 html"); - assert!(body.contains("id=\"file-comments\"")); + assert!(body.contains("id=\"comment-0\"")); assert!(body.contains("worth a look here")); assert!(body.contains("commenter")); assert!(body.contains("href=\"#L2\"")); assert!(!body.contains("class=\"outdated\"")); + // Interleaved directly into the blob, after line 2's row and before + // line 3's -- not below the whole table. + let line2 = body.find("id=\"L2\"").expect("line 2 renders"); + let card = body.find("comment-meta").expect("card renders"); + let line3 = body.find("id=\"L3\"").expect("line 3 renders"); + assert!( + line2 < card && card < line3, + "the card must land between line 2 and line 3, in document order" + ); } /// A comment whose anchored lines were since edited still renders (never
crates/cli/ents-web/src/assets/ents.css @@ -274,20 +274,32 @@ .commit-meta { color: var(--color-text-muted); font-size: .85rem; margin-top: .2rem; } .commit-meta a { text-decoration: underline; text-decoration-color: color-mix(in srgb, currentColor 25%, transparent); } -.blob { display: grid; grid-template-columns: auto minmax(0, 1fr); background: var(--color-surface); border: 1px solid var(--color-border); border-radius: var(--radius-sm); box-shadow: var(--shadow-sm); overflow: hidden; margin-bottom: 1.5rem; } -.blob pre { font-family: var(--font-mono); font-size: .82rem; line-height: 1.55; margin: 0; padding: 1rem 0; } -.blob pre.blob-nums { text-align: right; color: var(--color-text-muted); background: var(--color-code-bg); border-right: 1px solid var(--color-border); padding-left: 1ch; padding-right: 1ch; user-select: none; -webkit-user-select: none; } +/* The line-per-row source view (`crate::pages::files::source_view`): a + * `<table>` inside one `overflow-x: auto` wrapper so the whole blob scrolls + * together, its `.blob-nums` gutter cell pinned via `position: sticky` so + * line numbers stay put while wide code scrolls under them -- the same + * frozen-gutter behavior the previous two-`<pre>`-column layout got for + * free, now reconstructed for a per-line table so a comment card + * (`tr.blob-comment-row`) can interleave as a full-width row between any + * two line rows. */ +.blob { overflow-x: auto; background: var(--color-surface); border: 1px solid var(--color-border); border-radius: var(--radius-sm); box-shadow: var(--shadow-sm); margin-bottom: 1.5rem; } +.blob table { border-collapse: collapse; width: 100%; font-family: var(--font-mono); font-size: .82rem; line-height: 1.55; } +.blob td.blob-nums { position: sticky; left: 0; text-align: right; color: var(--color-text-muted); background: var(--color-code-bg); border-right: 1px solid var(--color-border); padding: 0 1ch; user-select: none; -webkit-user-select: none; white-space: nowrap; vertical-align: top; } .blob-nums a { display: block; color: inherit; text-decoration: none; } .blob-nums a:hover { color: var(--color-accent); } .blob-nums a:target { color: var(--color-accent); font-weight: 700; } -.blob-code { overflow-x: auto; min-width: 0; } -.blob-code code { display: block; font-family: inherit; padding: 0 1.25rem; white-space: pre; color: var(--color-text); } +.blob td.blob-code { padding: 0 1.25rem; white-space: pre; color: var(--color-text); vertical-align: top; } +.blob-code code { font-family: inherit; } +.blob tr.blob-comment-row td { padding: 0; background: var(--color-surface); } +.blob tr.blob-comment-row .card { margin: .5rem 1rem; } .binary { padding: 2.5rem; text-align: center; font-family: var(--font-mono); font-size: .85rem; color: var(--color-text-muted); } -/* File-anchored comment cards (`crate::pages::comments::comments_section`), - * below a blob view in `crate::pages::files` -- each comment is its own - * `.card`, its body reusing `.doc-body`'s prose styling (a comment body - * renders as AsciiDoc, same as a rendered document blob). */ +/* File-anchored comment cards (`crate::pages::comments::comment_card`), + * whether interleaved into a blob's own table (`tr.blob-comment-row`, + * above) or listed below it (`comments_section`/`outdated_comments_section`) + * -- each comment is its own `.card`, its body reusing `.doc-body`'s prose + * styling (a comment body renders as AsciiDoc, same as a rendered document + * blob). */ .comment-meta { display: flex; flex-wrap: wrap; align-items: center; gap: .5rem; padding: .7rem 1.1rem; font-size: .82rem; color: var(--color-text-muted); border-bottom: 1px solid var(--color-border); } .comment-meta .author { color: var(--color-text); font-weight: 600; } .outdated { color: var(--color-text-muted); font-style: italic; }
crates/cli/ents-web/src/pages/comments.rs @@ -6,10 +6,12 @@ //! already returns structured data for, rather than a bare reflected //! field list. //! -//! [`for_path`]/[`comments_section`] are this module's second entry -//! point: `crate::pages::files`'s blob view calls them to render the -//! comments anchored to the file it is showing, rather than duplicating -//! this module's own read-project-render pattern. +//! [`for_path`]/[`comment_card`]/[`comments_section`] are this module's +//! second entry point: `crate::pages::files`'s blob view calls them to +//! render the comments anchored to the file it is showing -- inline, +//! interleaved at the anchored line, or in a below-the-blob section for +//! one with no current line to interleave at -- rather than duplicating +//! this module's own read-project-render pattern or its card markup. use std::sync::Arc; @@ -202,17 +204,15 @@ } /// One comment as `crate::pages::files`'s blob view shows it: who wrote it -/// and when ([`super::ago`]), where its anchor lands on the displayed -/// file (a line-range link into the blob's own `#L<n>` gutter, or the -/// muted `outdated` marker when [`ents_anchor::project`] can no longer map -/// the anchored lines), and its body rendered as AsciiDoc -/// ([`crate::asciidoc`], this crate's default prose treatment for text -/// with no filename of its own to infer a MIME type from). Mirrors -/// `pre-redo:crates/git-ents-server/src/web/pages.rs`'s own `FileComment`, -/// salvaged per this crate's PORT-and-reverify policy: author/timestamp -/// there came from `git_comment::provenance`'s shell-out, here from -/// [`super::commit_authorship`] reading the comment ref's own tip commit -/// through `gix_object::Find`. +/// and when ([`super::ago`]), where its anchor lands (a line range, when it +/// has one to interleave at -- [`comment_card`]'s own doc), and its body +/// rendered as AsciiDoc ([`crate::asciidoc`], this crate's default prose +/// treatment for text with no filename of its own to infer a MIME type +/// from). Mirrors `pre-redo:crates/git-ents-server/src/web/pages.rs`'s own +/// `FileComment`, salvaged per this crate's PORT-and-reverify policy: +/// author/timestamp there came from `git_comment::provenance`'s shell-out, +/// here from [`super::commit_authorship`] reading the comment ref's own tip +/// commit through `gix_object::Find`. pub(crate) struct FileComment { /// The comment ref's own tip commit's author display name /// (`model.comment`: a comment stores no author field of its own). @@ -220,7 +220,9 @@ /// [`super::ago`] renders this against the current time. pub(crate) seconds: i64, /// The anchored range as it lands on the displayed file at `HEAD`, or - /// `None` for a whole-file anchor or an outdated projection. + /// `None` for a whole-file anchor or an outdated projection -- either + /// way, nothing for [`crate::pages::files`]'s blob view to interleave + /// the card after, so it renders in a below-the-blob section instead. pub(crate) lines: Option<LineRange>, /// Set when [`ents_anchor::project`] reports /// [`Projection::Outdated`]: the anchored lines themselves were @@ -295,37 +297,48 @@ out } -/// The comment cards under a blob view, one [`FileComment`] per -/// [`maud`]-rendered `.card`, mounted at `#file-comments` so -/// `crate::pages::files`'s own "N comments" link can jump straight to -/// them. Renders nothing at all -- not even an empty container -- when -/// `comments` is empty, so a file with no comments carries no extra -/// markup (`crate::pages::files`'s own blob view calls this -/// unconditionally rather than checking first). -pub(crate) fn comments_section(comments: &[FileComment]) -> Markup { - if comments.is_empty() { - return html! {}; - } +/// One comment's card: author, [`super::ago`] time, an in-page `#L<n>` +/// line-range link (or the muted `outdated` marker), and its body -- the +/// single rendering every comment-showing spot in `crate::pages::files` +/// shares ([`comments_section`]'s below-the-blob list, the blob view's own +/// inline-interleaved rows), so a comment's markup is defined in exactly +/// one place. `index` names this card's `id="comment-<index>"` anchor, +/// stable within whichever page rendered it (not a global id): +/// `crate::pages::files`'s crumbs "N comments" jump link targets +/// `comment-0`, the first comment in display order, regardless of whether +/// it landed inline or below the blob. +pub(crate) fn comment_card(index: usize, comment: &FileComment) -> Markup { html! { - div id="file-comments" { - @for comment in comments { - div.card { - div.comment-meta { - span.author { (comment.author) } - span { (super::ago(comment.seconds)) } - @if let Some(range) = comment.lines { - a href={ "#L" (range.start) } { - @if range.start == range.end { "line " (range.start) } - @else { "lines " (range.start) "-" (range.end) } - } - } - @if comment.outdated { - span.outdated { "outdated" } - } + div.card id={ "comment-" (index) } { + div.comment-meta { + span.author { (comment.author) } + span { (super::ago(comment.seconds)) } + @if let Some(range) = comment.lines { + a href={ "#L" (range.start) } { + @if range.start == range.end { "line " (range.start) } + @else { "lines " (range.start) "-" (range.end) } } - div.doc-body { (comment.body) } + } + @if comment.outdated { + span.outdated { "outdated" } } } + div.doc-body { (comment.body) } + } + } +} + +/// The comment cards under a blob view (a rendered document, a binary +/// placeholder, or -- for a raw-source view -- the ones with no current +/// line range to interleave at; see `crate::pages::files::source_view`), +/// one [`comment_card`] per entry. Renders nothing at all -- not even an +/// empty container -- when `comments` is empty, so a file with no comments +/// carries no extra markup (`crate::pages::files`'s own blob view calls +/// this unconditionally rather than checking first). +pub(crate) fn comments_section(comments: &[FileComment]) -> Markup { + html! { + @for (index, comment) in comments.iter().enumerate() { + (comment_card(index, comment)) } } }
crates/cli/ents-web/src/pages/files.rs @@ -18,11 +18,20 @@ //! that). //! //! A blob view also loads and renders the comments anchored to it -//! (`crate::pages::comments::for_path`/`comments_section`), below the blob -//! itself, and [`crumbs`] grows a "comment on this file" link (plus a -//! jump to those cards, once there is at least one) beside its own -//! trailing "history" link -- a directory listing carries neither, since -//! a comment anchors to a file, never a tree. +//! (`crate::pages::comments::for_path`), and [`crumbs`] grows a "comment on +//! this file" link (plus a jump to the first card, once there is at least +//! one) beside its own trailing "history" link -- a directory listing +//! carries neither, since a comment anchors to a file, never a tree. A +//! raw-source view (not a rendered document or a binary placeholder) +//! interleaves each comment's card directly after the row naming its +//! anchored range's last line, full width across the blob's line-number +//! and code columns ([`source_view`]); a comment with no current line +//! range (a whole-file anchor, or `ents_anchor::Projection::Outdated`) has +//! nowhere to interleave, and renders in a below-the-blob "outdated +//! comments" section instead ([`outdated_comments_section`]). Doc-rendered +//! and binary views keep every comment below the blob, unconditionally +//! (`crate::pages::comments::comments_section`), since there is no source +//! line to interleave at. use std::sync::Arc; @@ -151,6 +160,7 @@ .map_err(|source| Error::Repo(source.to_string()))?; let name = path.rsplit('/').next().unwrap_or(path); let comments = super::comments::for_path(state, &repo, path); + let (body, below) = blob_view(name, &blob.data, &comments)?; Ok(super::layout( &super::RepoHeader::from_state(state), &super::identity_label(state), @@ -158,8 +168,8 @@ path, html! { (crumbs(path, Some(comments.len()))) - (blob_view(name, &blob.data)?) - (super::comments::comments_section(&comments)) + (body) + (below) }, )) } else { @@ -234,10 +244,11 @@ /// of the code, not a tab of its own -- `crate::pages::mod`'s own doc) and, /// on a blob view (`comments` is `Some`), `crate::pages::comments`'s own /// add form for this file ("comment on this file") plus a jump straight to -/// [`super::comments::comments_section`]'s cards when there is at least -/// one comment already anchored here. -/// `comments` is `None` on a directory listing, where neither link makes -/// sense. +/// the first comment card (`id="comment-0"`, in display order -- see +/// [`super::comments::comment_card`]'s own doc) when there is at least one +/// comment already anchored here, wherever it renders (inline or below the +/// blob). `comments` is `None` on a directory listing, where neither link +/// makes sense. fn crumbs(path: &str, comments: Option<usize>) -> Markup { let parts: Vec<&str> = path.split('/').filter(|s| !s.is_empty()).collect(); let mut acc = String::new(); @@ -265,7 +276,7 @@ @if let Some(count) = comments { a.crumbs-history href={ "/comments?file=" (path) } { "comment on this file" } @if count > 0 { - a.crumbs-history href="#file-comments" { + a.crumbs-history href="#comment-0" { (count) @if count == 1 { " comment" } @else { " comments" } } } @@ -281,49 +292,294 @@ bytes.iter().take(8000).any(|b| *b == 0) } -/// A single blob's contents: a Markdown/AsciiDoc document rendered as such -/// via [`crate::markdown`]/[`crate::asciidoc`], a binary-content -/// placeholder, or a line-numbered source view of the raw text. -/// -/// The source view mirrors `pre-redo:crates/git-ents-server/src/web/pages.rs`'s -/// `blob_body`: a `.blob` grid pairing a `pre.blob-nums` gutter of -/// per-line `#L{n}` anchors with a `pre.blob-code` code column, highlighted -/// via [`highlight`] when `name`'s grammar is known and falling back to a -/// plain escaped `<code>` otherwise. +/// A single blob's contents, plus whatever comments belong below it: a +/// Markdown/AsciiDoc document rendered as such via +/// [`crate::markdown`]/[`crate::asciidoc`] or a binary-content placeholder +/// -- either way every `comment` renders below, unconditionally +/// ([`crate::pages::comments::comments_section`]), since there is no +/// source line to interleave a card at -- or [`source_view`]'s +/// line-per-row rendering, which interleaves a comment with a current line +/// range directly into the blob and returns the rest (no current line +/// range: a whole-file anchor, or `ents_anchor::Projection::Outdated`) as +/// a separate below-the-blob section ([`outdated_comments_section`]). /// /// # Errors /// /// Propagates [`crate::asciidoc::to_html`]'s own [`Error::Asciidoc`]. -fn blob_view(name: &str, bytes: &[u8]) -> Result<Markup> { +fn blob_view( + name: &str, + bytes: &[u8], + comments: &[super::comments::FileComment], +) -> Result<(Markup, Markup)> { if is_binary(bytes) { - return Ok(html! { div.binary { "Binary file (" (bytes.len()) " bytes) not shown." } }); + return Ok(( + html! { div.binary { "Binary file (" (bytes.len()) " bytes) not shown." } }, + super::comments::comments_section(comments), + )); } let Ok(text) = std::str::from_utf8(bytes) else { - return Ok(html! { div.binary { "Binary file (" (bytes.len()) " bytes) not shown." } }); + return Ok(( + html! { div.binary { "Binary file (" (bytes.len()) " bytes) not shown." } }, + super::comments::comments_section(comments), + )); }; if crate::markdown::is_markdown(name) { - return Ok(html! { div.card { div.doc-body { (crate::markdown::to_html(text)) } } }); + return Ok(( + html! { div.card { div.doc-body { (crate::markdown::to_html(text)) } } }, + super::comments::comments_section(comments), + )); } if crate::asciidoc::is_asciidoc(name) { - return Ok(html! { div.card { div.doc-body { (crate::asciidoc::to_html(text)?) } } }); + return Ok(( + html! { div.card { div.doc-body { (crate::asciidoc::to_html(text)?) } } }, + super::comments::comments_section(comments), + )); } - let lines = text.lines().count().max(1); let highlighted = highlight(name, text); - Ok(html! { + let below: Vec<(usize, &super::comments::FileComment)> = comments + .iter() + .enumerate() + .filter(|(_, comment)| comment.lines.is_none()) + .collect(); + Ok(( + source_view(text, highlighted, comments), + outdated_comments_section(&below), + )) +} + +/// The raw-source view: one table row per line (a `<tr>` pairing a +/// `.blob-nums` line-number cell carrying the row's `#L{n}` anchor with a +/// `.blob-code` cell, no wrapper beyond those two cells -- lean enough that +/// thousands of lines stay cheap), highlighted via [`highlight`] when +/// `highlighted` is `Some` and falling back to plain (still per-line, +/// still auto-escaped by `maud`'s own interpolation) text otherwise. Each +/// [`FileComment`](super::comments::FileComment) in `comments` whose +/// [`ents_anchor::LineRange`] is `Some` renders its card +/// ([`super::comments::comment_card`]) immediately after the row naming +/// its range's last line, full width across both columns +/// (`tr.blob-comment-row`, `colspan="2"`) -- multiple comments ending on +/// the same line stack in `comments`' own order (`comment::list`'s ref +/// order). A comment with no current line range is [`blob_view`]'s own +/// concern, not this function's: it never appears here. +fn source_view( + text: &str, + highlighted: Option<String>, + comments: &[super::comments::FileComment], +) -> Markup { + let physical_lines: Vec<&str> = text.lines().collect(); + let line_count = physical_lines.len().max(1); + + let mut code_lines: Vec<Markup> = match &highlighted { + Some(html) => split_highlighted_lines(html, line_count) + .into_iter() + .map(|fragment| html! { (PreEscaped(fragment)) }) + .collect(), + None => physical_lines + .iter() + .map(|line| html! { (*line) }) + .collect(), + }; + // Exactly `line_count` rows either way: arborium trims trailing + // newlines before highlighting (see `split_highlighted_lines`'s own + // doc), so a file ending in blank lines can highlight to fewer + // embedded newlines than `text.lines().count()` -- padding (never + // truncating in practice, since `split_highlighted_lines` never + // returns fewer than one fragment) keeps every gutter number paired + // with a code cell, with no per-row fallback indexing needed below. + code_lines.resize_with(line_count, Markup::default); + + let mut by_end_line: std::collections::BTreeMap<u64, Vec<usize>> = + std::collections::BTreeMap::new(); + for (index, comment) in comments.iter().enumerate() { + if let Some(range) = comment.lines { + by_end_line.entry(range.end).or_default().push(index); + } + } + + html! { div.blob { - pre.blob-nums { - @for n in 1..=lines { - a id={ "L" (n) } href={ "#L" (n) } { (n) } - } - } - pre.blob-code { - @match highlighted { - Some(html) => code.code { (PreEscaped(html)) }, - None => code { (text) }, + table { + tbody { + @for (index, code) in code_lines.into_iter().enumerate() { + @let n = index.saturating_add(1); + tr { + td.blob-nums { a id={ "L" (n) } href={ "#L" (n) } { (n) } } + @if highlighted.is_some() { + td.blob-code { code.code { (code) } } + } @else { + td.blob-code { code { (code) } } + } + } + @if let Some(indices) = by_end_line.get(&u64::try_from(n).unwrap_or(u64::MAX)) { + @for &comment_index in indices { + @if let Some(comment) = comments.get(comment_index) { + tr.blob-comment-row { + td colspan="2" { + (super::comments::comment_card(comment_index, comment)) + } + } + } + } + } + } } } } - }) + } +} + +/// The below-the-blob section for comments with no current line range to +/// interleave at (a whole-file anchor, or +/// `ents_anchor::Projection::Outdated`) -- titled to distinguish it from +/// the inline cards [`source_view`] interleaves directly into the blob, +/// since every comment reaching here either predates line-level anchoring +/// or has literally gone stale. Renders nothing at all when `comments` is +/// empty (mirrors [`super::comments::comments_section`]'s identical +/// stance). +fn outdated_comments_section(comments: &[(usize, &super::comments::FileComment)]) -> Markup { + if comments.is_empty() { + return html! {}; + } + html! { + h2 { "outdated comments" } + @for &(index, comment) in comments { + (super::comments::comment_card(index, comment)) + } + } +} + +/// Split [`highlight`]'s single HTML string into one HTML fragment per +/// source line (`line_count` of them, padding with an empty string past +/// whatever [`tokenize`] actually produced -- arborium trims trailing +/// newlines from its input before highlighting, so a file ending in +/// several blank lines can highlight to fewer embedded newlines than +/// `text.lines().count()`; [`source_view`]'s own row loop indexes +/// defensively for the same reason). +/// +/// The hard part: a highlight span **can** cross a newline (a multiline +/// block comment, a triple-quoted string), so it is not enough to split on +/// `\n` -- a span open at a line boundary must be closed before the split +/// and reopened after it, or the two resulting fragments are not +/// independently well-formed HTML. This walks [`tokenize`]'s token stream +/// with an explicit stack of open span classes: a `Text` token's embedded +/// newlines close every open span, end the current line, and reopen them +/// (in the same order) at the start of the next. +fn split_highlighted_lines(html: &str, line_count: usize) -> Vec<String> { + let mut lines: Vec<String> = Vec::with_capacity(line_count.max(1)); + let mut current = String::new(); + let mut open: Vec<&str> = Vec::new(); + + for token in tokenize(html) { + match token { + Token::Open(class) => { + current.push_str("<span class=\""); + current.push_str(class); + current.push_str("\">"); + open.push(class); + } + Token::Close => { + current.push_str("</span>"); + open.pop(); + } + Token::Text(text) => { + let mut parts = text.split('\n'); + if let Some(first) = parts.next() { + current.push_str(first); + } + for rest in parts { + for _ in &open { + current.push_str("</span>"); + } + lines.push(std::mem::take(&mut current)); + for class in &open { + current.push_str("<span class=\""); + current.push_str(class); + current.push_str("\">"); + } + current.push_str(rest); + } + } + } + } + lines.push(current); + lines +} + +/// One tokenized fragment of arborium's `HtmlFormat::ClassNames` output +/// (`arborium_highlight::render::spans_to_html`'s own doc): an opening +/// `<span class="...">`, its matching `</span>`, or a run of +/// already-escaped text between tags. That renderer never emits any tag +/// but these two, and every text run it emits is already HTML-escaped +/// (`&lt;`, `&amp;`, ...) -- [`split_highlighted_lines`] never re-escapes +/// or splits an entity, since [`Token::Text`] is only ever split on +/// literal `\n` bytes, never re-parsed. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum Token<'a> { + /// `<span class="{0}">`. + Open(&'a str), + /// `</span>`. + Close, + /// Already-escaped text between tags. + Text(&'a str), +} + +/// Tokenize `html` into a stream of [`Token`]s -- see [`Token`]'s own doc +/// for why a simple `<span class="...">`/`</span>` scan is sufficient +/// (arborium's own HTML renderer emits no other tag, and every text run is +/// already escaped so it never contains a literal `<`). Malformed input +/// (which arborium's own renderer never produces) degrades to treating the +/// unrecognized byte as plain text rather than panicking or looping +/// forever. +fn tokenize(html: &str) -> Vec<Token<'_>> { + const OPEN_PREFIX: &str = "<span class=\""; + const CLOSE_TAG: &str = "</span>"; + + let mut tokens = Vec::new(); + let mut rest = html; + while !rest.is_empty() { + // `.get(..)`/`.get(n..)` rather than direct indexing throughout: + // every offset here comes from `find`/`strip_prefix`, always a + // valid char boundary, but this function still never indexes a + // `str` directly (`clippy::string_slice`) or performs raw + // arithmetic on an offset (`clippy::arithmetic_side_effects`) -- + // `.get(end..)` then `strip_prefix('"')` finds "just past the + // quote" without ever computing `end + 1`. + if let Some(after_prefix) = rest.strip_prefix(OPEN_PREFIX) + && let Some(end) = after_prefix.find('"') + && let Some(class) = after_prefix.get(..end) + && let Some(after_quote) = after_prefix.get(end..).and_then(|s| s.strip_prefix('"')) + && let Some(after_gt) = after_quote.strip_prefix('>') + { + tokens.push(Token::Open(class)); + rest = after_gt; + continue; + } + if let Some(after) = rest.strip_prefix(CLOSE_TAG) { + tokens.push(Token::Close); + rest = after; + continue; + } + let next_tag = [rest.find(OPEN_PREFIX), rest.find(CLOSE_TAG)] + .into_iter() + .flatten() + .min(); + match next_tag { + Some(0) | None => { + // No recognized tag anywhere ahead (or, defensively, right + // at the cursor despite the checks above not matching it + // -- malformed input arborium never actually produces): + // take the rest as one text run rather than looping. + tokens.push(Token::Text(rest)); + rest = ""; + } + Some(idx) => { + let text = rest.get(..idx).unwrap_or(rest); + rest = rest.get(idx..).unwrap_or_default(); + tokens.push(Token::Text(text)); + } + } + } + tokens } /// Highlighted HTML for `source`, or `None` when `name`'s extension names @@ -352,9 +608,24 @@ mod tests { #![allow(clippy::expect_used, reason = "unit test")] + use ents_anchor::LineRange; use rstest::rstest; use super::*; + use crate::pages::comments::FileComment; + + /// A minimal [`FileComment`] fixture -- the `body`/`author`/`seconds` + /// values never matter to a rendering-position assertion, only + /// `lines`. + fn comment(lines: Option<LineRange>) -> FileComment { + FileComment { + author: "commenter".to_owned(), + seconds: 0, + lines, + outdated: false, + body: html! { p { "worth a look" } }, + } + } #[rstest] #[case::empty("", true)] @@ -392,35 +663,32 @@ #[test] fn blob_view_renders_markdown_as_a_heading_not_raw_markup() { - let rendered = blob_view("readme.md", b"# Title\n") - .expect("markdown renders") - .into_string(); - assert!(rendered.contains("<h1>Title</h1>")); + let (body, _below) = blob_view("readme.md", b"# Title\n", &[]).expect("markdown renders"); + assert!(body.into_string().contains("<h1>Title</h1>")); } #[test] fn blob_view_renders_asciidoc_as_a_heading_not_raw_markup() { - let rendered = blob_view("readme.adoc", b"= Title\n\nBody.\n") - .expect("asciidoc renders") - .into_string(); - assert!(rendered.contains("<h1>Title</h1>")); + let (body, _below) = + blob_view("readme.adoc", b"= Title\n\nBody.\n", &[]).expect("asciidoc renders"); + assert!(body.into_string().contains("<h1>Title</h1>")); } #[test] fn blob_view_escapes_plain_text_into_a_line_numbered_code_block() { - let rendered = blob_view("notes.txt", b"1 < 2 and true") - .expect("plain text renders") - .into_string(); + let (body, _below) = + blob_view("notes.txt", b"1 < 2 and true", &[]).expect("plain text renders"); + let rendered = body.into_string(); assert!(rendered.contains("blob-nums")); - assert!(rendered.contains("<pre class=\"blob-code\"><code>")); + assert!(rendered.contains("<td class=\"blob-code\"><code>")); assert!(rendered.contains("1 &lt; 2")); } #[test] fn blob_view_highlights_a_recognized_language_with_syntax_token_classes() { - let rendered = blob_view("main.rs", b"fn main() { let x = 1; }") - .expect("rust renders") - .into_string(); + let (body, _below) = + blob_view("main.rs", b"fn main() { let x = 1; }", &[]).expect("rust renders"); + let rendered = body.into_string(); assert!(rendered.contains("blob-nums")); assert!(rendered.contains("class=\"code\"")); assert!(rendered.contains("class=\"keyword\"")); @@ -428,10 +696,64 @@ #[test] fn blob_view_shows_a_placeholder_for_binary_content() { - let rendered = blob_view("data.bin", b"\0\x01\x02binary") - .expect("binary placeholder renders") - .into_string(); - assert!(rendered.contains("Binary file")); + let (body, _below) = + blob_view("data.bin", b"\0\x01\x02binary", &[]).expect("binary placeholder renders"); + assert!(body.into_string().contains("Binary file")); + } + + #[test] + fn blob_view_routes_a_doc_comment_below_the_blob_never_inline() { + let comments = vec![comment(Some(LineRange { start: 1, end: 1 }))]; + let (_body, below) = + blob_view("readme.md", b"# Title\n", &comments).expect("markdown renders"); + // A doc view has no source line to interleave at: every comment, + // even one with a current line range, renders in the below + // section -- `comments_section`'s plain, untitled list, not + // `outdated_comments_section`'s titled one. + assert!(below.into_string().contains("worth a look")); + } + + #[test] + fn source_view_interleaves_a_comment_directly_after_its_last_line() { + let comments = vec![comment(Some(LineRange { start: 1, end: 2 }))]; + let rendered = source_view("line 1\nline 2\nline 3\n", None, &comments).into_string(); + let line2 = rendered.find("id=\"L2\"").expect("line 2 renders"); + let card = rendered.find("comment-meta").expect("card renders"); + let line3 = rendered.find("id=\"L3\"").expect("line 3 renders"); + assert!( + line2 < card && card < line3, + "the card lands strictly between line 2 and line 3: {rendered}" + ); + } + + #[test] + fn source_view_stacks_multiple_comments_ending_on_the_same_line_in_order() { + let comments = vec![ + { + let mut c = comment(Some(LineRange { start: 1, end: 1 })); + c.body = html! { p { "first" } }; + c + }, + { + let mut c = comment(Some(LineRange { start: 1, end: 1 })); + c.body = html! { p { "second" } }; + c + }, + ]; + let rendered = source_view("line 1\nline 2\n", None, &comments).into_string(); + let first = rendered.find("first").expect("first comment renders"); + let second = rendered.find("second").expect("second comment renders"); + assert!(first < second, "stacked comments keep ref order"); + } + + #[test] + fn source_view_omits_a_comment_with_no_current_line_range() { + let comments = vec![comment(None)]; + let rendered = source_view("line 1\nline 2\n", None, &comments).into_string(); + assert!( + !rendered.contains("worth a look"), + "a comment with no lines has nowhere to interleave -- blob_view routes it below instead" + ); } #[test] @@ -439,4 +761,57 @@ assert_eq!(child_href("", "src"), "/files/src"); assert_eq!(child_href("src", "main.rs"), "/files/src/main.rs"); } + + #[test] + fn tokenize_splits_spans_and_text_without_touching_entities() { + let tokens = tokenize("<span class=\"keyword\">fn</span> 1 &lt; 2"); + assert_eq!( + tokens, + vec![ + Token::Open("keyword"), + Token::Text("fn"), + Token::Close, + Token::Text(" 1 &lt; 2"), + ] + ); + } + + #[test] + fn split_highlighted_lines_reopens_a_span_that_crosses_a_newline() { + // A three-line block comment as one span, per arborium's own + // `spans_to_html` shape (see that function's own tests): one + // `<span>` whose text contains embedded newlines, followed by an + // unrelated keyword span on the line after. + let html = "<span class=\"comment\">/*\nfoo\nbar*/</span>\n<span class=\"keyword\">fn</span> main() {}"; + let lines = split_highlighted_lines(html, 4); + assert_eq!( + lines, + vec![ + "<span class=\"comment\">/*</span>".to_owned(), + "<span class=\"comment\">foo</span>".to_owned(), + "<span class=\"comment\">bar*/</span>".to_owned(), + "<span class=\"keyword\">fn</span> main() {}".to_owned(), + ], + "each fragment is independently well-formed and still classed" + ); + } + + #[test] + fn split_highlighted_lines_never_re_escapes_or_splits_an_entity() { + let html = "<span class=\"operator\">&lt;</span>\nnext"; + let lines = split_highlighted_lines(html, 2); + assert_eq!( + lines, + vec![ + "<span class=\"operator\">&lt;</span>".to_owned(), + "next".to_owned(), + ] + ); + } + + #[test] + fn split_highlighted_lines_handles_plain_unhighlighted_text() { + let lines = split_highlighted_lines("a\nb\nc", 3); + assert_eq!(lines, vec!["a".to_owned(), "b".to_owned(), "c".to_owned()]); + } }