Skip to content

Review logic for getting CLI arguments #3310

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

Closed
scampi opened this issue Feb 1, 2019 · 6 comments
Closed

Review logic for getting CLI arguments #3310

scampi opened this issue Feb 1, 2019 · 6 comments
Labels
fun! :) good first issue Issues up for grabs, also good candidates for new rustfmt contributors

Comments

@scampi
Copy link
Contributor

scampi commented Feb 1, 2019

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).

@topecongiro
Copy link
Contributor

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.

@topecongiro topecongiro added the fun! :) label Apr 24, 2019
@scampi scampi added the good first issue Issues up for grabs, also good candidates for new rustfmt contributors label May 8, 2019
@sphynx
Copy link
Contributor

sphynx commented May 14, 2019

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

@sphynx
Copy link
Contributor

sphynx commented May 16, 2019

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?

@topecongiro
Copy link
Contributor

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

@sphynx
Copy link
Contributor

sphynx commented May 18, 2019

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

@mkantor
Copy link

mkantor commented Oct 8, 2019

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

@scampi scampi closed this as completed Oct 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fun! :) good first issue Issues up for grabs, also good candidates for new rustfmt contributors
Projects
None yet
Development

No branches or pull requests

4 participants