-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Clap derive should parse markdown doc comment into normal text #2389
Comments
There is quite a bit of discussion in #2401 |
One of the challenges with this is we'd basically need to pull in pulldown-cmark which is quite a large dependency. This would need to be put behind a feature flag. |
My main goal was to preserve newlines in the doc comment and that should be achievable with the Example// clap = { version = "3.0.0-rc.4", features = ["derive"] }
use clap::Parser;
#[derive(Parser, Debug)]
#[clap(name = "xh")]
struct Cli {
/// Optional key-value pairs to be included in the request.
///
/// • key:=value to add a complex JSON value (e.g. `numbers:=[1,2,3]`)
/// • key@filename to upload a file from filename (with --form)
/// • header:value to add a header
/// • header: to unset a header
/// • header; to add a header with an empty value
///
/// A backslash can be used to escape special characters (e.g. weird\:key=value).
#[clap(value_name = "REQUEST_ITEM", verbatim_doc_comment)]
raw_rest_args: Vec<String>,
}
fn main() {
let _ = Cli::parse();
} Output
I wouldn't mind if this issue was to be closed. |
I think its still an interesting topic for us to weigh out. I did call out the workaround in the issue so its more discoverable. |
We may need only a subset of markdown features in CLI parser:
There also could be some mechanism to cut off help message from doc comment as @epage suggested here — IMO markdown headers are a perfect mechanism for that e.g.:
I think this subset should satisfy 99.9% of all use cases of clap, everyone should like it, and it doesn't seem to be so complicated to require pulldown-cmark or any other heavy dependency. |
My experience writing one off parsers has made me a bit cautious of doing so. I threw together some md parser benchmarks to see what might be small enough for being an optional dependency. |
Another thought on markdown support, should we support it at runtime, compile time, or both? The core of this Issue is our line breaks which is a problem exclusive to the derive API. For that, we could build markdown parsing directly into Another step up is markdown formatting. For that, we'd want to expose it to both APIs. Say we generalized our The main benefit of this is that this would remove the binary-size overhead of the markdown parser. We'd still need to compile it (though it'd be optional). Though this would mean we wouldn't regress in This has the added benefit that we wouldn't need yet-another Setting in the API for doing controlling this at runtime. |
Another question for us to answer in this issue is how disruptive would markdown parsing be. While |
This reverts commit 333b993. PR clap-rs#1810's motivation was effectively "this is redundant with `\n`". That is a fine motivation. Unfortunately, we don't have a way to force a hard break in `clap_derive`. clap-rs#2389 should help with this eventually but we shouldn't hold up 3.0 to get that ready. So in the mean time, we are restoring `{n}`. We have clap-rs#3230 for tracking the re-removal of it.
One design problem I've been running into is how to render markdown at compile time when the colors aren't known until runtime. I think the way to do this is to allow a |
I'd only ask for italic and bold for basic formatting. |
In my case I just want to add a space in a list without having to add a newline between the bullets and without having to use verbatim doc comments (so wrap_help is respected properly) |
Is there some specification on how this should work? Should we fully parse markdown, or should we just implement the parts that make sense, i.e., lists, bold, italic? I'm currently quite annoyed by lists not working so I'd be willing to invest some time into implementing some solution here. |
How established is CommonMark in Rust docs contexts? I would suggest pulldown-cmark or, if another Markdown parser is established in Rust context whatever that is, as a full flow Markdown parser makes more sense than a "whatever makes sense/minimalist" approach. Markdown is notoriously hard to actually get right even if small things like emphasis or list parsing. |
Subsets of markdown were covered earlier in the thread |
#2389 (comment) you also cautioned against writing a custom parser should we just use pulldown-cmark and ignore the ones we don't support? Also should this be configurable, for some the mapping to ANSI is pretty clear e.g. bold or italic while for others it's unclear e.g. Another thing to consider would be should support be implemented for html tags like in color_print: https://docs.rs/color-print/latest/color_print/index.html#list-of-accepted-tags |
Looking at md parser benchmarks, I would be curious about how well we could get by with minimad. Details like It would be good to have a way for people to set colors but I would be worried about full HTML support. |
The only reason I mentioned I would agree that any block elements would be annoying to deal with, so only inline ones could work. Maybe adding some tags for all the styles definable could be useful, i.e.,
Makes sense, though one could also add the different markdown elements to The other question is should mark down parsing be enabled at runtime as well? If not, the compile time parsed markdown needs to be converted like you suggested here:
Even if we support markdown parsing at runtime, one might like to move that effort to compilation where possible, but we could also just implement it for runtime first and then check if there is a relevant performance penalty end disable it if so. The whole markdown feature should probably be behind a feature flag anyway. Would you be in favor of building a PoC for markdown parsing to test it out? |
I suspect we should do this all at compile time though either direction comes with its problems. One combination of challenges
iirc there were more. Feel free to play around and come back with more ideas. I feel like I'd want things settled a little more before adding an unstable feature for this. |
Maintainer's notes:
verbatim_doc_comment
attributeBlocked on finalizing details of
StyledStr
, includingtemplate
#1433Please complete the following tasks
Rust Version
rustc 1.48.0 (7eac88abb 2020-11-16)
Clap Version
3.0.0-beta.2
Minimal reproducible code
Steps to reproduce the bug with the above code
Actual Behaviour
Expected Behaviour
Additional Context
In StructOpt, it possible to use the
long_help
attribute to preserve newlines butI couldn't find something similar in clap-derive.Just found that it is called
long_about
in clap-derive.Debug Output
Will add if requested
The text was updated successfully, but these errors were encountered: