feat: introduce from_str_with_options() to support custom stylesheets #80

bolinfest · 2025-05-07T17:39:26Z

No description provided.

This introduces the use of the `tui-markdown` crate to parse an assistant message as Markdown and style it using ANSI for a better user experience. As shown in the screenshot below, it has support for syntax highlighting for _tagged_ fenced code blocks: <img width="907" alt="image" src="https://github.com/user-attachments/assets/900dc229-80bb-46e8-b1bb-efee4c70ba3c" /> That said, `tui-markdown` is not as configurable (or stylish!) as https://www.npmjs.com/package/marked-terminal, which is what we use in the TypeScript CLI. In particular: * The styles are hardcoded and `tui_markdown::from_str()` does not take any options whatsoever. It uses "bold white" for inline code style which does not stand out as much as the yellow used by `marked-terminal`: https://github.com/joshka/tui-markdown/blob/65402cbda70325f34e7ddf6fe1ec629bcd9459cf/tui-markdown/src/lib.rs#L464 I asked Codex to take a first pass at this and it came up with: joshka/tui-markdown#80 * If a fenced code block is not tagged, then it does not get highlighted. I would rather add some logic here: https://github.com/joshka/tui-markdown/blob/65402cbda70325f34e7ddf6fe1ec629bcd9459cf/tui-markdown/src/lib.rs#L262 that uses something like https://pypi.org/project/guesslang/ to examine the value of `text` and try to use the appropriate syntax highlighter. * When we have a fenced code block, we do not want to show the opening and closing triple backticks in the output. To unblock ourselves, we might want to bundle our own fork of `tui-markdown` temporarily until we figure out what the shape of the API should be and then try to upstream it.

codecov-commenter · 2025-05-07T18:24:38Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 45.27%. Comparing base (f1e7ad7) to head (ca436d1).

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #80      +/-   ##
==========================================
+ Coverage   38.22%   45.27%   +7.05%     
==========================================
  Files           5        7       +2     
  Lines         450      508      +58     
==========================================
+ Hits          172      230      +58     
  Misses        278      278

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

joshka

Lots of nits and a couple of main comments:

First up, thanks for making this configurable. It's something that was on my todo list for sometime when I had some time and energy to do it, so I'm always happy to see a contribution that pushes things forward.

In my initial thinking about this, I'd anticipated that Styling like this would have generally just been a struct with values rather than a trait. This obviously means that there would be more fields for headings to deal with. I'm curious whether you'd thought about any downsides with that approach that using a trait instead solves neatly. The only thing I can really think of is that there's some sort of dynamic values being returned, but I'm not sure that I can think of a good use case for that. This is not a hard blocker, but it's an interesting question, and we face a similar question with Ratatui core when thinking about how to implement theming, so it's worth getting some ideas / understanding here.

The main other questions I have:

non_exhaustive
Default vs BuiltIn

Overall I'm mostly fine with the approach you've taken - I mostly want to understand a few things to work out whether it's the best we can do, and would anticipate pushing forward with this once those are resolved.

tui-markdown/src/lib.rs

joshka · 2025-05-07T19:07:45Z

tui-markdown/src/lib.rs


+// Public-facing theming and configuration APIs --------------------------------
+mod style_sheet;
+pub use style_sheet::{BuiltinStyleSheet, DefaultStyleSheet, StyleSheet};


The Builtin vs Default distinction is a little confusing. Can you expand on why this is necessary / what it solves? Without understanding this, I'd intuitively suggest perhaps keep only one of these.

I removed this for now.

tui-markdown/src/lib.rs

tui-markdown/src/style_sheet.rs

joshka · 2025-06-01T20:25:12Z

@bolinfest are you keen to get this over the line?

…ons for now

joshka · 2025-06-09T02:09:07Z

For now, I changed this so that there's a constructor that accepts a stylesheet, I'd anticipate that adding more options would add builder style methods similar to the way that we do these in Ratatui (Matklad describes this as the builder-lite pattern in https://matklad.github.io/2022/05/29/builder-lite.html).

I've removed the wrapping and line number options, and will add them when that feature is added.

This introduces the use of the `tui-markdown` crate to parse an assistant message as Markdown and style it using ANSI for a better user experience. As shown in the screenshot below, it has support for syntax highlighting for _tagged_ fenced code blocks: <img width="907" alt="image" src="https://github.com/user-attachments/assets/900dc229-80bb-46e8-b1bb-efee4c70ba3c" /> That said, `tui-markdown` is not as configurable (or stylish!) as https://www.npmjs.com/package/marked-terminal, which is what we use in the TypeScript CLI. In particular: * The styles are hardcoded and `tui_markdown::from_str()` does not take any options whatsoever. It uses "bold white" for inline code style which does not stand out as much as the yellow used by `marked-terminal`: https://github.com/joshka/tui-markdown/blob/65402cbda70325f34e7ddf6fe1ec629bcd9459cf/tui-markdown/src/lib.rs#L464 I asked Codex to take a first pass at this and it came up with: joshka/tui-markdown#80 * If a fenced code block is not tagged, then it does not get highlighted. I would rather add some logic here: https://github.com/joshka/tui-markdown/blob/65402cbda70325f34e7ddf6fe1ec629bcd9459cf/tui-markdown/src/lib.rs#L262 that uses something like https://pypi.org/project/guesslang/ to examine the value of `text` and try to use the appropriate syntax highlighter. * When we have a fenced code block, we do not want to show the opening and closing triple backticks in the output. To unblock ourselves, we might want to bundle our own fork of `tui-markdown` temporarily until we figure out what the shape of the API should be and then try to upstream it.

joshka · 2025-11-02T23:07:31Z

@codex take a look at the original commit on this and the updated version with the current PR. Review both the original commit and the updated changes to help validate the usefulness of both approaches and then compare to see if any problems were caused by simplifying this. Try to synthesize or provide an understanding of why the more complex approach might be necessary (use cases etc.).

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2025-11-02T23:10:43Z

tui-markdown/src/lib.rs

        if self.needs_newline {
            self.push_line(Line::default());
        }
-        let style = match level {
-            HeadingLevel::H1 => styles::H1,
-            HeadingLevel::H2 => styles::H2,
-            HeadingLevel::H3 => styles::H3,
-            HeadingLevel::H4 => styles::H4,
-            HeadingLevel::H5 => styles::H5,
-            HeadingLevel::H6 => styles::H6,
-        };
+        let style = self.styles.heading(level as u8);
        let content = format!("{} ", "#".repeat(level as usize));
        self.push_line(Line::styled(content, style));


Preserve 1‑based heading levels when styling

The new start_heading now passes level as u8 into StyleSheet::heading, but HeadingLevel’s discriminants start at 0. This means # H1 is now handed the value 0, ## H2 the value 1, etc., so the default sheet (and any user sheet expecting 1–6) will use the wrong colours and modifiers—H1 currently renders with the fallback style intended for H4+. The original commit explicitly mapped HeadingLevel::{H1…H6} to 1–6 to avoid this off‑by‑one. Please restore that mapping to keep existing themes working.

Useful? React with 👍 / 👎.

bolinfest marked this pull request as draft May 7, 2025 17:39

bolinfest mentioned this pull request May 7, 2025

feat: introduce the use of tui-markdown openai/codex#851

Merged

joshka reviewed May 7, 2025

View reviewed changes

joshka mentioned this pull request Jun 6, 2025

Improve Markdown rendering openai/codex#1246

Closed

feat: introduce from_str_with_options() to support custom stylesheets

13deee0

joshka force-pushed the pr80 branch from 42a5467 to 13deee0 Compare June 8, 2025 23:40

joshka added 4 commits June 8, 2025 17:03

fix: lib.rs review comments

d49d1d6

fix: other review comments / nits

f6777dd

fix: doc updates, remove Builtin (just use DefaultStylesheet)

b196d40

fix: Add Options constructor and docs. Remove wrap / line number opti…

ca436d1

…ons for now

joshka marked this pull request as ready for review June 9, 2025 02:09

joshka mentioned this pull request Jun 9, 2025

Add option to turn off markdown symbols #89

Open

chatgpt-codex-connector bot reviewed Nov 2, 2025

View reviewed changes

feat: introduce from_str_with_options() to support custom stylesheets #80

Are you sure you want to change the base?

feat: introduce from_str_with_options() to support custom stylesheets #80

Uh oh!

Conversation

bolinfest commented May 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-commenter commented May 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

joshka left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

joshka May 7, 2025

Choose a reason for hiding this comment

Uh oh!

joshka Jun 9, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

joshka commented Jun 1, 2025

Uh oh!

joshka commented Jun 9, 2025

Uh oh!

joshka commented Nov 2, 2025

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector bot Nov 2, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

bolinfest commented May 7, 2025 •

edited

Loading

codecov-commenter commented May 7, 2025 •

edited

Loading

joshka left a comment •

edited

Loading