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.

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

Raw Markdown displayed in command line help #231

Closed
palant opened this issue May 7, 2024 · 3 comments
Closed

Raw Markdown displayed in command line help #231

palant opened this issue May 7, 2024 · 3 comments
Assignees
Labels
enhancement New feature or request

Comments

@palant
Copy link
Contributor

palant commented May 7, 2024

Describe the bug

Running a Pingora server with --help command line flag will produce descriptions including raw Markdown. Also, the names of command line flags unnecessarily. These are meant for online documentation and often make little sense as a command line help.

Pingora info

Please include the following information about your environment:

Pingora version: 2501d4a
Rust version: 1.79.0-nightly (28e7b2bc0 2024-04-05)
Operating system version: Fedora 39

Steps to reproduce

Run cargo run --example gateway -- --help from the command line.

Expected results

A human-readable output without any duplicated information.

Observed results

Raw Markdown and duplicated information on command line flags, pointless references to code (the command description in particular is meaningless) and references to code (ServerConf) that should have really been URLs:

basic 0.1.0
Command-line options

Call `Opt::from_args()` to build this object from the process's command line arguments.

USAGE:
    gateway [FLAGS] [OPTIONS]

FLAGS:
    -d, --daemon       
            Whether should run this server in the background
            
            `-d` or `--daemon` can be used
    -h, --help         
            Prints help information

        --nocapture    
            Not actually used. This flag is there so that the server is not upset seeing this flag passed from `cargo
            test` sometimes
    -t, --test         
            Test the configuration and exit
            
            When this flag is set, calling `server.bootstrap()` will exit the process without errors
            
            This flag is useful for upgrading service where the user wants to make sure the new service can start before
            shutting down the old server process.
            
            `-t` or `--test` can be used
    -u, --upgrade      
            Whether this server should try to upgrade from a running old server
            
            `-u` or `--upgrade` can be used
    -V, --version      
            Prints version information


OPTIONS:
    -c, --conf <conf>    
            The path to the configuration file.
            
            See [`ServerConf`] for more details of the configuration file.
            
            `-c` or `--conf` can be used

Additional context

The docstrings that this info is compiled from should really be adjusted. While an application extending the default command line options will overwrite the application description, adjusting the description of individual options is non-trivial.

Alternatively, the docstrings can be left unchanged and structopt help messages can be specified separately.

@andrewhavck andrewhavck added the enhancement New feature or request label May 9, 2024
@johnhurt johnhurt self-assigned this May 10, 2024
@drcaramelsyrup
Copy link
Contributor

Addressed in 479d9ba.

@palant
Copy link
Contributor Author

palant commented May 24, 2024

@drcaramelsyrup Unfortunately, this isn’t quite addressed. While the short help (-h command line flag) got quite usable, the long help (--help command line flag) that I quote in the original post is still problematic. The result of running cargo run --example gateway -- --help is currently:

basic 
Command-line options

Call `Opt::from_args()` to build this object from the process's command line arguments.

USAGE:
    gateway [OPTIONS]

OPTIONS:
    -c, --conf <CONF>
            The path to the configuration file.
            
            See [`ServerConf`] for more details of the configuration file.

    -d, --daemon
            Whether should run this server in the background

    -h, --help
            Print help information

    -t, --test
            Test the configuration and exit
            
            When this flag is set, calling `server.bootstrap()` will exit the process without errors
            
            This flag is useful for upgrading service where the user wants to make sure the new
            service can start before shutting down the old server process.

    -u, --upgrade
            This is the base set of command line arguments for a pingora-based service

The issue is that doc comments set both help and long_help values. The code now overwrites the help value but long_help is still set by doc comments, with all the Markdown code in place. This behavior is the same for structopt and clap.

From the documentation it looks like clap (unlike structopt) allows resetting long_help:

#[clap(
    short,
    long,
    help = "...",
    long_help = None
)]

@palant
Copy link
Contributor Author

palant commented May 24, 2024

Btw, I have no idea what the description of the --upgrade command line flag is supposed to tell me but that’s a different issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants