Render <kbd> keyboard-shortcut tags as keycap badges in the Markdown viewer#13916
Open
fbartho wants to merge 10 commits into
Open
Render <kbd> keyboard-shortcut tags as keycap badges in the Markdown viewer#13916fbartho wants to merge 10 commits into
fbartho wants to merge 10 commits into
Conversation
Add a kbd inline style flag to FormattedTextStyles and recognize <kbd>text</kbd> in both the file-viewer Markdown grammar (a delimiter pair mirroring <u>) and the paste-to-HTML path. Tags are matched case-insensitively; unmatched or unterminated tags degrade to literal text. Round-trips back to <kbd> in the table Markdown serializer. The kbd flag is kept distinct from inline_code so it can serialize back to <kbd> rather than a backtick span and later carry a differentiated keycap visual.
…rpdotdev#13733) Add a first-class BufferTextStyle::Kbd so the file Markdown viewer's piece-table markers carry the kbd style, mirroring InlineCode across TextStyles, TextStylesWithMetadata, and StyleSummary. Serialize kbd runs back to <kbd> in the local Markdown table serializer and as a <kbd> element in the HTML export and table-cell paths. The table cell offset map already skips source-only markers generically, so <kbd> tags need no special handling there; a regression test locks that in. Round-trip and HTML tests exercise the full buffer path. The random fuzz golden is refreshed since Kbd joins the style generator.
Give kbd-styled runs the same monospace font and background/border chip treatment as inline code in style_and_font, reusing inline_code_style so there is no new theming surface. Keep it a shared branch so a future differentiated keycap look can diverge without touching inline code.
…arpdotdev#13733) The GUI formatted-text element (used for AI/notebook formatted text) renders FormattedTextInline directly rather than via the editor buffer, so it needs its own kbd branch. Give kbd runs the same inline-code chip background and monospace font here too, for parity with the file Markdown viewer.
Render <kbd> keycaps as a bordered badge that is visually distinct from an inline-code chip, rather than reusing the identical chip styling. Both renderers (editor style_and_font and warpui_core formatted-text element) now split the shared inline_code/kbd styling branch: a keycap draws a visible outline in the run's foreground color, while an inline-code chip stays borderless (its border color matches its background). The foreground color contrasts with the chip background by construction, guaranteeing the outline is visible. Render tests now assert the discriminator (keycap border visible and distinct from the inline-code chip, which stays borderless); the shared styling made these fail before the split.
Give the <kbd> keycap a heavier bottom edge for a "raised key" cue, matching the GitHub/MDN keycap convention, on top of the visible border that already distinguishes it from an inline-code chip. TextBorder gains an optional bottom_width. When set, paint_run_background draws a second bottom-only border overlay thicker than the uniform border (TextBorder.width alone cannot express an asymmetric edge). The GPU paths already carry per-side border widths, so no shader work is needed; the macOS CoreText attribute round-trip carries the new field so the border survives native line-breaking. Only the kbd keycap sets bottom_width; the inline-code chip leaves it None.
) Add a guardrail test asserting every export path (markdown, HTML, plain buffer text) round-trips the authored `<kbd>Cmd</kbd>` text and never the rendered key glyph (⌘). Key-glyph substitution is a render-only concern and must never leak into the data model or serializers; this test locks that invariant in before any glyph-substitution work lands.
Nested `<kbd><kbd>Ctrl</kbd>+<kbd>N</kbd></kbd>` previously leaked literal `<kbd>`/`</kbd>` text because parse_kbd (copied from the non-nesting <u> logic) deactivated the still-open outer delimiter when the first inner close was seen. Track open-kbd nesting depth and drop inner `<kbd>`/`</kbd>` tags so the whole span renders as one flat keycap over the inner content, deterministically for any depth. The nesting is discarded at parse time, so serialization emits the canonical flat form `<kbd>Ctrl+N</kbd>` rather than the authored nested markup. Depth-aware per-key badging (outer keycap wrapping inner keycaps) is deferred to issue warpdotdev#13912. Tests cover the flat-collapse shape, deep nesting, the no-tag-leak guarantee, unmatched/unterminated edge cases, and the flat serialization.
github-actions
Bot
added
the
external-contributor
Contributor
|
I'm starting a first review of this pull request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
![oz-for-oss[bot]](https://avatars.githubusercontent.com/in/3450139?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Overview
This PR adds first-class <kbd> inline styling through Markdown parsing, buffer conversion, HTML/Markdown serialization, and both text rendering paths so keyboard shortcuts render as keycap badges instead of inline-code chips.
Concerns
- Combined
<kbd>+ underline styles can serialize with crossed tags because the Markdown serializer now opens<kbd>before<u>but closes</kbd>before</u>. - Case-insensitive
<kbd>parsing does not preserve the original matched tag text on malformed/unmatched fallbacks, so mixed-case literal tags can be silently rewritten.
Verdict
Found: 0 critical, 2 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| buf.push_str("~~"); | ||
| } | ||
|
|
||
| if start_kbd { |
Contributor
There was a problem hiding this comment.
<kbd> before <u> conflicts with the new close order, so a run that is both kbd and underline serializes as <kbd><u>…</kbd></u>. Move the kbd start after the underline start so exported tags remain properly nested.
| ) -> IResult<&'a str, InlineToken<'a>, E> { | ||
| context( | ||
| "kbd_start", | ||
| map(tag_no_case("<kbd>"), |_| InlineToken::Delimiter { |
Contributor
There was a problem hiding this comment.
tag_no_case discards the matched spelling, so malformed mixed-case tags fall back as synthesized lowercase literals (for example, <KBD>Esc becomes <kbd>Esc, and </KBD> becomes </kbd>). Preserve the matched slice for literal fallback or keep malformed tags case-sensitive.
…#13733) A run that is both kbd and underlined serialized as crossed tags (<kbd><u>…</kbd></u>) because the Markdown open order was <kbd> then <u> while the close order is </kbd> then </u>. Open <u> before <kbd> so the tags mirror the close order and nest properly (<u><kbd>…</kbd></u>), which also round-trips back to the same styles. The HTML export paths already nested correctly and are unchanged.
…arpdotdev#13733) Case-insensitively matched <kbd>/</kbd> tags that degraded to literal text were rewritten to canonical lowercase, so <KBD>Esc became <kbd>Esc and a stray </KBD> became </kbd>. tag_no_case discarded the matched slice, and the literal fallback reconstructed the tag from the canonical spelling. Thread the matched source slice through the delimiter and kbd-end tokens and echo it verbatim on the literal-text path. The well-formed case-insensitive path (<KBD>x</KBD> to keycap) is unchanged.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #13733
Summary
Renders Markdown
<kbd>keyboard-shortcut elements as a distinct keycap badge, addressing #13733. A keycap is now visually differentiated from an inline-code span, and nested<kbd>no longer leaks literal tags.What changed
Parsing & round-trip (pre-existing commits in this stack):
<kbd>/</kbd>(case-insensitive) into a dedicatedkbdinline style, kept distinct frominline_codeso it round-trips back to<kbd>rather than a backtick code span.kbdstyle through the buffer and Markdown/HTML serialization.Keycap styling (this stack):
TextBordergained an optionalbottom_width; the GPU paths already carry per-side border widths, so no shader work was needed. The macOS CoreText attribute round-trip carries the new field.style_and_fontand the warpui_core formatted-text element) apply this consistently.Nested
<kbd>flat-collapse (this stack):<kbd><kbd>Ctrl</kbd>+<kbd>N</kbd></kbd>previously leaked literal<kbd>/</kbd>text (the parser copied the non-nesting<u>logic, which deactivated the still-open outer delimiter on the first inner close). It now collapses deterministically into one flat keycap over the inner content (Ctrl+N), for any nesting depth. Serialization emits the canonical flat form<kbd>Ctrl+N</kbd>.Testing
cargo testonmarkdown_parser(156),warp_editor(478 pass; the 8render::tests::*failures are pre-existingparking_lot::once/env_loggerstatic-init poisoning, unrelated to this change),warpui_core;clippy -D warnings;fmt --check— all clean.Known limitations (deliberate scope splits)
<kbd>into a single badge. Rendering an outer keycap that wraps inner keycaps (browser/GitHub compound-shortcut markup) requires depth-aware representation through the parser, buffer (CRDT-adjacent), serializers, and both renderers — tracked separately in Markdown viewer: depth-aware nested <kbd> for compound shortcuts #13912.<kbd>Cmd</kbd>for the platform glyph ⌘ at render time is deferred. It changes character count, which desyncs the editor's offset map and the elements' selection/copy offsets (both assume rendered↔authored 1:1); doing it safely needs offset-remapping infrastructure. Tracked in Markdown viewer: platform key glyphs in <kbd> keycaps (Cmd → ⌘) #13913, with the offset-safety rationale and the rejected read-only-only approach documented there. The serialization round-trip guardrail added here (test_kbd_serialization_preserves_authored_text_not_glyph) is a forward guardrail for that work.Test document used for the screenshot