Signed by hannaSSH key fingerprint: SHA256:4g9cWhkAAw8gwqhJUcVbSnGEvdGwklW+1Aa/fMUu59k
assets/base.css +4 −0
| @@ -2114,6 +2114,10 @@ pre.code { | |||
| 2114 | 2114 | color: var(--fb-accent); | |
| 2115 | 2115 | border-color: var(--fb-accent); | |
| 2116 | 2116 | } | |
| 2117 | + | /* An inline #n issue/PR cross-reference link. */ | |
| 2118 | + | .issue-ref { | |
| 2119 | + | font-weight: 600; | |
| 2120 | + | } | |
| 2117 | 2121 | .repo-tabs a, | |
| 2118 | 2122 | .profile-tabs a, | |
| 2119 | 2123 | .subnav a { | |
crates/web/src/issues.rs +20 −5
| @@ -201,6 +201,7 @@ pub(crate) async fn new_form( | |||
| 201 | 201 | } | |
| 202 | 202 | ||
| 203 | 203 | /// `GET …/-/issues/{n}` — a single issue with comments and controls. | |
| 204 | + | #[allow(clippy::too_many_lines)] // Cohesive view: header, thread, sidebar. | |
| 204 | 205 | pub(crate) async fn view( | |
| 205 | 206 | state: &AppState, | |
| 206 | 207 | viewer: Option<User>, | |
| @@ -213,11 +214,23 @@ pub(crate) async fn view( | |||
| 213 | 214 | if (kind.is_pull && !ctx.repo.pulls_enabled) || (!kind.is_pull && !ctx.repo.issues_enabled) { | |
| 214 | 215 | return Err(AppError::NotFound); | |
| 215 | 216 | } | |
| 216 | - | let issue = state | |
| 217 | + | let Some(issue) = state | |
| 217 | 218 | .store | |
| 218 | 219 | .issue_by_number(&ctx.repo.id, number, kind.is_pull) | |
| 219 | 220 | .await? | |
| 220 | - | .ok_or(AppError::NotFound)?; | |
| 221 | + | else { | |
| 222 | + | // A `#n` reference may point at the other kind — redirect there. | |
| 223 | + | if let Some(other) = state | |
| 224 | + | .store | |
| 225 | + | .issue_by_number(&ctx.repo.id, number, !kind.is_pull) | |
| 226 | + | .await? | |
| 227 | + | { | |
| 228 | + | let seg = if other.is_pull { "pulls" } else { "issues" }; | |
| 229 | + | let dest = format!("/{}/{}/-/{seg}/{number}", ctx.owner.username, ctx.repo.path); | |
| 230 | + | return Ok(Redirect::to(&dest).into_response()); | |
| 231 | + | } | |
| 232 | + | return Err(AppError::NotFound); | |
| 233 | + | }; | |
| 221 | 234 | let author = username(state, &issue.author_id).await; | |
| 222 | 235 | let comments = state.store.list_comments(&issue.id).await?; | |
| 223 | 236 | let issue_labels = state.store.issue_labels(&issue.id).await?; | |
| @@ -241,6 +254,7 @@ pub(crate) async fn view( | |||
| 241 | 254 | ||
| 242 | 255 | let (jar, chrome) = build_chrome(state, jar, viewer, uri.path().to_string()); | |
| 243 | 256 | let csrf = chrome.csrf.clone(); | |
| 257 | + | let base = format!("/{}/{}", ctx.owner.username, ctx.repo.path); | |
| 244 | 258 | let body = html! { | |
| 245 | 259 | (repo_header(state, &ctx, kind.tab, &ctx.repo.default_branch, &branches)) | |
| 246 | 260 | div class="issue-view" { | |
| @@ -259,12 +273,12 @@ pub(crate) async fn view( | |||
| 259 | 273 | div class="issue-main" { | |
| 260 | 274 | article class="issue-thread" { | |
| 261 | 275 | @let body_action = format!("/issue/{}/edit-body", issue.id); | |
| 262 | - | (comment_card(&author, &issue.body, issue.created_at, | |
| 276 | + | (comment_card(&author, &base, &issue.body, issue.created_at, | |
| 263 | 277 | (can_write || is_author).then_some((body_action.as_str(), csrf.as_str())))) | |
| 264 | 278 | @for (c, cauthor) in &comment_rows { | |
| 265 | 279 | @let editable = can_write || viewer_id.as_deref() == Some(c.author_id.as_str()); | |
| 266 | 280 | @let action = format!("/comment/{}/edit", c.id); | |
| 267 | - | (comment_card(cauthor, &c.body, c.created_at, | |
| 281 | + | (comment_card(cauthor, &base, &c.body, c.created_at, | |
| 268 | 282 | editable.then_some((action.as_str(), csrf.as_str())))) | |
| 269 | 283 | } | |
| 270 | 284 | @if issue.locked() { (locked_note(can_write)) } | |
| @@ -310,6 +324,7 @@ pub(crate) async fn view( | |||
| 310 | 324 | /// issue/PR description. | |
| 311 | 325 | pub(crate) fn comment_card( | |
| 312 | 326 | author: &str, | |
| 327 | + | repo_base: &str, | |
| 313 | 328 | body: &str, | |
| 314 | 329 | created_at: i64, | |
| 315 | 330 | edit: Option<(&str, &str)>, | |
| @@ -330,7 +345,7 @@ pub(crate) fn comment_card( | |||
| 330 | 345 | } | |
| 331 | 346 | } | |
| 332 | 347 | } | |
| 333 | - | div class="comment-body markdown" { (markdown::render(body)) } | |
| 348 | + | div class="comment-body markdown" { (markdown::render_with_refs(body, repo_base)) } | |
| 334 | 349 | @if let Some((action, csrf)) = edit { | |
| 335 | 350 | form class="comment-editor" method="post" action=(action) { | |
| 336 | 351 | input type="hidden" name="_csrf" value=(csrf); | |
crates/web/src/markdown.rs +90 −2
| @@ -21,6 +21,18 @@ use maud::{Markup, PreEscaped}; | |||
| 21 | 21 | /// Render `source` markdown to sanitized, syntax-highlighted HTML. | |
| 22 | 22 | #[must_use] | |
| 23 | 23 | pub fn render(source: &str) -> Markup { | |
| 24 | + | PreEscaped(render_html(source)) | |
| 25 | + | } | |
| 26 | + | ||
| 27 | + | /// Like [`render`], but also linkifies `#123` issue/PR references against | |
| 28 | + | /// `repo_base` (e.g. `/owner/repo`). Used for issue/PR descriptions and comments. | |
| 29 | + | #[must_use] | |
| 30 | + | pub fn render_with_refs(source: &str, repo_base: &str) -> Markup { | |
| 31 | + | PreEscaped(linkify_refs(&render_html(source), repo_base)) | |
| 32 | + | } | |
| 33 | + | ||
| 34 | + | /// The shared comrak + ammonia pipeline, producing sanitized HTML. | |
| 35 | + | fn render_html(source: &str) -> String { | |
| 24 | 36 | let mut options = comrak::Options::default(); | |
| 25 | 37 | options.extension.table = true; | |
| 26 | 38 | options.extension.strikethrough = true; | |
| @@ -33,8 +45,63 @@ pub fn render(source: &str) -> Markup { | |||
| 33 | 45 | plugins.render.codefence_syntax_highlighter = Some(&adapter); | |
| 34 | 46 | ||
| 35 | 47 | let unsafe_html = comrak::markdown_to_html_with_plugins(source, &options, &plugins); | |
| 36 | - | let safe = sanitizer().clean(&unsafe_html).to_string(); | |
| 37 | - | PreEscaped(safe) | |
| 48 | + | sanitizer().clean(&unsafe_html).to_string() | |
| 49 | + | } | |
| 50 | + | ||
| 51 | + | /// Turn `#123` into a link to the issue/PR, but only in ordinary text — never | |
| 52 | + | /// inside a tag, an attribute, or a `<code>`/`<pre>` block. Runs on already | |
| 53 | + | /// sanitized HTML, so the injected anchors are trusted. The link targets the | |
| 54 | + | /// issues path; the issue view redirects to the PR view when the number is a PR. | |
| 55 | + | fn linkify_refs(html: &str, base: &str) -> String { | |
| 56 | + | let bytes = html.as_bytes(); | |
| 57 | + | // Build as bytes so multibyte UTF-8 is copied verbatim (only ASCII markers | |
| 58 | + | // are special-cased). | |
| 59 | + | let mut out: Vec<u8> = Vec::with_capacity(html.len() + 32); | |
| 60 | + | let mut i = 0; | |
| 61 | + | let mut in_tag = false; | |
| 62 | + | let mut code_depth: usize = 0; | |
| 63 | + | while i < bytes.len() { | |
| 64 | + | let c = bytes[i]; | |
| 65 | + | if in_tag { | |
| 66 | + | out.push(c); | |
| 67 | + | if c == b'>' { | |
| 68 | + | in_tag = false; | |
| 69 | + | } | |
| 70 | + | i += 1; | |
| 71 | + | continue; | |
| 72 | + | } | |
| 73 | + | if c == b'<' { | |
| 74 | + | let rest = &html[i..]; | |
| 75 | + | if rest.starts_with("<code") || rest.starts_with("<pre") { | |
| 76 | + | code_depth += 1; | |
| 77 | + | } else if rest.starts_with("</code") || rest.starts_with("</pre") { | |
| 78 | + | code_depth = code_depth.saturating_sub(1); | |
| 79 | + | } | |
| 80 | + | in_tag = true; | |
| 81 | + | out.push(b'<'); | |
| 82 | + | i += 1; | |
| 83 | + | continue; | |
| 84 | + | } | |
| 85 | + | // A `#` that follows a non-alphanumeric boundary and is followed by digits. | |
| 86 | + | if code_depth == 0 && c == b'#' && (i == 0 || !bytes[i - 1].is_ascii_alphanumeric()) { | |
| 87 | + | let mut j = i + 1; | |
| 88 | + | while j < bytes.len() && bytes[j].is_ascii_digit() { | |
| 89 | + | j += 1; | |
| 90 | + | } | |
| 91 | + | if j > i + 1 { | |
| 92 | + | let num = &html[i + 1..j]; | |
| 93 | + | out.extend_from_slice( | |
| 94 | + | format!("<a class=\"issue-ref\" href=\"{base}/-/issues/{num}\">#{num}</a>") | |
| 95 | + | .as_bytes(), | |
| 96 | + | ); | |
| 97 | + | i = j; | |
| 98 | + | continue; | |
| 99 | + | } | |
| 100 | + | } | |
| 101 | + | out.push(c); | |
| 102 | + | i += 1; | |
| 103 | + | } | |
| 104 | + | String::from_utf8(out).unwrap_or_else(|_| html.to_string()) | |
| 38 | 105 | } | |
| 39 | 106 | ||
| 40 | 107 | /// The ammonia sanitizer, extended to allow the highlighter's `<span class="hl-*">` | |
| @@ -140,6 +207,27 @@ mod tests { | |||
| 140 | 207 | } | |
| 141 | 208 | ||
| 142 | 209 | #[test] | |
| 210 | + | fn linkifies_issue_refs_outside_code() { | |
| 211 | + | let html = render_with_refs("see #12 and `#34` here", "/o/r").into_string(); | |
| 212 | + | assert!( | |
| 213 | + | html.contains(r#"<a class="issue-ref" href="/o/r/-/issues/12">#12</a>"#), | |
| 214 | + | "plain #12 links: {html}" | |
| 215 | + | ); | |
| 216 | + | // The `#34` inside a code span is left alone. | |
| 217 | + | assert!(html.contains("#34</code>"), "code #34 untouched: {html}"); | |
| 218 | + | assert!( | |
| 219 | + | !html.contains("issues/34"), | |
| 220 | + | "code #34 must not link: {html}" | |
| 221 | + | ); | |
| 222 | + | } | |
| 223 | + | ||
| 224 | + | #[test] | |
| 225 | + | fn does_not_link_word_hash() { | |
| 226 | + | let html = render_with_refs("color abc#5 def", "/o/r").into_string(); | |
| 227 | + | assert!(!html.contains("issues/5"), "abc#5 is not a ref: {html}"); | |
| 228 | + | } | |
| 229 | + | ||
| 230 | + | #[test] | |
| 143 | 231 | fn highlights_fenced_code() { | |
| 144 | 232 | let html = render("```rust\nfn main() {}\n```").into_string(); | |
| 145 | 233 | // A keyword span survives sanitizing with its highlight class. | |
crates/web/src/pulls.rs +73 −4
| @@ -93,6 +93,7 @@ pub(crate) async fn new_form( | |||
| 93 | 93 | } | |
| 94 | 94 | ||
| 95 | 95 | /// `GET …/-/pulls/{n}` — a pull request: conversation, commits, and diff. | |
| 96 | + | #[allow(clippy::too_many_lines)] // Cohesive view: header, thread, merge panel, diff. | |
| 96 | 97 | pub(crate) async fn view( | |
| 97 | 98 | state: &AppState, | |
| 98 | 99 | viewer: Option<User>, | |
| @@ -104,11 +105,27 @@ pub(crate) async fn view( | |||
| 104 | 105 | if !ctx.repo.pulls_enabled { | |
| 105 | 106 | return Err(AppError::NotFound); | |
| 106 | 107 | } | |
| 107 | - | let issue = state | |
| 108 | + | let Some(issue) = state | |
| 108 | 109 | .store | |
| 109 | 110 | .issue_by_number(&ctx.repo.id, number, true) | |
| 110 | 111 | .await? | |
| 111 | - | .ok_or(AppError::NotFound)?; | |
| 112 | + | else { | |
| 113 | + | // A `#n` reference to a plain issue lands here — redirect to it. | |
| 114 | + | if ctx.repo.issues_enabled | |
| 115 | + | && state | |
| 116 | + | .store | |
| 117 | + | .issue_by_number(&ctx.repo.id, number, false) | |
| 118 | + | .await? | |
| 119 | + | .is_some() | |
| 120 | + | { | |
| 121 | + | let dest = format!( | |
| 122 | + | "/{}/{}/-/issues/{number}", | |
| 123 | + | ctx.owner.username, ctx.repo.path | |
| 124 | + | ); | |
| 125 | + | return Ok(Redirect::to(&dest).into_response()); | |
| 126 | + | } | |
| 127 | + | return Err(AppError::NotFound); | |
| 128 | + | }; | |
| 112 | 129 | let pr = state | |
| 113 | 130 | .store | |
| 114 | 131 | .pull_by_issue(&issue.id) | |
| @@ -135,6 +152,7 @@ pub(crate) async fn view( | |||
| 135 | 152 | ||
| 136 | 153 | let (jar, chrome) = build_chrome(state, jar, viewer, uri.path().to_string()); | |
| 137 | 154 | let csrf = chrome.csrf.clone(); | |
| 155 | + | let repo_base = format!("/{}/{}", ctx.owner.username, ctx.repo.path); | |
| 138 | 156 | let body = html! { | |
| 139 | 157 | (repo_header(state, &ctx, Tab::Pulls, &ctx.repo.default_branch, &branches)) | |
| 140 | 158 | div class="issue-view" { | |
| @@ -156,12 +174,12 @@ pub(crate) async fn view( | |||
| 156 | 174 | div class="issue-main" { | |
| 157 | 175 | article class="issue-thread" { | |
| 158 | 176 | @let body_action = format!("/issue/{}/edit-body", issue.id); | |
| 159 | - | (comment_card(&author, &issue.body, issue.created_at, | |
| 177 | + | (comment_card(&author, &repo_base, &issue.body, issue.created_at, | |
| 160 | 178 | (can_write || is_author).then_some((body_action.as_str(), csrf.as_str())))) | |
| 161 | 179 | @for (c, cauthor) in &comment_rows { | |
| 162 | 180 | @let editable = can_write || viewer_id.as_deref() == Some(c.author_id.as_str()); | |
| 163 | 181 | @let action = format!("/comment/{}/edit", c.id); | |
| 164 | - | (comment_card(cauthor, &c.body, c.created_at, | |
| 182 | + | (comment_card(cauthor, &repo_base, &c.body, c.created_at, | |
| 165 | 183 | editable.then_some((action.as_str(), csrf.as_str())))) | |
| 166 | 184 | } | |
| 167 | 185 | (merge_panel(&issue, &pr, mergeable, blocked, &csrf)) | |
| @@ -570,12 +588,47 @@ pub async fn merge( | |||
| 570 | 588 | .store | |
| 571 | 589 | .mark_pull_merged(&issue.id, &user.id, &sha, strategy.as_str()) | |
| 572 | 590 | .await?; | |
| 591 | + | // Close any issues the PR body says it closes ("closes #1", "fixes #2"). | |
| 592 | + | for number in closing_refs(&issue.body) { | |
| 593 | + | if let Some(target) = state.store.issue_by_number(&repo.id, number, false).await? | |
| 594 | + | && target.state == model::IssueState::Open | |
| 595 | + | { | |
| 596 | + | let _ = state | |
| 597 | + | .store | |
| 598 | + | .set_issue_state(&target.id, model::IssueState::Closed) | |
| 599 | + | .await; | |
| 600 | + | } | |
| 601 | + | } | |
| 573 | 602 | Ok::<String, AppError>(issue_url(&state, &repo, &issue).await) | |
| 574 | 603 | } | |
| 575 | 604 | .await; | |
| 576 | 605 | respond_redirect(result) | |
| 577 | 606 | } | |
| 578 | 607 | ||
| 608 | + | /// Parse GitHub-style closing keywords (`closes #1`, `fixes #2`, `resolves #3`) | |
| 609 | + | /// from a PR body, returning the referenced issue numbers. | |
| 610 | + | fn closing_refs(text: &str) -> Vec<i64> { | |
| 611 | + | const KEYWORDS: &[&str] = &[ | |
| 612 | + | "close", "closes", "closed", "fix", "fixes", "fixed", "resolve", "resolves", "resolved", | |
| 613 | + | ]; | |
| 614 | + | let lower = text.to_lowercase(); | |
| 615 | + | let words: Vec<&str> = lower.split_whitespace().collect(); | |
| 616 | + | let mut out = Vec::new(); | |
| 617 | + | for pair in words.windows(2) { | |
| 618 | + | if KEYWORDS.contains(&pair[0]) | |
| 619 | + | && let Some(rest) = pair[1].strip_prefix('#') | |
| 620 | + | { | |
| 621 | + | let digits: String = rest.chars().take_while(char::is_ascii_digit).collect(); | |
| 622 | + | if let Ok(n) = digits.parse::<i64>() { | |
| 623 | + | out.push(n); | |
| 624 | + | } | |
| 625 | + | } | |
| 626 | + | } | |
| 627 | + | out.sort_unstable(); | |
| 628 | + | out.dedup(); | |
| 629 | + | out | |
| 630 | + | } | |
| 631 | + | ||
| 579 | 632 | /// Run the blocking server-side merge on the blocking pool. | |
| 580 | 633 | async fn run_merge( | |
| 581 | 634 | state: &AppState, | |
| @@ -628,3 +681,19 @@ async fn run_merge( | |||
| 628 | 681 | other => AppError::from(other), | |
| 629 | 682 | }) | |
| 630 | 683 | } | |
| 684 | + | ||
| 685 | + | #[cfg(test)] | |
| 686 | + | mod tests { | |
| 687 | + | use super::closing_refs; | |
| 688 | + | ||
| 689 | + | #[test] | |
| 690 | + | fn parses_closing_keywords() { | |
| 691 | + | assert_eq!(closing_refs("Closes #1 and fixes #2."), vec![1, 2]); | |
| 692 | + | assert_eq!(closing_refs("resolves #10"), vec![10]); | |
| 693 | + | // No keyword, or a plain mention, does not close. | |
| 694 | + | assert!(closing_refs("see #3").is_empty()); | |
| 695 | + | assert!(closing_refs("closesnt #4").is_empty()); | |
| 696 | + | // Deduped and sorted. | |
| 697 | + | assert_eq!(closing_refs("fix #5 fix #5 close #2"), vec![2, 5]); | |
| 698 | + | } | |
| 699 | + | } | |