Rustfmt: Review logic for getting CLI arguments

Created on 1 Feb 2019  Â·  6Comments  Â·  Source: rust-lang/rustfmt

In #2694 it seems that our handling of the env::args() is problematic in some environments like windows. The first item is skipped which is usually the executable name, but not always:

The first element is traditionally the path of the executable, but it can be set to arbitrary text, and may not even exist. This means this property should not be relied upon for security purposes.

Since this logic is used in the binaries shipped with rustfmt, it could be better to use another library like clap where that is not needed (I think).

fun! good first issue

Most helpful comment

@sphynx Thank you for your work! I think we can just use the original clap style message.

All 6 comments

It would be great if we could try out whether clap or structopt (preferably the latter) can replace the current command line interface implementation of rustfmt.

Hello! I am working on it, porting CLI code to structopt.

It looks like I am spending a lot of effort in order to make help and usage messages generated by structopt/clap to look exactly the same as current ones generated by getopts instead of just accepting that new default format. It requires some tinkering of options and format settings in clap and structopt.

Do you think it's worth the effort? Or maybe we should keep clap defaults and consider it more idiomatic?

Now, after all my customizations the help messages look almost the same (with some stylistic differences). Please see examples below.

Old one:

➜ cargo fmt -h

usage: cargo fmt [options]

Options:
    -h, --help          show this message
    -q, --quiet         no output printed to stdout
    -v, --verbose       use verbose output
    -p, --package <package>
                        specify package to format (only usable in workspaces)
        --version       print rustfmt version and exit
        --all           format all packages (only usable in workspaces)

This utility formats all bin and lib files of the current crate using rustfmt. Arguments after `--` are passed to rustfmt.

New one (with some tinkering):

➜  target/debug/cargo-fmt -h
cargo-fmt

USAGE:
    cargo fmt [OPTIONS] [-- <rustfmt_options>...]

OPTIONS:
    -q, --quiet                   No output printed to stdout
    -v, --verbose                 Use verbose output
        --version                 Print rustfmt version and exit
    -p, --package <package>...    Specify package to format (only usable in workspaces)
        --all                     Format all packages (only usable in workspaces)
    -h, --help                    Show this message

ARGS:
    <rustfmt_options>...    rustfmt options

This utility formats all bin and lib files of the current crate using rustfmt.
Arguments after `--` are passed to rustfmt.

More idiomatic clap-style help message (with less tinkering of settings and not many changes in defaults):

➜  target/debug/cargo-fmt -h
cargo-fmt 1.2.2
Tool to find and fix Rust formatting issues

USAGE:
    cargo fmt [FLAGS] [OPTIONS] [-- <rustfmt_options>...]

FLAGS:
        --all        Format all packages (only usable in workspaces)
    -h, --help       Prints help information
    -q, --quiet      No output printed to stdout
    -v, --verbose    Use verbose output
        --version    Print rustfmt version and exit

OPTIONS:
    -p, --package <package>...    Specify package to format (only usable in workspaces)

ARGS:
    <rustfmt_options>...    rustfmt options

This utility formats all bin and lib files of the current crate using rustfmt.
Arguments after `--` are passed to rustfmt.

@scampi @topecongiro What do you think?

@sphynx Thank you for your work! I think we can just use the original clap style message.

@topecongiro Ok, thank you for the feedback!
I’ll polish the code and prepare a pull request in a couple of days.

Should this issue be closed since #3569 has been merged?

Was this page helpful?
0 / 5 - 0 ratings

Related issues

torkleyy picture torkleyy  Â·  5Comments

jonhoo picture jonhoo  Â·  4Comments

thomaseizinger picture thomaseizinger  Â·  3Comments

fulara picture fulara  Â·  4Comments

MoSal picture MoSal  Â·  5Comments