Skip to content

text_view: Improve inline code render with rounded style and mono font - #2949

Merged
huacnlee merged 5 commits into
longbridge:mainfrom
kossoy:feat/inline-code-mono-font-family
Sep 8, 2026
Merged

text_view: Improve inline code render with rounded style and mono font#2949
huacnlee merged 5 commits into
longbridge:mainfrom
kossoy:feat/inline-code-mono-font-family

Conversation

@kossoy

@kossoy kossoy commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Inline code currently keeps the body font while fenced code uses the theme’s mono font. This change makes inline code use cx.theme().tokens.typography.mono as well, with matching font measurements for mixed image/text wrapping and table columns.

image

The existing highlight payload carries the font family alongside HighlightStyle. A shared run builder and mark-to-highlight conversion keep rendering and measurement consistent. Oversized inline code can wrap at word boundaries, or at Unicode grapheme boundaries for oversized words.

No public font configuration is added to TextViewStyle. Existing struct literals remain compatible, and fenced code retains its existing StyleRefinement overrides.

Validation on Linux:

  • Base text:: unit tests: 111 passed.
  • Component text:: unit tests: 68 passed.
  • Component theme_compat tests: 2 passed, including the legacy TextViewStyle struct literal.
  • Clippy for Base and Component tests with warnings denied: passed.
  • Formatting of changed Rust files, diff whitespace checks, and spelling checks: passed.

Regression coverage includes a mono font with twice the body advance, theme-driven table measurement, mixed flow wrap widths, long identifiers, CJK, and combining characters. Native visual inspection was not performed; CI for the final revision is tracked separately.

@huacnlee huacnlee left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The requirement is right — inline code should render in the mono family, and consolidating the two duplicate run builders into one text_runs is a genuine cleanup. But I don't think the shape of the change is the one this needs, and there are two measurement paths that were left behind. Requesting changes on the modeling question first, since fixing it makes most of the rest go away.

Inline code should ride in highlights, not in a second parallel list

code_ranges is introduced as an independent partition of the same text, keyed to the same byte offsets, and then threaded through every layer that already carries highlights: InlineFlowItem, MeasureItem, LineFragmentKind, PositionedFragment, Inline, and both Paragraph builders. Two lists over one coordinate space now have to be sliced, cloned and cleared in lockstep at every one of those sites. That is the source of most of the +166.

It also forces text_runs to reconcile the two partitions at render time — sorting, clamping, and splitting a highlight run at a code boundary. That reconciliation is dead code for every input this pipeline can actually produce. In node.rs the same inner_range goes into both lists:

if style.code {
    highlight = highlight.highlight(node_cx.style.inline_code_highlight());
    code_ranges.push(inner_range.clone());
}
// ...
node_highlights.push((inner_range, highlight));

Every code range therefore has a highlights entry with identical bounds, and gpui::combine_highlights is an endpoint sweep that cuts at the start and end of every input range. So each code range's endpoints are always cut points, and a highlight segment arriving at text_runs is either entirely inside a code range or entirely outside it. It can never straddle one.

That makes text_runs_split_a_highlight_at_the_code_boundary a test for an input the production path cannot construct: bold 0..10 with code 6..10 reaches text_runs already split into 0..6 bold and 6..10 bold+code.

The suggestion: change the payload of the existing list from HighlightStyle to a small struct carrying the highlight plus the mono flag (HighlightStyle has no family field, which is the real constraint here — that part of the diagnosis is correct). Same files touched, one list instead of two, no splitting, no sort, no per-fragment Vec<(Range<usize>, ())> allocated in inline_flow.rs:453 just to reuse slice_ranges, and the run builder stays the original loop plus one assignment to run.font.family.

Two measurement paths still assume a single font

These are the reason I'd rather not land the current shape: the information lives on a channel that the measurement code does not subscribe to.

inline_flow.rs:573 — line breaking is measured in the body font only. line_ranges() builds one line_wrapper(text_style.font(), font_size) and wraps the whole flow with it, while layout_flow now shapes each fragment with mono runs and sums those shaped widths into line_width/max_width. Mono glyphs are generally wider, so the break points chosen for the body font yield a line whose shaped width exceeds wrap_width, and InlineFlowLayout.size reports max_width > wrap_width — horizontal overflow. Repro shape, in a narrow text view:

Text ![badge](x.png) with a fairly long `inline_code_span` at the end.

The same class of error existed for bold and italic, but the proportional-to-mono delta is much larger.

node.rs:2081 — table column widths are measured with a body-font run per cell. The max-content pass does text_style.to_run(line.len()) over the cell's plain text with no knowledge of the code ranges, but cells render through Paragraph::renderInline and are now shaped in mono. A column of `method_name()` values gets a col_w narrower than the content, so cells wrap to extra lines or clip inside CELL_PAD_PX.

Smaller points

  • The family bypasses TextViewStyle, with no opt-out. It is read straight from cx.theme().tokens.typography.mono inside the base element (inline.rs:364, inline_flow.rs:216), but TextViewStyle::inline_code is the documented seam for inline code presentation. An app that styles inline code as a badge in the body face can no longer do so, and inline code cannot be given a family distinct from fenced blocks. CLAUDE.md asks the base layer to stay visually unopinionated with presentation above the seam, so this wants an optional family on TextViewStyle defaulting to the mono token.
  • Per-frame work. text_runs re-clones, re-filters and re-sorts on every layout pass, and the push closure rescans the whole vec for each run, giving O(highlights × code_ranges) plus an allocation per call — in Inline::request_layout (per frame, per visible inline element) and once per line fragment in layout_flow. Folding the flag into the highlights payload removes this entirely.

What I verified

cargo check -p gpui-base passes, clippy is clean with --all-targets, and both new tests pass. I traced the text_runs splitting logic for length and underflow correctness and it is sound in isolation: run lengths always sum to text_len, and empty, reversed, unsorted, overlapping and out-of-range inputs are all handled. All three Inline::new call sites and both Paragraph builders were updated consistently, and the per-line slice_ranges keeps the code ranges in the same coordinate space as the sliced text and highlights. The problems are the modeling and the two measurement paths, not the run construction.


🤖 Review assisted by Claude Code

An inline code span was styled through TextViewStyle::inline_code, a
HighlightStyle, which carries no font family, so it kept the body face
while fenced blocks rendered in the mono family.

The highlight list payload is now InlineHighlight { style, font_family }
and the run builder assigns run.font.family when a highlight names one.
The family comes from TextViewStyle::inline_code_font_family, which
defaults to the theme mono token in from_theme (and the component theme
adapter), so the base element never reads the theme directly.

Both measurement paths shape with the same runs the renderer uses: the
inline-flow line wrapper receives a mono span as a fixed-width element
of its shaped width, and table column max-content measures each cell
line with its inline highlights.
@kossoy
kossoy force-pushed the feat/inline-code-mono-font-family branch from 378c544 to 314efe8 Compare September 5, 2026 10:15
@kossoy

kossoy commented Sep 5, 2026

Copy link
Copy Markdown
Contributor Author

Reworked per the review and force-pushed (314efe8, rebased on current main). Per point:

Modeling. code_ranges is gone. The highlights payload is now InlineHighlight { style: HighlightStyle, font_family: Option<SharedString> }, threaded where (Range<usize>, HighlightStyle) was: Inline, InlineFlowItem, MeasureItem, LineFragmentKind, PositionedFragment, both Paragraph builders. inline::combine_highlights is the same endpoint sweep as gpui::combine_highlights over the new payload, so every code range is cut at its endpoints and nothing straddles. text_runs is the original single loop plus run.font.family = family when the highlight names one. The splitting logic, its sort, the per-fragment Vec<(Range<usize>, ())> and text_runs_split_a_highlight_at_the_code_boundary are deleted; the replacement test (combine_highlights_cuts_a_bold_span_at_the_code_boundary) exercises the real input shape. The two duplicated mark-to-highlight loops in node.rs now share mark_highlight, and runs_for_highlights is folded into text_runs.

Measurement paths.

  • inline_flow.rs line_ranges(): a text item is emitted to the line wrapper as body-font LineFragment::text pieces around each family-bearing span, and the span itself as LineFragment::element(width, len) with width from layout_line over the same runs layout_flow shapes with. The wrapper breaks around the span at its true width.
  • node.rs table max-content: extracted to measure_table_columns; each cell line is shaped with text_runs over Paragraph::inline_highlights (the marks resolved through the same mark_highlight), so a column of code spans is measured in the code family. render_scroll_table is otherwise unchanged.

Presentation seam. TextViewStyle::inline_code_font_family: Option<SharedString> with with_inline_code_font_family/inline_code_font_family(); from_theme sets it to theme.tokens.typography.mono, from_colors (and so Default) to TypographyTokens::default().mono. None keeps inline code in the body face. The component TextViewStyle gains the same optional field (None = themed family, resolved in resolve_component_style), and the theme adapter sets it from theme.mono_font_family. Base no longer reads cx.theme() for this.

Per-frame work. No clone/filter/sort in the run builder; the flag lives on the payload.

Tests covering the two measurement paths, both against a test PlatformTextSystem (inline::test_fonts::WideMonoTextSystem) whose Mono family shapes at twice the body advance, so a body-font measurement of a mono span is short by half:

  • text::inline_flow::tests::inline_code_near_the_wrap_width_does_not_overflow_the_flow - text, image, and a code span that fills wrap_width exactly, plus a trailing word: asserts size.width <= wrap_width, the span fragment is shaped at the mono width, the span stays on line one and the trailing word wraps. Against the old body-font wrapper it fails with flow width 220px exceeds wrap width 205px.
  • text::node::tests::table_column_of_inline_code_cells_fits_the_mono_width - a one-column table of `method_name()`: asserts col_w >= mono width + CELL_PAD_PX. Against a body-font run it fails with col_w 120 must fit the mono width 208 plus padding 16.

Also text_runs_shape_a_code_highlight_in_its_font_family, combine_highlights_cuts_a_bold_span_at_the_code_boundary, and from_theme_maps_base_semantic_tokens now checks the family. cargo check -p gpui-base -p gpui-component, cargo test -p gpui-base --lib text:: (107 passed), cargo clippy --all-targets (no new warnings) and cargo fmt --check are clean.

@huacnlee huacnlee added this to the 0.7.0 milestone Sep 8, 2026
huacnlee
huacnlee previously approved these changes Sep 8, 2026

@huacnlee huacnlee left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved after the theme-font simplification and wrapping fixes; Security: PASS for the affected scope. Local tests pass; CI for this revision is still pending.

@huacnlee huacnlee removed this from the 0.7.0 milestone Sep 8, 2026
@huacnlee
huacnlee enabled auto-merge (squash) September 8, 2026 16:31
@huacnlee huacnlee changed the title text: render inline code spans in the theme's mono font text_view: Improve inline code render with rounded style and mono font Sep 8, 2026
@huacnlee
huacnlee merged commit 4833601 into longbridge:main Sep 8, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants