-
Notifications
You must be signed in to change notification settings - Fork 8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: highlight code blocks #51
Conversation
Hmm I think Code is actually used for inline snippetes |
Fixed 👌 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally LGTM. I started to review this earlier this week, but didn't hit submit. Apologies on the delay. A few minor comments.
@@ -0,0 +1,352 @@ | |||
--- | |||
source: tui-markdown/src/lib.rs | |||
expression: "from_str(indoc!\n{\n \"\n ```rust\n fn main() {\n println!(\\\"Hello, world!\\\");\n }\n ```\n \"\n})" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if there's a good way to write this as a multi-line string instead of with inline \n characters?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it always the expression inside the macro? Maybe it's an idea to assign variables with descriptive names.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems to work by assigning to a variable first, I've also changed the blocks slightly so it's a bit easier when reviewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In theory this could also be manually written in yaml as something like
expression: |
from_str(indoc!{"
...
From some naive testing, insta doesn't seem to care too much about the expression regardless, so the indoc parts are probably unnecessary there either (their effect is just to remove the initial indentation in the rust code). This is not a big deal however.
Co-authored-by: Josh McKinney <[email protected]>
Thanks for the feedback! Updated the PR. |
Added a test to verify, looks alright |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #51 +/- ##
==========================================
+ Coverage 37.66% 41.68% +4.01%
==========================================
Files 5 5
Lines 446 475 +29
==========================================
+ Hits 168 198 +30
+ Misses 278 277 -1 ☔ View full report in Codecov by Sentry. |
The ratatui/underline-color feature was added automatically due to feature unification at the workspace level (due to markdown-reader pulling in ratatui/default). This is probably a small problem for targets that don't have this feature available, but if it's one that causes problems, this can be solved in another PR I think. |
I enabled this by default. To disable set default-features = false. |
Thanks for the PR @timonv |
Hey there, love the simplicity of tui-markdown. Using this for a project that renders code, and I noticed it was adding an extra new line after starting the codeblock, and adding newlines between each code line. Additionaly, the code event does not get triggered.
After fixing that, I figured code highlighting would be cool. Added it behind a feature flag as the library is not lightweight.
Added tests with insta. Since the github pipeline is not in the same repository, I couldn't test for both features. Perhaps using cargo-hack with --each-feature is a nice solution.
Might be nice to also set a background for the full block, with the maximum line length from the full block. Current solution and syntect just do line length, which in my opinion doesn't look great.