diff --git a/CLAUDE.md b/CLAUDE.md index fcaafe6..d877053 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -84,21 +84,38 @@ The app is one window with three panes, and the mental model that drives the cod language server join. `TreeSitterHighlighter` survives for the diff and for markdown snippets, which live in no editor. Don't reach for an `NSTextView` API here: the package's `TextView` is not one. -- **Markdown** (`Views/MarkdownPreview.swift`) — a hand-rolled block renderer used - for `.md` files, PR descriptions and comments. A fenced block is coloured by - `Models/MarkdownCodeHighlighter.swift`, which runs the editor's - `TreeSitterHighlighter` over the snippet and takes its colours from the - current palette — the fence's language word replaces the file name the editor - detects from, and the result is cached per (language, palette, text). A - ` ```mermaid ` fence goes to `Views/MermaidDiagramView.swift`. - Both hosts render a comment as Markdown *with HTML in it*, and a bot leans on - that hard, so `Support/MarkdownHTMLText.swift` handles the tags a comment - actually uses: `` is dropped, `
`/`` and - `
` become blocks the parser recurses into, and every inline tag is - rewritten as the Markdown that says the same thing before the line is - classified. It is not an HTML parser — an unknown tag is dropped and its text - kept. `MarkdownText.Block` is `indirect` because of those containers, so a new - case has to be answered in `MarkdownHTML` too. +- **Markdown** — the parse and the renderer are separate on purpose, and only + the renderer is ours. `Support/MarkdownParser.swift` walks + [swift-markdown](https://github.com/swiftlang/swift-markdown)'s tree (cmark-gfm, + which is what both hosts render a comment with) into `MarkdownText.Block`, and + `Views/MarkdownPreview.swift` draws those blocks for `.md` files, PR + descriptions and comments alike. Read `Docs/Markdown.md` before changing any of + it: it records why the parser was replaced and why a library that renders too + was the wrong trade. + - The blocks carry **Markdown strings**, which the views read back with + `AttributedString(markdown:)` — so everything the parser writes into one goes + through `Support/MarkdownInline.swift`, which escapes plain text and is also + where the four things cmark does not answer live: a bare URL, `:tada:`, + `#123` and `@name`. That is one scan over `Text` nodes, not four passes. + - HTML arrives **raw and flat**: cmark splits `
` into three siblings — + the open tag, the inner Markdown parsed properly, the close tag — so + `Support/MarkdownHTMLText.swift` reads one raw block into events and + `ContainerStack` in the parser keeps the stack. It is not an HTML parser: an + unknown tag is dropped and its text kept, `` goes nowhere, and + only the plain shape of a `` is read. + - Comments are dropped **from the tree, not from the text**, because every + node carries a 1-based source line and that is what a tickable checkbox + writes back to (`MarkdownTask.toggling`). Cutting lines out beforehand would + move every line number in the document. + - A fenced block is coloured by `Models/MarkdownCodeHighlighter.swift`, which + runs the editor's `TreeSitterHighlighter` over the snippet and takes its + colours from the current palette — the fence's language word replaces the + file name the editor detects from, and the result is cached per (language, + palette, text). A ` ```mermaid ` fence goes to `Views/MermaidDiagramView.swift`. + - `MarkdownText.Block` is `indirect` because a quote, a `
` section and + a list item all hold blocks, so a new case has to be answered in + `Support/MarkdownHTML.swift` too — that is the PDF, and it walks the same + blocks on purpose so the page and the file cannot drift. - **Diagrams** (`Views/DiagramWebView.swift`) — mermaid fences and `.drawio` files are both drawn by the real JavaScript library in a `WKWebView`, and both go through this one representable: it loads a page from `Resources/` as a diff --git a/Docs/Development.md b/Docs/Development.md index d00cb06..9f5de4e 100644 --- a/Docs/Development.md +++ b/Docs/Development.md @@ -97,6 +97,7 @@ Nothing is bundled — a server is used if it is on your `PATH`. | C / C++ / Objective-C | `clangd` | | Ruby, PHP, Dart, Lua, Kotlin | `ruby-lsp`, `intelephense`, `dart`, `lua-language-server`, `kotlin-lsp` (JetBrains') | | JSON, YAML, HTML, CSS, Bash | the matching `vscode-*-language-server` / `yaml-language-server` / `bash-language-server` | +| Markdown | `marksman` — link and heading completion, go-to-definition between documents, and a diagnostic for a link that points nowhere | That table is only the default list: **Settings → Language Servers** installs the missing ones, corrects a command, and adds a server for any language that is not @@ -109,8 +110,8 @@ your global npm packages or your gems, and that folder goes on the front of the repository wants one; **Settings → Language Servers** has the switch and a **Remove Downloaded** button. Only the npm-published servers are fetched this way — the ones that arrive with a toolchain (`sourcekit-lsp`, `clangd`, `dart`) -or as a platform binary (`rust-analyzer`, `kotlin-lsp`, `ruby-lsp`) keep their -Install button. +or as a platform binary (`rust-analyzer`, `kotlin-lsp`, `ruby-lsp`, `marksman`) +keep their Install button. Servers are **started while the dashboard is on screen**, not when a file is opened. Selecting a repository lists its files, works out which languages are @@ -163,9 +164,11 @@ Sources/Workspace/ Support/ViewerMotion.swift how long a screen takes to arrive, in one place Support/PullRequestReference.swift finding a `#123` written in a commit message Support/FileOperations.swift what the Files tab does to disk: create, copy, move, rename, trash + Support/MarkdownParser.swift cmark's block tree walked into the preview's blocks; ticking a box + Support/MarkdownInline.swift one scan over the text: escaping, bare URLs, :emoji:, #123, @name Support/MarkdownPDF.swift the preview as a PDF: measure, render, cut into sheets Support/MarkdownHTML.swift the same document as a printable page, diagrams and all - Support/MarkdownHTMLText.swift the HTML in a comment: `
`, `
`, `&`, hidden comments + Support/MarkdownHTMLText.swift the HTML in a comment: `
`, `
`, `
`, `&`, hidden comments Support/StreamingShellProcess.swift a command that stays up, read a line at a time Support/JSONValue.swift JSON of a shape only known at runtime (tool inputs) Support/RemoteImage.swift pictures Markdown points at: download once, draw, fall back @@ -241,7 +244,7 @@ Sources/Workspace/ ChatInputField.swift the growing text box the comment boxes are built on ChatCompletionList.swift the @ list, and reading the token under the caret MentionTextBox.swift a comment box where @ names a person on the pull request - MarkdownPreview.swift the Markdown renderer: blocks, tables, code chips + MarkdownPreview.swift the Markdown renderer: blocks, lists, tables, code chips, the outline — see Docs/Markdown.md DiagramWebView.swift the web view both diagram renderers are driven in MermaidDiagramView.swift a ```mermaid fence, drawn in the Markdown column DrawioPreview.swift a .drawio file, drawn in the whole pane diff --git a/Docs/Markdown.md b/Docs/Markdown.md new file mode 100644 index 0000000..713db95 --- /dev/null +++ b/Docs/Markdown.md @@ -0,0 +1,142 @@ +# Markdown: swift-markdown + +**Done.** This was written as a plan and is kept as the reasoning — every stage +at the bottom has landed. What follows is why the parser is what it is; the walk +itself is `Support/MarkdownParser.swift`. + +Markdown is rendered three times over in this app — a `.md` file in the viewer, a +pull request's description, and every comment on it — and all three go through one +type, `MarkdownText` in `Views/MarkdownPreview.swift`. Its parser *was* +hand-rolled and **line-by-line**: each source line was classified on its own +(`parse(_:depth:)`), which is where every remaining gap came from. A paragraph +hard-wrapped at 80 columns became one `.paragraph` per line and was drawn as a +ladder; a continuation line under a bullet lost its bullet; `Title` over `=====` +came out as a paragraph and a rule; a fence inside a list item was hoisted out of +it. None of these were small bugs with small fixes — they were all the same +missing thing, which is a block tree. + +**The decision: replace the parser, keep the renderer.** +[swiftlang/swift-markdown](https://github.com/swiftlang/swift-markdown) is a +parser and nothing else — an immutable, thread-safe markup tree over +[swift-cmark](https://github.com/swiftlang/swift-cmark), the same cmark-gfm both +hosts use. So `MarkdownText.Block`, every view under it, `MarkdownPDF`, +`MarkdownCodeHighlighter`, the palettes, the mermaid and draw.io fences and the +mention chips all stay. Only `blocks(in:relativeTo:)` and `parse(_:depth:)` +change: a `MarkupWalker` builds the same blocks the views already draw. + +This was measured rather than assumed — every claim below was run against +swift-markdown 0.8.0 before being written down. + +## What the parser answers for free + +Each of these was a line in `TODO.md`, and each one is a node in the tree rather +than code of ours: + +| Today | What the tree gives | +| --- | --- | +| Hard-wrapped paragraphs come apart | **one** `Paragraph` holding `SoftBreak`s | +| A continuation line under a bullet becomes its own paragraph | it stays inside the `ListItem`'s paragraph | +| Nested checklists lose their indent | a nested `UnorderedList` *inside* the item — structure, not an `indent:` we carry | +| `1. [ ] thing` is not a checklist | `OrderedList` → `ListItem checkbox: [ ]` | +| Setext headings render as paragraph + rule | `Heading level: 1` | +| Rules only match `---` and `***` | `___`, `----` and `- - -` all give `ThematicBreak` | +| Indented code blocks are prose | `CodeBlock language: none` | +| Reference-style links stay literal | resolved to a `Link` with its destination; the `[ref]: …` line disappears | +| Table alignment is dropped | `columnAlignments: \|l\|c\|r\|` | +| A fence inside a list item is hoisted out | nested, as written | +| Checklists are not clickable | every node carries a **source range** (line:column) | +| No outline for long documents | `Heading` nodes, each with a range | + +The source ranges are the interesting one. A tickable checkbox has never been +about drawing a toggle — it is about knowing *which* `[ ]` in the file to flip, +and the tree says so exactly. For a `.md` file the write-back is `OpenDocument`; +for a pull request comment it is the edit path that already exists (see #19). + +## What it does not answer + +Worth writing down so it is not investigated twice. The package attaches exactly +three cmark extensions — `table`, `strikethrough`, `tasklist` — and no others: + +- **Bare URLs are still not links.** No autolink extension, so + `https://example.com` stays `Text`. Ours to do. +- **Footnotes are worse than missing.** `[^1]: note` is read as a *reference link + definition*, so `text[^1]` comes out as a link pointing at `note`. Adopting the + library does not fix footnotes, and this mangling must be caught rather than + passed through. +- **`:tada:`, `#123` and `@name`** are not Markdown at all. They are an inline + scan over `Text` nodes — and so is autolinking a bare URL, so that is one piece + of work rather than four. `CommitMessageText` already navigates `#123`. +- **HTML arrives raw.** `
`, `
` and `` come through as + `HTMLBlock` with the literal string in it, and `` as `InlineHTML`. So + `Support/MarkdownHTMLText.swift` stays exactly as load-bearing as it was. A + `
` *does* arrive whole in one `HTMLBlock`, which turned out to be enough + to read the plain shape — ``, `" }.joined() + case .table(let headers, let rows, let alignments): + let head = headers.enumerated() + .map { "\(inline($0.element))" } + .joined() let body = rows.map { row in - "" + row.map { "" }.joined() + "" + "" + row.enumerated() + .map { "\(inline($0.element))" } + .joined() + "" }.joined(separator: "\n") return """
`, `` — into the same headers and + rows a `|…|` table gives. Anything past that (a `colspan`, an `align=` on a + cell) wants [SwiftSoup](https://github.com/scinfu/SwiftSoup) rather than more + of the tag scanner. +- **A bare `- [ ]` is still not a box**, because cmark-gfm wants text after the + brackets. That stops being a bug worth fixing: both hosts *are* cmark-gfm, so + matching it is fidelity. The `TODO.md` line should be struck rather than done. +- Everything interactive or visual — the copy button on a fence, click-to-zoom on + an image, the checkbox's own control — is renderer work either way. The parser + only makes the checkbox's *effect* possible. + +## Why not the library that renders too + +[swift-markdown-ui](https://github.com/gonzalezreal/swift-markdown-ui) would draw +it as well, with themes, block styles and a code-highlighter hook. It is the +wrong trade here: raw HTML is not rendered at all, which is a straight regression +for the bot comments both hosts serve; its checkboxes are read-only; and mermaid, +draw.io, the palettes and the mention chips would all have to be re-plumbed +through its theming to arrive back where we already are. `Down` and `Ink` render +to `NSAttributedString` or HTML, meaning a `WKWebView` or the loss of the SwiftUI +blocks — the wrong direction for a column that hosts live diagram views. + +## The stages + +Each landed on its own and left the app working. + +1. ✅ **The tree, behind the same door.** The two packages are in `Package.swift`, + pinned exactly at 0.8.0. `MarkdownText.blocks(in:relativeTo:links:)` calls + `MarkdownParser`, and `parse(_:depth:)` is gone. This is where the whole first + table arrived at once. +2. ✅ **Lists became nested.** `Block` no longer carries `bullet(indent:)`, + `numbered(indent:)` and `task(done:)` as flat rows; there is one `.list` case + holding a `MarkdownList`, whose items hold blocks of their own. Answered in + `MarkdownListView`, and in `MarkdownHTML` — which writes the marker and the + item's column out by hand rather than as `
    `, so the page and the PDF + cannot drift. +3. ✅ **HTML against a flat stream.** `MarkdownHTMLText.events(in:)` reads one raw + block into `open`/`close`/`summary`/`table`/`markdown`, and `ContainerStack` + in the parser keeps the stack. Loose HTML — a README's + `

    ` — is handed back as Markdown and re-read as a + document of its own, which is what makes it a picture. +4. ✅ **One inline pass.** `MarkdownInline.decorated(_:links:inLink:)`, over `Text` + nodes only, doing the escaping in the same walk. The footnote hazard is caught + in both directions: a definition cmark left as a paragraph is lifted to the + end of the document, and one it ate as a link reference definition is drawn as + the marker it was rather than as a link to the note's words. +5. ✅ **Ticking a box.** `MarkdownList.Item.line` and `MarkdownTask.toggling`. For + a `.md` file the tick lands in `OpenDocument` as an unsaved edit — ⌘S writes + it, the way every other edit in the app is written. For a PR it goes back + through `updateDescription` / `updateComment`, from what the host holds rather + than from what is drawn. +6. ✅ **The rest of the list.** Table alignment, the outline behind a button in + the corner, the copy button on hover, and a picture that opens at full size. + +## What it cost, and what it did not + +- **The parse is cached exactly as it was.** `MarkdownText` is drawn on every pass + of a pull request view, so `cachedBlocks` and `cachedInlineText` both survived — + the `MarkdownLinks` of the document joined the key, since the same `#123` is + another repository's pull request. A tree costs 2–7 ms on this repository's own + documents, and it is built once per (text, folder, repository). +- **Comments are dropped from the tree, not from the text.** `strippingComments` + is gone. It cut whole lines out before parsing, which moved every line number + after them — and a source range is the one thing every write-back depends on. +- **The blocks still carry Markdown strings**, so the views, `MarkdownPDF`, + `MarkdownCodeHighlighter`, the palettes, the mermaid and draw.io fences all + stayed. The price is that everything the parser writes into a block has to be + escaped for the second read, which is why `MarkdownInline` owns both jobs. +- Both packages are ordinary SwiftPM dependencies with no asset catalogue and + nothing fetched at runtime, so the offline rule in `CLAUDE.md` holds. diff --git a/Package.resolved b/Package.resolved index c5dcaeb..88a57e9 100644 --- a/Package.resolved +++ b/Package.resolved @@ -1,5 +1,5 @@ { - "originHash" : "238ea5e32729f9074cf8bf102b537b6f1abe8407e272f2c2cbe007282717556c", + "originHash" : "6af3bb23156d57dab34dbeae805bee5287cdf3aaaa96e7855f2a3b975cfc52f9", "pins" : [ { "identity" : "codeeditsourceeditor", @@ -55,6 +55,15 @@ "version" : "2.1.1" } }, + { + "identity" : "swift-cmark", + "kind" : "remoteSourceControl", + "location" : "https://github.com/swiftlang/swift-cmark.git", + "state" : { + "revision" : "924936d0427cb25a61169739a7660230bffa6ea6", + "version" : "0.8.0" + } + }, { "identity" : "swift-collections", "kind" : "remoteSourceControl", @@ -64,6 +73,15 @@ "version" : "1.6.0" } }, + { + "identity" : "swift-markdown", + "kind" : "remoteSourceControl", + "location" : "https://github.com/swiftlang/swift-markdown.git", + "state" : { + "revision" : "3c6f9523da3a1ec2fd829673e472d95b8097a3b8", + "version" : "0.8.0" + } + }, { "identity" : "swiftlintplugin", "kind" : "remoteSourceControl", diff --git a/Package.swift b/Package.swift index 7733b55..fcbb20b 100644 --- a/Package.swift +++ b/Package.swift @@ -41,6 +41,17 @@ let package = Package( // internal layout can be rearranged in a patch release and the compiler // is the only thing that would notice — and only if the names change. .package(url: "https://github.com/CodeEditApp/CodeEditTextView.git", exact: "0.12.1"), + // The Markdown parser, and only a parser: an immutable markup tree over + // swift-cmark — the same cmark-gfm both hosts render a comment with, so + // the preview reads a document the way the host that served it does. + // Nothing of the renderer comes from here; see Docs/Markdown.md for why + // a library that draws as well was the wrong trade. + .package(url: "https://github.com/swiftlang/swift-markdown.git", exact: "0.8.0"), + // The C parser under it. Named here rather than left transitive for the + // same reason CodeEditTextView is: the grammar of the document is what + // decides how every `.md` file, PR description and comment comes out, so + // the version that decides it should be one this manifest states. + .package(url: "https://github.com/swiftlang/swift-cmark.git", exact: "0.8.0"), // The terminal engine. Upstream ghostty ships no reusable framework, so // this package supplies libghostty as a prebuilt universal // (arm64 + x86_64) xcframework that SwiftPM downloads and @@ -56,7 +67,8 @@ let package = Package( .product(name: "GhosttyKit", package: "libghostty-spm"), .product(name: "CodeEditLanguages", package: "CodeEditLanguages"), .product(name: "CodeEditSourceEditor", package: "CodeEditSourceEditor"), - .product(name: "CodeEditTextView", package: "CodeEditTextView") + .product(name: "CodeEditTextView", package: "CodeEditTextView"), + .product(name: "Markdown", package: "swift-markdown") ], path: "Sources/Workspace", // The diagram renderers' HTML hosts with their bundled scripts, and diff --git a/README.md b/README.md index 38ee09b..d7700be 100644 --- a/README.md +++ b/README.md @@ -127,7 +127,11 @@ missing ones, so you never have to work out which command it wanted. | Feature | What it does | | --- | --- | -| Markdown | Rendered, not raw — tables, task lists, code chips, and the pictures a README keeps beside itself — and saved as PDF with ⇧⌘E. | +| Markdown | Rendered, not raw, by the same parser both hosts use — nested lists, tables with their alignment, quotes, footnotes, `:tada:`, code chips, and the pictures a README keeps beside itself. Saved as PDF with ⇧⌘E. | +| Tick a box | Checkboxes in a `.md` file, a PR description or a comment are real: click one and it is written back. | +| Jump around | An outline button on any document with headings worth jumping between. | +| Copy a block | A fenced block copies whole on hover, without selecting around it. | +| Open a picture | Click a screenshot to see it at full size instead of squinting at the capped one. | | Mermaid | ` ```mermaid ` fences drawn as diagrams. | | draw.io | `.drawio` files drawn as the diagram, with pages, zoom and layers. | | PDF | Read page by page, with a floating page and zoom bar. | @@ -202,6 +206,9 @@ Workspace is mostly other people's excellent work, joined together: SwiftPM by [Lakr233](https://github.com/Lakr233/libghostty-spm). - **[tree-sitter](https://github.com/tree-sitter/tree-sitter)** by Max Brunsfeld and every grammar author — the syntax colours everywhere in the app. +- **[swift-markdown](https://github.com/swiftlang/swift-markdown)** by Apple and + the Swift community — the Markdown parser, over cmark-gfm, which is what both + hosts render a comment with. - **[mermaid](https://github.com/mermaid-js/mermaid)** by Knut Sveidqvist and contributors — the diagrams in Markdown. - **[draw.io](https://github.com/jgraph/drawio)** by JGraph — the `.drawio` viewer. @@ -248,7 +255,8 @@ Scripts/make-icon.sh # rebuild Resources/AppIcon.icns from Assets/icon.png Background reading lives in [`Docs/`](Docs/) — [Development.md](Docs/Development.md) for how the project is built and released, [LSP.md](Docs/LSP.md) for the language -server layer, [Terminal.md](Docs/Terminal.md) for the terminal. +server layer, [Terminal.md](Docs/Terminal.md) for the terminal, and +[Markdown.md](Docs/Markdown.md) for how the Markdown renderer is put together. Found a bug or want something? [Open an issue](https://github.com/necessarylion/workspace/issues) — screenshots diff --git a/Sources/Workspace/Editor/EditorThemeBridge.swift b/Sources/Workspace/Editor/EditorThemeBridge.swift index a15f46d..7477231 100644 --- a/Sources/Workspace/Editor/EditorThemeBridge.swift +++ b/Sources/Workspace/Editor/EditorThemeBridge.swift @@ -20,6 +20,26 @@ extension SyntaxTheme { /// and ``MarkdownCodeHighlighter`` keep colouring at full detail through /// ``TreeSitterHighlighter``. The editor is now the coarsest view of a file /// in the app, not the finest. + /// + /// **Markdown is the extreme of that**, and worth writing down so it is not + /// investigated twice: its grammar captures nothing *but* names on the + /// unrecognised list — `text.title`, `text.literal`, `text.uri`, + /// `text.reference`, `punctuation.special`, `punctuation.delimiter`, + /// `string.escape` — so every range is dropped and a `.md` file in the editor + /// paints in one flat colour. It is not broken, it is this list; and the + /// preview (⌘-toggled, and the way a `.md` file is meant to be read here) + /// does not go through any of this — `MarkdownParser` reads the document and + /// `MarkdownText` draws it, with ``TreeSitterHighlighter`` reaching only as + /// far as the snippet inside a fence. + /// + /// Fixing it would mean rewriting the vendored markdown queries onto the + /// names the package knows, which loses the bold heading and the italic + /// emphasis — no slot carries them — or giving the editor a + /// `highlightProviders` of our own. There is also a second break under it, + /// so neither is a one-line change: the vendored injection query names the + /// inline grammar `markdown_inline`, `TreeSitterLanguage`'s raw value is + /// `markdownInline`, and the layer is dropped for want of the match, so + /// `**bold**` and `*italic*` are not even parsed today. var editorTheme: EditorTheme { EditorTheme( text: attribute(for: nil), diff --git a/Sources/Workspace/Models/LanguageServerCatalog.swift b/Sources/Workspace/Models/LanguageServerCatalog.swift index 8fe0b13..2b24b67 100644 --- a/Sources/Workspace/Models/LanguageServerCatalog.swift +++ b/Sources/Workspace/Models/LanguageServerCatalog.swift @@ -446,6 +446,24 @@ final class LanguageServerCatalog { languageID: "css", installCommand: "npm install -g vscode-langservers-extracted" ), + .init( + // The one markdown server worth starting in an editor: it answers + // about the *document*, not the prose. Heading and link completion, + // go-to-definition across `[](other.md)` and `[[wiki]]` links, + // references on a heading, and a diagnostic for a link that points + // nowhere — which is the thing a docs folder actually gets wrong. + // + // A single self-contained binary with no runtime behind it, so it + // starts in the time gopls does; the grammar checkers that also call + // themselves markdown servers (`ltex-ls`, `harper-ls`) do a + // different job and the JVM one is far slower than anything else + // here. + language: TreeSitterLanguage.markdown.rawValue, + executable: "marksman", + command: "marksman server", + languageID: "markdown", + installCommand: "brew install marksman" + ), .init( language: TreeSitterLanguage.bash.rawValue, executable: "bash-language-server", diff --git a/Sources/Workspace/Support/MarkdownHTML.swift b/Sources/Workspace/Support/MarkdownHTML.swift index db217b1..7388b4d 100644 --- a/Sources/Workspace/Support/MarkdownHTML.swift +++ b/Sources/Workspace/Support/MarkdownHTML.swift @@ -42,12 +42,8 @@ enum MarkdownHTML { return "\(inline(text))" case .paragraph(let text): return "

    \(inline(text))

    " - case .bullet(let indent, let text): - return item(marker: indent > 0 ? "◦" : "•", text: inline(text), indent: indent) - case .numbered(let indent, let number, let text): - return item(marker: "\(escape(number)).", text: inline(text), indent: indent) - case .task(let done, let text): - return item(marker: done ? "☑" : "☐", text: inline(text), indent: 0) + case .list(let list): + return html(for: list, depth: 0) case .quote(let alert, let blocks): let head = alert.map { "

    \(escape($0.title))

    " } ?? "" return "
    \(head)\(blocks.map(html(for:)).joined(separator: "\n"))
    " @@ -81,10 +77,14 @@ enum MarkdownHTML { // stylesheet's `max-width: 100%` still keeps it inside the sheet. let style = width.map { " style=\"width: \(Int($0))px\"" } ?? "" return "

    \"\(escape(alt))\"

    " - case .table(let headers, let rows): - let head = headers.map { "
