Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Markdown parsing in doc comments #5891

Merged
merged 2 commits into from
Feb 3, 2025
Merged

Markdown parsing in doc comments #5891

merged 2 commits into from
Feb 3, 2025

Conversation

ModProg
Copy link
Contributor

@ModProg ModProg commented Jan 24, 2025 •
edited
Loading

fixes #2389

  • add tests for markdown parsing
markdown output comment
paragraph surrounded by empty line
# heading bold + underline + surrounded by empty line default style for heading
![image]() alt text anyone feeling sixel?!
<html> verbatim
> blockquote image alternatively could use >
code block image same styling as `code`
(un)ordered lists ordered list example
*emphasis* italic
**strong** bold
~stikethrough~ strikethrough
[link](url) underlined alt text + OSC 8
`inline code ` bold default style for literal
--- hrule --- surrounded by empty lines
| tables | verbatim
\\\n and \n hard break ends line

Open Questions

  • unicode support i.e. • for bullet lists, Options::ENABLE_SMART_PUNCTUATION for pulldown_cmark, unicode lines for blockquotes
  • using the default clap styling for code makes it the same to bold.
  • should we make images a link to the image source
  • should we add custom html tags a la https://docs.rs/color-print/latest/color_print/ this would allow users to use colors without us actually supporting runtime style information.
  • should we use other enumeration for nested lists? e.g. a., b. and i., ii.? we could use https://docs.rs/nominals for that
  • styling for code blocks and block quotes

@ModProg ModProg mentioned this pull request Jan 24, 2025
Closed
2 tasks
ModProg
ModProg commented Jan 26, 2025
View reviewed changes
Cargo.toml Outdated Show resolved Hide resolved
epage
epage reviewed Jan 27, 2025
View reviewed changes
clap_derive/Cargo.toml Outdated Show resolved Hide resolved
@ModProg ModProg requested a review from epage January 29, 2025 00:56
@epage
Copy link
Member

epage commented Jan 29, 2025

Could you clean up the commits for how you'd like this reviewed?

Generally, its best to have a commit that adds tests before the feature work. In this case, i would expect it to show the current handling so the diff shows how formatting changes.

@ModProg
Copy link
Contributor Author

ModProg commented Jan 29, 2025

@epage Changed the commits as requested.

@ModProg
Copy link
Contributor Author

ModProg commented Jan 29, 2025

I also added the open questions I had in the PR description

@epage epage marked this pull request as ready for review January 31, 2025 22:38
@epage
Copy link
Member

epage commented Jan 31, 2025

Before merging, preference on whether #2389 should be used as the tracking issue or if we should open a dedicated one?

@ModProg
Copy link
Contributor Author

ModProg commented Jan 31, 2025

Maybe a new one would be good, we could transfer the open questions from the PR description there to discuss them more easily.

@ModProg
Copy link
Contributor Author

ModProg commented Feb 2, 2025

@epage should I squash the commits now to match the commit message requirements?

@epage epage merged commit c99872c into clap-rs:master Feb 3, 2025
26 of 27 checks passed
@epage
Copy link
Member

epage commented Feb 3, 2025

If you'd like, go ahead and create the tracking issue so you have edit permissions.

As it was just a test commit, I wasn't too worried.

@ModProg
Copy link
Contributor Author

ModProg commented Feb 3, 2025

I created it in #5900

@dpc
Copy link

dpc commented Feb 3, 2025 •
edited
Loading

This is such a cool feature.

Out of curiosity: Have you considered usind Djot https://github.com/jgm/djot? I think it might be relevant when Markdown is added to a crate that a lot of people focus very much w.r.t binary size and performance. AFAIA, djot is specifically a simplification of Markdown to make parsing faster and easier.

@epage
Copy link
Member

epage commented Feb 4, 2025

This is currently limited to clap_derive and happens during build time, so build time is the only consideration. I created https://github.com/rosetta-rs/md-rosetta-rs to compare different implementations. We aren't locked into one though parity with rustdoc is a bonus.

As for using a different format, that is unlikely. Besides having a rust implementation, compatibility with rustdoc is important. At first glance, I didn't see what all they changed but we'd at least needa subset of markdown.

@ilyagr ilyagr mentioned this pull request Feb 4, 2025
Open
8 tasks
@dpc
Copy link

dpc commented Feb 4, 2025 •
edited
Loading

@epage Oh, cool. I can add jotdown just out of curiosity! :)

As for using a different format, that is unlikely.

It isn't really a different format. It kind of is just a slightly more strict Markdown, AFAICT.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ilyagr ilyagr ilyagr left review comments

@epage epage epage approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Clap derive should parse markdown doc comment into normal text
4 participants
@ModProg @epage @dpc @ilyagr