\(inline($0))
\(inline($0))
@@ -94,10 +94,43 @@ enum MarkdownHTML { """ case .rule: return "
" - case .spacer: - // Blank lines are what separates the blocks, and the spacing is in - // the stylesheet. - return "" + } + } + + /// One list, drawn the way the preview draws it: the marker in a column of + /// its own, and everything nested under an item indented by that column. + /// + /// Written out by hand rather than as `
    `/`
      `, because the two have to + /// agree — the page and the PDF walk the same blocks on purpose, and a + /// browser's own list styling would not line up with `MarkdownListView`. + private static func html(for list: MarkdownList, depth: Int) -> String { + list.items.enumerated().map { index, item in + let marker: String + if let isDone = item.isDone { + marker = isDone ? "☑" : "☐" + } else if list.isOrdered { + marker = "\(list.start + index)." + } else { + marker = depth == 0 ? "•" : (depth == 1 ? "◦" : "▪") + } + let body = item.blocks.map { block in + if case .list(let nested) = block { return html(for: nested, depth: depth + 1) } + return html(for: block) + }.joined(separator: "\n") + return """ +
      \(marker)
      \(body)
      + """ + }.joined(separator: "\n") + } + + /// The `|:---:|` a column was written with, as the one thing CSS needs to + /// say it. + private static func style(_ alignments: [MarkdownColumn], _ column: Int) -> String { + guard column < alignments.count else { return "" } + switch alignments[column] { + case .leading: return "" + case .center: return " style=\"text-align: center\"" + case .trailing: return " style=\"text-align: right\"" } } @@ -118,15 +151,6 @@ enum MarkdownHTML { return "data:image/png;base64,\(png.base64EncodedString())" } - /// A list row: the marker in a column of its own, so a wrapped line lines up - /// under the text rather than under the bullet. - private static func item(marker: String, text: String, indent: Int) -> String { - let inset = indent * 18 - return """ -
      \(marker)\(text)
      - """ - } - // MARK: - Inline /// Bold, italic, `code`, strikethrough and links, read off the same @@ -216,8 +240,13 @@ enum MarkdownHTML { .details { margin: 0 0 8pt 0; padding-left: 10pt; border-left: 1px solid #e2e2e6; } .details .summary { font-weight: 600; margin-bottom: 5pt; } .details > :last-child { margin-bottom: 0; } - .item { display: flex; gap: 7px; align-items: baseline; } + .item { display: flex; gap: 7px; align-items: baseline; margin: 0 0 3pt 0; } .item .marker { color: #77777f; flex: none; } + /* The item's own column: a wrapped line lines up under the text rather + than under the bullet, and a sub-list is indented by exactly the width + of its parent's marker. */ + .item .itembody { flex: 1 1 auto; min-width: 0; } + .item .itembody > :last-child { margin-bottom: 0; } hr { border: none; border-top: 1px solid #e2e2e6; margin: 12pt 0; } table { border-collapse: collapse; diff --git a/Sources/Workspace/Support/MarkdownHTMLText.swift b/Sources/Workspace/Support/MarkdownHTMLText.swift index bcc0b4c..ab57f55 100644 --- a/Sources/Workspace/Support/MarkdownHTMLText.swift +++ b/Sources/Workspace/Support/MarkdownHTMLText.swift @@ -5,175 +5,249 @@ import Foundation /// GitHub and Bitbucket both render a comment as Markdown *with HTML in it*, and /// a bot leans on that hard: CodeRabbit writes its whole review as nested /// `
      ` sections, hides its bookkeeping in ``, and reaches for -/// `
      ` inside table cells. None of that is Markdown, so the block parser used -/// to lay it out as literal angle brackets. +/// `
      ` inside table cells. None of that is Markdown, so cmark hands it over +/// untouched — an `HTMLBlock` with the literal string in it, or an `InlineHTML` +/// in the middle of a paragraph — and this is what reads it. /// /// Nothing here is a real HTML parser — it is the handful of tags a comment /// actually uses. Anything else is dropped rather than shown, which is what a /// reader expects from a tag their viewer does not know. +/// +/// The shape of the job changed when `MarkdownParser` took over the document: +/// cmark splits `
      ` into **three siblings** — the open tag, the inner +/// Markdown parsed properly as Markdown, the close tag — so this no longer owns +/// a whole section and recurses into it. It reads one raw block at a time and +/// says what happened in it (``Event``); the parser keeps the stack. enum MarkdownHTMLText { - // MARK: - Comments - - /// The text with every `` taken out, comments spanning lines - /// included. A line that was nothing but a comment goes with it: leaving the - /// empty line behind would open a gap where the reader sees no reason for one. - /// - /// Fenced code is left alone — a comment there is the point of the example. - static func strippingComments(_ text: String) -> String { - guard text.contains("", range: index.. Bool { + switch (self, other) { + case (.details, .details), (.blockquote, .blockquote): true + default: false } - result.append(kept) } - return result.joined(separator: "\n") } - // MARK: - Containers - - /// A `
      ` or `
      ` and the text either side of it. These two - /// are the tags that hold *blocks*, so they cannot be flattened into a line - /// the way an inline tag can — the parser has to recurse into them. - struct Container { - enum Tag { case details, blockquote } - var tag: Tag - /// `
      `, which GitHub shows already unfolded. - var isOpen: Bool - var before: Substring - var inner: Substring - var after: Substring + /// What one `HTMLBlock` turned out to say, in the order it said it. + enum Event { + case open(Container) + case close(Container) + /// A ``, already rewritten as Markdown. It belongs to whatever + /// `
      ` is open around it. + case summary(String) + /// A `
` read whole. Cells hold Markdown. + case table(headers: [String], rows: [[String]]) + /// Everything that was not one of the above, rewritten as the Markdown + /// that says the same thing — for the parser to read as a document of + /// its own. This is what turns a README's + /// `

` into a picture. + case markdown(String) } - /// The first container in `text` that is not nested inside another one. + /// Reads one raw HTML block into the events above. /// - /// Only tags of the same name are counted while looking for the close, so a - /// `
` inside a `
` — the shape every CodeRabbit review - /// has — does not end the section early; the recursion picks it up from the - /// inner text. A container that is never closed takes the rest of the text, - /// which is the reading a browser gives it too. - static func container(in text: String) -> Container? { - guard text.contains("<") else { return nil } - let fenced = fences(in: text) - var index = text.startIndex - var open: HTMLTag? - var innerStart = text.startIndex - var depth = 0 + /// A comment goes nowhere: `` is dropped here rather than cut out + /// of the source beforehand, which is what keeps every source range in the + /// document pointing at the line it was written on — and a tickable + /// checkbox is a source range. + static func events(in rawHTML: String) -> [Event] { + var events: [Event] = [] + var loose = "" + var index = rawHTML.startIndex - while index < text.endIndex { - if let fence = fenced.first(where: { $0.contains(index) }) { - index = fence.upperBound + /// Whatever plain HTML has piled up since the last structural tag. + func flush() { + defer { loose = "" } + let text = markdown(from: loose) + guard !text.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty else { return } + events.append(.markdown(text)) + } + + while index < rawHTML.endIndex { + // A comment, which the reader was never meant to see. + if rawHTML[index] == "<", rawHTML[index...].hasPrefix("", range: after..` out of a `
`, handing back its text and the - /// rest of the section. The two are rarely on lines of their own — - /// `…
` on one line is the usual shape — so - /// what is left either side is stitched back together. - static func summary(in text: Substring) -> (summary: String, body: String)? { - var index = text.startIndex - var opening: HTMLTag? + /// The text between an opening tag and the one that closes it, counting + /// tags of the same name so a nested one does not end it early, plus the + /// index just past the close. A tag that is never closed takes the rest, + /// which is the reading a browser gives it too. + private static func contents( + of name: String, + after opening: HTMLTag, + in text: String + ) -> (inner: String, end: String.Index) { + var index = opening.range.upperBound + var depth = 1 while index < text.endIndex { - guard text[index] == "<", let tag = tag(at: index, in: text) else { + guard text[index] == "<", let found = tag(at: index, in: text[...]) else { index = text.index(after: index) continue } - index = tag.range.upperBound - // A section of its own starts before this one ever said what it - // was: the summary further down belongs to that one, not to us. - if opening == nil, !tag.isClosing, tag.name == "details" { return nil } - guard tag.name == "summary" else { continue } - if let start = opening, tag.isClosing { - return ( - summary: String(text[start.range.upperBound..` read into the same headers and rows a `|…|` table gives. + /// + /// The header is whichever row is written with `
` carries nothing and would draw as a blank + // stripe — but dropping one renumbers every row after it, so where the + // header sits is corrected in the same breath. Read out of the old + // numbering afterwards it is off by the empties above it, and a table + // whose only rows are an empty one and a header takes the array out of + // range altogether. + let named = headerRow ?? 0 + guard named < rows.count else { return nil } + let position = named - rows[..`, ``, a stray `

` — is dropped and - /// its text kept. + /// What one `InlineHTML` node — a tag in the middle of a sentence — turns + /// into. + enum Inline { + /// The Markdown that says the same thing, `**` for `` and so on. + case markdown(String) + /// An ``, which is a block of its own in the preview rather than + /// something a line of text can hold. + case image(source: String, alt: String, width: CGFloat?) + /// A tag with no meaning here: dropped, its text kept. + case nothing + } + + /// One inline tag, read. `links` carries the addresses of the `` tags + /// still open, so the closing tag writes the address of the one it belongs + /// to; the caller keeps it across a paragraph's nodes. + static func inline(_ rawHTML: String, links: inout [String]) -> Inline { + // Bookkeeping a bot hides mid-sentence. + if rawHTML.hasPrefix("` goes first, so nothing - // downstream has to know it was ever there. - let parsed = parse(MarkdownHTMLText.strippingComments(text), depth: 0) + static func blocks(in text: String, relativeTo base: URL? = nil, links: MarkdownLinks = .none) -> [Block] { + let parsed = MarkdownParser.blocks(in: text, links: links) guard let base else { return parsed } return resolvingImages(parsed, relativeTo: base) } + /// The headings of a document, for the outline: the level, the words with + /// their styling taken off, and the index of the block they came from — + /// which is what ``headingID(_:)`` names and the scroll view scrolls to. + @MainActor + static func outline(of blocks: [Block]) -> [(index: Int, level: Int, title: String)] { + blocks.enumerated().compactMap { index, block in + guard case .heading(let level, let text) = block else { return nil } + let plain = (try? AttributedString( + markdown: text, + options: .init(interpretedSyntax: .inlineOnlyPreservingWhitespace) + )).map { String($0.characters) } ?? text + let title = plain.trimmingCharacters(in: .whitespaces) + return title.isEmpty ? nil : (index, level, title) + } + } + /// Turns `![](assets/preview.jpeg)` into the file it means. /// /// A README is written to be read beside its own folder, and the pictures in @@ -565,7 +742,8 @@ struct MarkdownText: View { case .image(let address, let alt, let width): guard let file = localFile(for: address, relativeTo: base) else { return block } return .image(url: file.absoluteString, alt: alt, width: width) - // A picture inside a quote or a folded section is still a picture. + // A picture inside a quote, a folded section or a list item is + // still a picture. case .quote(let alert, let nested): return .quote(alert, resolvingImages(nested, relativeTo: base)) case .disclosure(let summary, let isOpen, let nested): @@ -574,6 +752,13 @@ struct MarkdownText: View { isOpen: isOpen, blocks: resolvingImages(nested, relativeTo: base) ) + case .list(var list): + list.items = list.items.map { item in + var resolved = item + resolved.blocks = resolvingImages(item.blocks, relativeTo: base) + return resolved + } + return .list(list) default: return block } @@ -633,323 +818,297 @@ struct MarkdownText: View { switch block { case .mermaid: true case .quote(_, let nested), .disclosure(_, _, let nested): containsDiagram(nested) + case .list(let list): list.items.contains { containsDiagram($0.blocks) } default: false } } } +} - /// Peels off the `

` and `
` sections, outermost first, - /// and hands what is between them to the line parser. A section's own - /// contents come back through here, which is what lets a review nested four - /// deep read as four foldable sections rather than as its tags. +// MARK: - Environment + +/// How deep in a list the text being drawn sits. It travels in the environment +/// rather than as a parameter because a sub-list is drawn by a nested +/// `MarkdownText` that was handed blocks, not a depth — and the same is true of +/// a list inside a quote or a `
` section. +private struct MarkdownListDepthKey: EnvironmentKey { + static let defaultValue = 0 +} + +/// Where a `#123` and an `@name` in the Markdown being drawn point. Nothing by +/// default: a release note has no repository behind it, and a `#123` that +/// cannot be resolved is better left as the words it was written as. +private struct MarkdownLinksKey: EnvironmentKey { + static let defaultValue = MarkdownLinks.none +} + +/// What ticking a checkbox does, and what it writes to. +/// +/// `perform` takes the 1-based source line the item was written on and the state +/// it should end up in. `target` is what those lines belong to — a file's path, +/// a comment's id — and it is the whole of this type's equality. +/// +/// That equality is the point of the type existing. A closure is never equal to +/// another closure, so an action rebuilt on each pass reads as a change to +/// everything under it: a page of pull request comments would rebuild all of its +/// text on any redraw at all, which is exactly how it came to stutter under a +/// scroll. Two actions writing to the same place are the same action, whatever +/// closure was made this time round. +struct MarkdownTaskToggle: Equatable { + let target: String + /// The text this action was built against. /// - /// `depth` is only a backstop against text that nests without end; real - /// output from a bot bottoms out around five. - private static func parse(_ text: String, depth: Int) -> [Block] { - guard depth < 12, let container = MarkdownHTMLText.container(in: text) else { - return tidied(plainBlocks(in: text, depth: depth)) - } - var result = plainBlocks(in: String(container.before), depth: depth) - - switch container.tag { - case .details: - let split = MarkdownHTMLText.summary(in: container.inner) - let summary = MarkdownHTMLText.markdown(from: split?.summary ?? "") - .trimmingCharacters(in: .whitespacesAndNewlines) - result.append(.disclosure( - // A section with no `` still needs something to click. - summary: summary.isEmpty ? "Details" : summary, - isOpen: container.isOpen, - blocks: parse(split?.body ?? String(container.inner), depth: depth + 1) - )) - case .blockquote: - result.append(.quote(nil, parse(String(container.inner), depth: depth + 1))) - } + /// It is here because `target` alone is *too* stable. An action that reads + /// its source when it runs would need nothing else — but a comment's does + /// not: the words it flips are a `String` captured when the closure was + /// made. Equal on `target` alone, the first action would be kept for as + /// long as the comment kept its id, so a second tick would be computed + /// against the body from before the first one and posted back over it. + /// Comparing the content is what retires the stale closure, and it costs + /// nothing in the ordinary case: the same string, so `==` answers on the + /// pointer. + let content: String + let perform: (Int, Bool) -> Void + + static func == (lhs: Self, rhs: Self) -> Bool { + lhs.target == rhs.target && lhs.content == rhs.content + } +} - result += parse(String(container.after), depth: depth) - return tidied(result) +/// Nothing by default: a release note and a `
` section from a bot have +/// checkboxes nobody can write back to, and a box that cannot be ticked should +/// not look as though it can. +struct MarkdownTaskToggleKey: EnvironmentKey { + // `nil` is as shared-mutable as this gets, but the type it is nil *of* + // holds a closure, and a closure is not `Sendable`. The environment is read + // on the main actor and nowhere else. + nonisolated(unsafe) static let defaultValue: MarkdownTaskToggle? = nil +} + +extension EnvironmentValues { + var markdownListDepth: Int { + get { self[MarkdownListDepthKey.self] } + set { self[MarkdownListDepthKey.self] = newValue } } - /// A run of blank lines is one paragraph break, and the blank lines a - /// section opens and closes with are nothing at all — the stack's own - /// spacing is what holds the blocks apart. - /// - /// This matters here in a way it does not in a hand-written document: HTML - /// needs a blank line either side of a tag for the Markdown inside it to be - /// read as Markdown, so a bot writing `
` sections leaves two or - /// three, and each one used to push the next section further down. - private static func tidied(_ blocks: [Block]) -> [Block] { - var result: [Block] = [] - for block in blocks { - guard case .spacer = block else { - result.append(block) - continue - } - if let last = result.last, case .spacer = last { continue } - if result.isEmpty { continue } - result.append(block) - } - if let last = result.last, case .spacer = last { result.removeLast() } - return result + var markdownLinks: MarkdownLinks { + get { self[MarkdownLinksKey.self] } + set { self[MarkdownLinksKey.self] = newValue } } - private static func plainBlocks(in text: String, depth: Int) -> [Block] { - var result: [Block] = [] - var codeBuffer: [String] = [] - var inCode = false - /// The word after the opening ``` — "swift", "mermaid", or nothing. - var codeLanguage = "" - var tableBuffer: [[String]] = [] - var quoteBuffer: [String] = [] - - /// A quote is parsed as a document of its own rather than line by line: - /// it can hold anything, a `
` section included, and a bot's - /// alert is several paragraphs behind one bar. - func flushQuote() { - guard !quoteBuffer.isEmpty else { return } - var lines = quoteBuffer - quoteBuffer.removeAll() - var alert: Alert? - if let first = lines.firstIndex(where: { !$0.trimmingCharacters(in: .whitespaces).isEmpty }), - let marked = Alert(marker: lines[first].trimmingCharacters(in: .whitespaces)) { - alert = marked - lines.remove(at: first) - } - result.append(.quote(alert, parse(lines.joined(separator: "\n"), depth: depth + 1))) - } + var markdownTaskToggle: MarkdownTaskToggle? { + get { self[MarkdownTaskToggleKey.self] } + set { self[MarkdownTaskToggleKey.self] = newValue } + } +} - /// A fence is a diagram when it says so and has something in it; - /// everything else stays a code block. - func flushCode() { - let body = codeBuffer.joined(separator: "\n") - let language = codeLanguage - codeBuffer.removeAll() - codeLanguage = "" - let hasContent = !body.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty - if language.lowercased() == "mermaid", hasContent { - result.append(.mermaid(body)) - } else { - result.append(.code(language: language, text: body)) - } - } +/// Names a block so the outline can scroll to it, and leaves it alone otherwise. +private struct HeadingAnchor: ViewModifier { + let id: String? - func flushTable() { - guard !tableBuffer.isEmpty else { return } - defer { tableBuffer.removeAll() } - // A lone |…| line is not a table, just a paragraph with pipes. - guard tableBuffer.count >= 2 else { - result.append(.paragraph(tableBuffer[0].joined(separator: " | "))) - return - } - let headers = tableBuffer[0] - var rows = Array(tableBuffer.dropFirst()) - // Drop the |---|---| separator row if present. - if let first = rows.first, first.allSatisfy({ cell in - !cell.isEmpty && cell.allSatisfy { "-: ".contains($0) } - }) { - rows.removeFirst() - } - // Pad short rows so every GridRow has the same number of cells. - rows = rows.map { row in - row + Array(repeating: "", count: max(0, headers.count - row.count)) - } - result.append(.table(headers: headers, rows: rows)) + func body(content: Content) -> some View { + if let id { + content.id(id) + } else { + content } + } +} - for rawLine in text.split(separator: "\n", omittingEmptySubsequences: false) { - // Whatever HTML the line still carries becomes the Markdown that - // says the same thing, so everything below reads one syntax. - var line = inCode ? String(rawLine) : MarkdownHTMLText.markdown(from: String(rawLine)) - var trimmed = line.trimmingCharacters(in: .whitespaces) - - // The quote comes off before the pictures are pulled out: its lines - // are re-parsed as a document, and one that went through here first - // would have its pictures hoisted out of the quote and left after it. - if !inCode, trimmed.hasPrefix(">") { - flushTable() - var rest = Substring(trimmed).dropFirst() - if rest.hasPrefix(" ") { rest = rest.dropFirst() } - quoteBuffer.append(String(rest)) - continue - } - flushQuote() - - // Pictures come out of the line before anything else looks at it, so - // an image sitting on a bullet or at the end of a sentence still - // becomes a picture and the words around it still read as words. - // `AttributedString` would otherwise swallow the whole `![…](…)` - // and draw nothing at all. - var images: [Picture] = [] - if !inCode, line.contains("![") { - let split = splitImages(from: line) - line = split.text - trimmed = line.trimmingCharacters(in: .whitespaces) - images = split.images - } - defer { - for image in images { - result.append(.image(url: image.url, alt: image.alt, width: image.width)) - } - } - // A line that was nothing but pictures leaves no text behind, and an - // empty line here would only add a gap above them. - let isImageOnly = !images.isEmpty && trimmed.isEmpty +// MARK: - Lists - if !inCode, trimmed.hasPrefix("|"), trimmed.dropFirst().contains("|") { - tableBuffer.append(tableCells(trimmed)) - continue - } - flushTable() +/// One list, drawn as its items — each of which is a document of its own. +/// +/// The nesting is real: a sub-list, a second paragraph and a fenced block under +/// a bullet are blocks of the item, so they are drawn by a nested `MarkdownText` +/// inside the item's own column. That is what lines a wrapped line up under the +/// text rather than under the bullet, and what indents a sub-list by exactly the +/// width of its parent's marker. +private struct MarkdownListView: View { + let list: MarkdownList + /// Read here rather than in `MarkdownText`, so that every paragraph in the + /// document does not take a dependency on two values only a list uses. + @Environment(\.markdownListDepth) private var depth + @Environment(\.markdownTaskToggle) private var onToggle - if trimmed.hasPrefix("```") { - if inCode { - flushCode() - } else { - codeLanguage = String(trimmed.dropFirst(3)).trimmingCharacters(in: .whitespaces) + var body: some View { + VStack(alignment: .leading, spacing: 6) { + ForEach(Array(list.items.enumerated()), id: \.offset) { index, item in + HStack(alignment: .firstTextBaseline, spacing: 8) { + marker(for: item, at: index) + content(of: item) + // No strikethrough on a ticked item: the box already + // says it is done, and struck-through text is the + // harder to read the more there is of it. + .foregroundStyle(item.isDone == true ? .secondary : .primary) } - inCode.toggle() - continue - } - - if inCode { - codeBuffer.append(line) - continue + .environment(\.markdownListDepth, depth + 1) } + } + } - // Two spaces (or one tab) of leading indentation = one list level. - let leading = line.prefix { $0 == " " || $0 == "\t" } - let indent = leading.reduce(0) { $0 + ($1 == "\t" ? 2 : 1) } / 2 - - if isImageOnly { - // The pictures are all this line had; `defer` appends them. - } else if trimmed.isEmpty { - result.append(.spacer) - } else if trimmed == "---" || trimmed == "***" { - result.append(.rule) - } else if trimmed.hasPrefix("#") { - let level = trimmed.prefix(while: { $0 == "#" }).count - let title = trimmed.dropFirst(level).trimmingCharacters(in: .whitespaces) - result.append(.heading(level: min(level, 6), text: title)) - } else if trimmed.hasPrefix("- ") || trimmed.hasPrefix("* ") || trimmed.hasPrefix("+ ") { - let rest = String(trimmed.dropFirst(2)) - // Trimmed: a bot writes its checkbox id in a comment between the - // box and the label, and taking the comment out leaves the gap. - if rest.hasPrefix("[ ] ") { - result.append(.task(done: false, text: rest.dropFirst(4).trimmingCharacters(in: .whitespaces))) - } else if rest.hasPrefix("[x] ") || rest.hasPrefix("[X] ") { - result.append(.task(done: true, text: rest.dropFirst(4).trimmingCharacters(in: .whitespaces))) - } else { - result.append(.bullet(indent: indent, text: rest)) - } - } else if let ordered = orderedItem(trimmed) { - result.append(.numbered(indent: indent, number: ordered.number, text: ordered.text)) - } else { - result.append(.paragraph(trimmed)) - } + /// Nearly every item is one paragraph, and that one is drawn as the line it + /// is. The nested `MarkdownText` is what carries a sub-list, a second + /// paragraph or a fence — and it is a whole stack, a `ForEach` and a round + /// of environment reads, so a long checklist is worth not paying it for. + @ViewBuilder + private func content(of item: MarkdownList.Item) -> some View { + if item.blocks.count == 1, case .paragraph(let text) = item.blocks[0] { + MarkdownText.inline(text) + } else { + MarkdownText(blocks: item.blocks) } + } - flushQuote() - flushTable() - // An unterminated fence — the file is still being typed, most likely. - if inCode, !codeBuffer.isEmpty { - flushCode() + @ViewBuilder + private func marker(for item: MarkdownList.Item, at index: Int) -> some View { + if let isDone = item.isDone { + MarkdownCheckbox( + isDone: isDone, + // A box with no source line behind it — one that came out of a + // bot's HTML — is drawn but not offered. + onToggle: item.line.flatMap { line in + onToggle.map { toggle in { toggle.perform(line, !isDone) } } + } + ) + } else if list.isOrdered { + Text("\(list.start + index).") + .foregroundStyle(.secondary) + .monospacedDigit() + } else { + Text(Self.bullet(at: depth)) + .foregroundStyle(.secondary) } - return result } - /// One picture pulled out of a line. - private struct Picture { - var url: String - var alt: String - var width: CGFloat? + /// A different mark at each level, the way a printed list distinguishes + /// them — the indent alone is easy to lose in a long item. + private static func bullet(at depth: Int) -> String { + switch depth { + case 0: "•" + case 1: "◦" + default: "▪" + } } +} - /// Pulls every `![alt](url)` out of one line, handing back what is left of - /// the line and the pictures in the order they appeared. - /// - /// Bitbucket writes an attribute list after the image — - /// `![](…png){: data-layout='center' }` — which is not Markdown any parser - /// here knows; it is swallowed along with the image rather than left behind - /// as stray braces in the middle of a sentence. `MarkdownHTMLText` writes - /// the same list to carry the one attribute that *is* read: the width an - /// `` asked for. - private static func splitImages(from line: String) -> (text: String, images: [Picture]) { - var text = "" - var images: [Picture] = [] - var index = line.startIndex - - while index < line.endIndex { - guard line[index] == "!", - let bracket = line.index(index, offsetBy: 1, limitedBy: line.endIndex), - bracket < line.endIndex, line[bracket] == "[", - let altEnd = line[bracket...].firstIndex(of: "]"), - let open = line.index(altEnd, offsetBy: 1, limitedBy: line.endIndex), - open < line.endIndex, line[open] == "(", - // A closing paren inside the address would end it early, but - // an address with one in it is rare enough not to trade the - // simplicity for. - let close = line[open...].firstIndex(of: ")") - else { - text.append(line[index]) - index = line.index(after: index) - continue - } +/// The box itself. A button when there is somewhere to write the answer, and +/// otherwise the picture of one. +private struct MarkdownCheckbox: View { + let isDone: Bool + let onToggle: (() -> Void)? - let alt = String(line[line.index(after: bracket).. CGFloat? { - guard let key = attributes.range(of: "width=") else { return nil } - let digits = attributes[key.upperBound...].prefix { $0.isNumber } - guard let number = Int(digits), number > 0 else { return nil } - return CGFloat(number) +// MARK: - Code + +/// A fenced block, with the one thing people do to a fenced block on it. +/// +/// Everything in the preview is selectable, but a code block is the one thing +/// that is copied *whole* — and selecting exactly it, with no line of the prose +/// either side, is fiddly in a wall of text. +private struct MarkdownCodeBlock: View { + let code: String + let text: Text + + var body: some View { + ScrollView(.horizontal) { + // Coloured by the editor's own tree-sitter setup when the fence + // names a language we have a grammar for, plain when it does not. + text + .font(.system(.callout, design: .monospaced)) + .frame(maxWidth: .infinity, alignment: .leading) + .padding(10) + // Room for the button, so the last words of a long first line + // do not run under it. + .padding(.trailing, 28) + } + .background(.quaternary.opacity(0.4), in: RoundedRectangle(cornerRadius: 6)) + // The fill alone is faint against a dark viewer; the outline is + // what actually marks where the block starts and stops. + .overlay(RoundedRectangle(cornerRadius: 6).stroke(.quaternary, lineWidth: 1)) + // The hover lives in the overlay, not here. Scrolling drags the whole + // document under a pointer that never moved, so every block the pointer + // crosses reports a hover — and with the state on this view, each of + // those rebuilt the fence and its coloured text. In there it rebuilds + // one button. + .overlay { MarkdownCopyButton(code: code) } } +} + +/// The copy button, and the hover that reveals it. +/// +/// Everything in the preview is selectable, but a code block is the one thing +/// that is copied *whole* — and selecting exactly it, with no line of the prose +/// either side, is fiddly in a wall of text. +private struct MarkdownCopyButton: View { + let code: String - /// "| a | b |" → ["a", "b"]. - private static func tableCells(_ line: String) -> [String] { - var inner = Substring(line) - if inner.hasPrefix("|") { inner = inner.dropFirst() } - if inner.hasSuffix("|") { inner = inner.dropLast() } - return inner - .split(separator: "|", omittingEmptySubsequences: false) - .map { $0.trimmingCharacters(in: .whitespaces) } + @State private var isHovered = false + @State private var hasCopied = false + + var body: some View { + // Shown on hover only: a page of fenced examples otherwise wears a row + // of buttons nobody asked for. It stays up while it says "Copied", so + // the word is readable after the pointer has moved on. + button + .frame(maxWidth: .infinity, maxHeight: .infinity, alignment: .topTrailing) + .contentShape(Rectangle()) + // Nothing here takes a click except the button, so a drag that + // starts on the code still selects it. + .allowsHitTesting(isHovered || hasCopied) + .onHover { isHovered = $0 } } - /// "3. text" or "3) text" → ("3", "text"); nil when not an ordered item. - private static func orderedItem(_ line: String) -> (number: String, text: String)? { - let digits = line.prefix(while: \.isNumber) - guard !digits.isEmpty, digits.count <= 4 else { return nil } - let rest = line.dropFirst(digits.count) - guard rest.hasPrefix(". ") || rest.hasPrefix(") ") else { return nil } - return (String(digits), rest.dropFirst(2).trimmingCharacters(in: .whitespaces)) + @ViewBuilder + private var button: some View { + if isHovered || hasCopied { + Button { + NSPasteboard.general.clearContents() + NSPasteboard.general.setString(code, forType: .string) + hasCopied = true + Task { + try? await Task.sleep(for: .seconds(1.5)) + hasCopied = false + } + } label: { + Image(systemName: hasCopied ? "checkmark" : "doc.on.doc") + .font(.caption) + .foregroundStyle(hasCopied ? Color.green : Color.secondary) + .padding(5) + .background(.background.opacity(0.8), in: RoundedRectangle(cornerRadius: 5)) + } + .buttonStyle(.plain) + .pointerCursor() + .help(hasCopied ? "Copied" : "Copy this block") + .padding(5) + .transition(.opacity) + } } } diff --git a/Sources/Workspace/Views/PullRequestDetailView.swift b/Sources/Workspace/Views/PullRequestDetailView.swift index 28ea1e5..8f7d34e 100644 --- a/Sources/Workspace/Views/PullRequestDetailView.swift +++ b/Sources/Workspace/Views/PullRequestDetailView.swift @@ -95,6 +95,10 @@ struct PullRequestDetailView: View { // transaction is the one that ended the pull request, so a build tick, // a keystroke or a comment landing still redraws with nothing animated. .animation(ViewerMotion.contentChange, value: isOpen) + // Set once for the whole page: the description, every comment and every + // reply nested under one are all this repository's Markdown, so a + // `#123` in any of them is this repository's pull request. + .environment(\.markdownLinks, MarkdownLinks(remote: project.remote)) // A half-written description belongs to the pull request it was opened // on, and this one view is reused as the viewer moves between them. .onChange(of: item.id) { @@ -682,6 +686,19 @@ struct PullRequestDetailView: View { .transition(ViewerMotion.contentArrival) } else if !pr.body.isEmpty { MarkdownText(text: pr.body) + .environment( + \.markdownTaskToggle, + MarkdownTaskToggle( + target: "pr-\(pr.number)-description", + // The action below asks the host for the + // description when it runs, so nothing + // stale can be captured — but the body is + // still what it was built for, and saying + // so keeps every one of these honest. + content: pr.body, + perform: toggleDescriptionTask + ) + ) .font(.callout) .transition(ViewerMotion.contentArrival) } @@ -729,6 +746,19 @@ struct PullRequestDetailView: View { if saved { descriptionDraft = nil } } + /// Ticking a box in the description. There is no half of a description to + /// save, so the whole of it is written back — from what the host holds + /// rather than from what is drawn, for the same reason the edit box opens + /// on that: replacing a Bitbucket mention with the name beside it would + /// post the name as words and lose the mention. + private func toggleDescriptionTask(line: Int, isDone: Bool) { + Task { + let source = await store.editableDescription(project: project, pr: pr) + guard let updated = MarkdownTask.toggling(line: line, to: isDone, in: source) else { return } + _ = await store.updateDescription(updated, on: item, project: project, pr: pr) + } + } + private var diffTab: some View { // The spinner and the diff share the pane, so the one gives way to the // other rather than being cut to it. Keyed to `isLoading` alone: a diff @@ -2355,6 +2385,26 @@ struct CommentBubble: View { /// Sends the rewritten text. The box is drawn only when this is given. var onEditSubmitted: ((String) async -> Void)? + /// Ticking a box in a comment: the flip is made on the Markdown the *host* + /// holds — which on Bitbucket is not quite what is on screen, see + /// ``PullRequestComment/rawBody`` — and the whole comment is posted back. + /// The line numbers agree because a mention put back in its account-id form + /// is a replacement within a line, not a line of its own. + /// + /// Nothing at all for a comment nobody here may edit, which is most of them: + /// a bot's review, and everyone else's words. + private var taskToggle: MarkdownTaskToggle? { + guard let onEditSubmitted else { return nil } + let body = comment.editableBody + // `content` is that same body, so that ticking a second box after the + // conversation has come back from the host builds its answer on what + // the host now holds rather than on what this closure captured. + return MarkdownTaskToggle(target: "comment-\(comment.id)", content: body) { line, isDone in + guard let updated = MarkdownTask.toggling(line: line, to: isDone, in: body) else { return } + Task { await onEditSubmitted(updated) } + } + } + var body: some View { VStack(alignment: .leading, spacing: 5) { HStack(spacing: 6) { @@ -2416,6 +2466,10 @@ struct CommentBubble: View { .foregroundStyle(.tertiary) } else { MarkdownText(text: comment.body) + // A checklist in a comment is only tickable by whoever + // could edit the comment anyway — the tick *is* an edit, + // and it goes back through the same path the box does. + .environment(\.markdownTaskToggle, taskToggle) .font(.callout) } diff --git a/Sources/Workspace/Views/ViewerView.swift b/Sources/Workspace/Views/ViewerView.swift index fff2cbc..23b1c04 100644 --- a/Sources/Workspace/Views/ViewerView.swift +++ b/Sources/Workspace/Views/ViewerView.swift @@ -376,8 +376,36 @@ struct ViewerView: View { case .text: if document.isMarkdown && store.markdownPreview { // With the file's own address: a README's pictures are - // paths beside it, and this is what they are beside. - MarkdownPreview(text: document.text, baseURL: document.url) + // paths beside it, and this is what they are beside. And + // with the repository it belongs to, so a `#123` in it is + // that repository's pull request. + MarkdownPreview( + text: document.text, + baseURL: document.url, + // The repository the file is *in*, not the one selected + // in the sidebar: with more than one open, a `#123` in + // a file from another checkout would otherwise point at + // the selected repository's pull request. + links: MarkdownLinks( + remote: (store.project(containing: document.url) ?? store.selectedProject)?.remote + ), + onToggleTask: MarkdownTaskToggle( + target: document.url.path, + content: document.text + ) { line, isDone in + // The edit lands in the document, not on disk: a + // tick is an edit like any other here, so the tab + // marks it unsaved and ⌘S writes it. Auto-saving + // from a preview would be the one write in the app + // nobody asked for. + guard let updated = MarkdownTask.toggling( + line: line, + to: isDone, + in: document.text + ) else { return } + document.text = updated + } + ) } else if document.isDrawio && store.drawioPreview { DrawioPreview(xml: document.text) } else { diff --git a/TODO.md b/TODO.md index 8d4f327..bf22f54 100644 --- a/TODO.md +++ b/TODO.md @@ -1,105 +1,74 @@ -[x] Resolved comments should be collapse and show resolved status -[x] Resolve / unresolve button on a comment thread in PR detail -[x] When PR detail is opened, build status should be refresh in every 10s -[x] Full refresh button in PR details. and instead of copy PR link in tile, replace with open in browser. - ## Still missing in PR details Roughly in order of how much each one hurts. Everything below was checked against `PullRequestDetailView.swift`, `PullRequestSidebar.swift` and the `PullRequest*` models — none of it exists yet, on either host. -[ ] Mergeable / conflict state. Nothing reads `mergeable` or - `mergeStateStatus`, so Merge is always offered at full strength and a - conflict is only discovered when the host refuses. The sync badge answers a - different question ("behind the target"), not "will this merge". -[ ] Delete a comment. Reply, edit and resolve are all there; delete is not, so - a mistyped comment has to be finished on the web. -[ ] Edit the title. The description has a full Markdown editor, but the title — - the thing every list shows — can only be changed in the browser. -[ ] Draft ↔ Ready for review. `pr.isDraft` is drawn as a badge and never - toggled, so marking a PR ready needs the host. -[ ] Delete the source branch on merge. The merge sheet says outright "nothing is - deleted"; there is no option to, which is the normal end of a merge. -[ ] Remove a reviewer, and re-request a review. `addReviewers` is the only - write; somebody added by mistake stays, and a stale approval cannot be - asked for again. -[ ] Labels. Not in the model, not in the sidebar, not editable — and on most - repos they are how a PR is triaged. -[ ] Assignees. Same: `PullRequestReviewer.swift` only mentions the GitHub - assignees endpoint as a source of *candidates*, never as a field to show or - set. -[ ] Reactions on comments (👍/🎉/…). Both hosts have them; nothing here reads - or posts one, so a comment can only be answered with another comment. -[ ] Linked issues ("Closes #123"). Not parsed, not shown, and not opened — - unlike `#123` in a commit message, which already navigates. -[ ] Per-file "viewed" checkbox in the diff. `DiffFileList` selects a file but - keeps no read/unread state, so a long review loses its place on every - reload. -[ ] Suggested changes (` ```suggestion ` blocks) with an Apply button. They - render as plain fenced code today. -[ ] Reopen a closed PR. The reject sheet says it "can be reopened on GitHub" — - which is exactly where you have to go to do it. Once `state != .open` the - whole action bar disappears. -[ ] Unresolved-thread count in the summary bar. It says "N comments"; what a - reviewer actually wants before merging is how many threads are still open. -[ ] Change the target branch. Fixed at whatever the PR was opened against. +- [ ] Mergeable / conflict state. Nothing reads `mergeable` or + `mergeStateStatus`, so Merge is always offered at full strength and a + conflict is only discovered when the host refuses. The sync badge answers a + different question ("behind the target"), not "will this merge". +- [ ] Delete a comment. Reply, edit and resolve are all there; delete is not, so + a mistyped comment has to be finished on the web. +- [ ] Edit the title. The description has a full Markdown editor, but the title — + the thing every list shows — can only be changed in the browser. +- [ ] Draft ↔ Ready for review. `pr.isDraft` is drawn as a badge and never + toggled, so marking a PR ready needs the host. +- [ ] Delete the source branch on merge. The merge sheet says outright "nothing is + deleted"; there is no option to, which is the normal end of a merge. +- [ ] Remove a reviewer, and re-request a review. `addReviewers` is the only + write; somebody added by mistake stays, and a stale approval cannot be + asked for again. +- [ ] Labels. Not in the model, not in the sidebar, not editable — and on most + repos they are how a PR is triaged. +- [ ] Assignees. Same: `PullRequestReviewer.swift` only mentions the GitHub + assignees endpoint as a source of *candidates*, never as a field to show or + set. +- [ ] Reactions on comments (👍/🎉/…). Both hosts have them; nothing here reads + or posts one, so a comment can only be answered with another comment. +- [ ] Linked issues ("Closes #123"). Not parsed, not shown, and not opened — + unlike `#123` in a commit message, which already navigates. +- [ ] Per-file "viewed" checkbox in the diff. `DiffFileList` selects a file but + keeps no read/unread state, so a long review loses its place on every + reload. +- [ ] Suggested changes (` ```suggestion ` blocks) with an Apply button. They + render as plain fenced code today. +- [ ] Reopen a closed PR. The reject sheet says it "can be reopened on GitHub" — + which is exactly where you have to go to do it. Once `state != .open` the + whole action bar disappears. +- [ ] Unresolved-thread count in the summary bar. It says "N comments"; what a + reviewer actually wants before merging is how many threads are still open. +- [ ] Change the target branch. Fixed at whatever the PR was opened against. ## Still missing in Markdown preview -Checked against `Views/MarkdownPreview.swift` (the block parser and renderer) -and `Support/MarkdownHTMLText.swift`. Everything here affects `.md` files, PR -descriptions and comments alike — they all go through `MarkdownText`. - -[ ] Checklists don't work properly. Four separate things: - - **Not clickable.** `.task` draws a plain `Image(systemName:)`, so a box - can be read but never ticked — a TODO.md or a PR checklist has to be - edited by hand, or on the host. - - **Nested checklists lose their indent.** `.bullet` and `.numbered` both - carry `indent:`; `.task` does not (`MarkdownPreview.swift:173`), so - ` - [ ] sub` is drawn flush left, level with its parent. - - **A box with no text after it is not a box.** The test is - `rest.hasPrefix("[ ] ")` (line 837), so a bare `- [ ]` falls through to a - bullet and shows the literal brackets. - - **Ordered checklists are not recognised.** `1. [ ] thing` goes to - `orderedItem` and prints `[ ]` as text. -[ ] Hard-wrapped paragraphs come apart. Every source line becomes its own - `.paragraph` block (line 847), and the stack puts 8pt between blocks — so a - paragraph wrapped at 80 columns is drawn as a loose ladder of lines instead - of one flowing block. This is the biggest visual gap for real `.md` files. -[ ] Multi-line list items, same cause: a continuation line under a bullet - becomes a paragraph of its own and loses both the indent and the bullet. -[ ] Indented code blocks (four spaces). Only ``` fences are recognised, so - older READMEs render their code as ordinary prose. -[ ] Setext headings. `Title` over `=====` or `-----` — the `---` is matched as - a horizontal rule first (line 827), so it comes out paragraph + rule. -[ ] Horizontal rules only match `---` and `***` exactly. `___`, `----` and - `- - -` are all valid and all render as text. -[ ] Bare URLs are not links. Both hosts autolink `https://…` written on its - own; here only `[text](url)` and `` become links. (Inferred from what - `AttributedString(markdown:)` does — worth a quick check before fixing.) -[ ] Reference-style links. `[text][ref]` with `[ref]: https://…` further down: - the link stays literal and the definition line renders as a paragraph. -[ ] Footnotes (`[^1]`). Not parsed at all. -[ ] Table alignment. The `|:---:|` row is dropped (line 747) and every cell is - leading-aligned, so numeric columns cannot be right-aligned. -[ ] HTML tables. `MarkdownHTMLText` knows `
` and `
` as - containers and nothing else, so a bot's `
`; a table with none is + /// read with its first row as the header, because a table drawn with no + /// header band at all is not a shape the preview has. + private static func table(in html: String) -> (headers: [String], rows: [[String]])? { + var rows: [[String]] = [] + var headerRow: Int? + var index = html.startIndex + + while index < html.endIndex { + guard html[index] == "<", let found = tag(at: index, in: html[...]) else { + index = html.index(after: index) + continue + } + switch found.name { + case "tr" where !found.isClosing: + rows.append([]) + index = found.range.upperBound + case "th", "td": + guard !found.isClosing else { + index = found.range.upperBound + continue + } + if rows.isEmpty { rows.append([]) } + if found.name == "th", headerRow == nil { headerRow = rows.count - 1 } + let (inner, end) = contents(of: found.name, after: found, in: html) + rows[rows.count - 1].append( + markdown(from: inner).trimmingCharacters(in: .whitespacesAndNewlines) ) + index = end + default: + index = found.range.upperBound } - if !tag.isClosing { opening = tag } } - return nil + + // An empty `
` collapses into a run of - text with its structure gone. -[ ] Emoji shortcodes (`:tada:`, `:warning:`). Both hosts render them; here they - stay as the colon-name. -[ ] `#123` and `@name` are not links in rendered Markdown. `CommitMessageText` - already navigates `#123` in a commit subject — a PR description and a - comment, where they are far more common, do not. -[ ] A code fence or a nested list inside a list item is hoisted out of it. The - parser is line-by-line with no block nesting below quotes and `
`. -[ ] Copy button on a code block. Everything is selectable, but a fenced block - is the one thing people copy whole. -[ ] Images cannot be opened. They are capped at 420pt tall and there is no - click-to-zoom, so a wide screenshot in a PR description can only be - squinted at. -[ ] No outline for long documents. Headings are drawn but not anchored, so a - big README has no way to jump around it. +The list that used to be here was one bug written out sixteen ways: the parser +read the document a line at a time and there was no block tree. It has been +replaced by [swift-markdown](https://github.com/swiftlang/swift-markdown) — +cmark-gfm, which is what both hosts render a comment with — so hard-wrapped +paragraphs, multi-line list items, nested and ordered checklists, setext +headings, every spelling of a horizontal rule, indented code blocks, +reference-style links, table alignment and a fence inside a list item are all +answered by the tree rather than by code of ours. Bare URLs, `:tada:`, `#123`, +`@name` and footnotes are one scan over the text on top of it, checkboxes tick +and write back, fenced blocks have a copy button, pictures open at full size and +a long document has an outline. See `Docs/Markdown.md` for the reasoning and +`Support/MarkdownParser.swift` for the walk. + +Struck rather than done: **a bare `- [ ]` is not a box**. cmark-gfm wants text +after the brackets, and both hosts *are* cmark-gfm — so matching it is fidelity, +not a bug. + +What is left, and both need a real HTML parser rather than the tag scanner in +`Support/MarkdownHTMLText.swift`: + +- [ ] HTML tables are read, but only the plain shape — `
`, ``, `
`, + ``. A `colspan`, a `rowspan` or an `align=` on a cell is dropped, and a + table with a blank line inside it is split by cmark before we ever see it. + [SwiftSoup](https://github.com/scinfu/SwiftSoup) is the tool if that ever + matters. +- [ ] `` inside a table cell is drawn as its alt text. A picture is a block + of its own here, and a cell has nowhere to put one. ## Missing elsewhere in the app @@ -109,59 +78,58 @@ not against the README. ### Git -[ ] No branch switcher. Only two branches can ever be checked out: the - repository's default (the dashboard button) and a PR's source branch. There - is no list of local or remote branches, and no create, rename or delete. - `git for-each-ref` appears exactly once, in `DefaultBranch.swift:114`, and - only to guess which branch is the default. -[ ] No stash. Nothing to park half-done work on — which compounds the one - above, since switching branches is what stashing is usually for. -[ ] No blame. No way to ask who wrote a line, or why. -[ ] No history for a single file. The dashboard lists the repository's recent - commits; there is no log for the file you are reading. -[ ] No amend, revert or cherry-pick. Commit and push are there, so fixing the - commit you just made means dropping to the terminal. -[ ] No tags. Not listed, not created — so nothing in the app says what shipped. -[ ] No diff between two arbitrary refs. A PR's diff and a commit's patch are - the only two; `main...my-branch` cannot be asked for. -[ ] No submodule awareness at all. +- [ ] No branch switcher. Only two branches can ever be checked out: the + repository's default (the dashboard button) and a PR's source branch. There + is no list of local or remote branches, and no create, rename or delete. + `git for-each-ref` appears exactly once, in `DefaultBranch.swift:114`, and + only to guess which branch is the default. +- [ ] No stash. Nothing to park half-done work on — which compounds the one + above, since switching branches is what stashing is usually for. +- [ ] No blame. No way to ask who wrote a line, or why. +- [ ] No history for a single file. The dashboard lists the repository's recent + commits; there is no log for the file you are reading. +- [ ] No amend, revert or cherry-pick. Commit and push are there, so fixing the + commit you just made means dropping to the terminal. +- [ ] No tags. Not listed, not created — so nothing in the app says what shipped. +- [ ] No diff between two arbitrary refs. A PR's diff and a commit's patch are + the only two; `main...my-branch` cannot be asked for. +- [ ] No submodule awareness at all. ### The PR board -[ ] **You cannot create a pull request.** Read, review, comment, approve, merge - and reject are all there; opening one is not. The README's shortcut table - lists ⌃⌘P "Create PR with Claude Code" and nothing in the code implements - it — there is no such `ShortcutAction` case and no handler. -[ ] The board has no filter or search. `PullRequestTable.swift` sorts and never - filters — no by-author, no "needs my review", no title search. -[ ] No cross-repo review inbox. Each repository's PRs are their own list, so - there is no single view of everything waiting on you. +- [ ] **You cannot create a pull request.** Read, review, comment, approve, merge + and reject are all there; opening one is not. The README's shortcut table + lists ⌃⌘P "Create PR with Claude Code" and nothing in the code implements + it — there is no such `ShortcutAction` case and no handler. +- [ ] The board has no filter or search. `PullRequestTable.swift` sorts and never + filters — no by-author, no "needs my review", no title search. +- [ ] No cross-repo review inbox. Each repository's PRs are their own list, so + there is no single view of everything waiting on you. ### Editor and language servers -`LanguageService` covers diagnostics, completions, hover, document symbols and -go-to-definition. The rest of the protocol is unimplemented: +`LanguageService` covers diagnostics, completions, hover, document symbols and go-to-definition. The rest of the protocol is unimplemented: -[ ] Rename symbol (`textDocument/rename`). -[ ] Find references (`textDocument/references`). -[ ] Code actions / quick fixes. A diagnostic can be read but never acted on, - which is the single most-used thing an LSP offers after completion. -[ ] Format document (`textDocument/formatting`), and so no format-on-save. -[ ] Signature help — completions are there, parameter hints are not. -[ ] Workspace symbol search. ⌘P finds files; nothing finds a symbol. -[ ] Inlay hints and code lens. +- [ ] Rename symbol (`textDocument/rename`). +- [ ] Find references (`textDocument/references`). +- [ ] Code actions / quick fixes. A diagnostic can be read but never acted on, + which is the single most-used thing an LSP offers after completion. +- [ ] Format document (`textDocument/formatting`), and so no format-on-save. +- [ ] Signature help — completions are there, parameter hints are not. +- [ ] Workspace symbol search. ⌘P finds files; nothing finds a symbol. +- [ ] Inlay hints and code lens. ### Search and diffs -[ ] No replace across files. `FileSearch.swift` finds and never writes. -[ ] No diff options — no ignore-whitespace, no context-line count, no way to - hide whitespace-only changes. A reformatting commit is unreadable. +- [ ] No replace across files. `FileSearch.swift` finds and never writes. +- [ ] No diff options — no ignore-whitespace, no context-line count, no way to + hide whitespace-only changes. A reformatting commit is unreadable. ### Shortcuts -[ ] ⌘N and ⇧⌘N are not rebindable. They are `.keyboardShortcut` literals in - `WorkspaceApp.swift:45` and `:50` instead of `ShortcutAction` cases, which - is exactly what CLAUDE.md says not to do — and it makes the README's "every - shortcut the app binds is rebindable" untrue. -[ ] README shortcut table needs a pass: ⌘, and ⇧⌘O are each listed twice, and - ⌃⌘P documents a command that does not exist. +- [ ] ⌘N and ⇧⌘N are not rebindable. They are `.keyboardShortcut` literals in + `WorkspaceApp.swift:45` and `:50` instead of `ShortcutAction` cases, which + is exactly what CLAUDE.md says not to do — and it makes the README's "every + shortcut the app binds is rebindable" untrue. +- [ ] README shortcut table needs a pass: ⌘, and ⇧⌘O are each listed twice, and + ⌃⌘P documents a command that does not exist.