Redesign CLI

[kamalmarhubi]

In the run up to 1.0, I think it would be nice to redesign the CLI with a fresh look at the current functionality. There's been a lot of features added, and some of them aren't super well exposed in the CLI.

I think the big area for improvement is in write-mode. Internally it makes sense as a single enum but for the user I think it is better to break it out into multiple flags. Here is a draft --help output I threw together using clap:

rustfmt 0.5.0
Format Rust code

USAGE:
    rustfmt-cli-v2 [OPTIONS] [ARGS]

OPTIONS:
    -c, --format-children      Formats child modules of FILES
    -i, --in-place             Edits files in place
    -n, --no-backup            Prevents creation of backup files when modifying sources
        --checkstyle           Outputs a checkstyle XML file
    -d, --diff                 Output a unified diff of changes
        --file-lines <JSON>    Formats specified line ranges; see `--help-for file-lines` for details
        --format-diff          Formats parts of files modified in the diff read from stdin
        --coverage             Display how much of the input file was processed
    -v, --verbose              Be verbose; specify multiple times for more
    -h, --help                 Prints help information
        --help-for <TOPIC>     Displays detailed help for TOPIC [values: config, file-lines]
    -V, --version              Prints version information

ARGS:
    <FILE>...    Files to format; if unspecified, will format from stdin instead

It also includes some features I want to implement before 1.0, namely outputting a unified diff, and formatting based on a diff (continuation of #434).

I wanted to put this out for comment before actually reworking the rustfmt binary. The code for creating the CLI is at https://github.com/kamalmarhubi/rustfmt/blob/cli-v2/src/bin/rustfmt-cli-v2.rs.

[nrc]

Sounds good to me!

Details: -i and -n seem connected i.e., -n without -i doesn't seem to make sense. We probably want the default to be no backups too, and have an option for leaving backups (this is a good opportunity to change that default, actually).

I think --help/-h should take an optional parameter, rather than having a separate --help-for.

[kamalmarhubi]

Details: -i and -n seem connected i.e., -n without -i doesn't seem to make sense.

The dependency is actually already set up via clap:

$ target/debug/rustfmt-cli-v2 -n
error: The following required arguments were not provided:
    --in-place

USAGE:
    rustfmt-cli-v2 --in-place --no-backup

For more information try --help

The error message could be a bit better, but I think it makes enough sense. I don't think it's straightforward to change how that prints in clap.

There's similar conflict checking for trying to specify --diff and --coverage at the same time, or other similar combinations.

We probably want the default to be no backups too, and have an option for leaving backups (this is a good opportunity to change that default, actually).

Yeah, I went with a conservative default, but I agree that the default should be changed (#856).

I think --help/-h should take an optional parameter, rather than having a separate --help-for.

Great idea! I'll see if I can do it; clap is sometimes a bit annoying.

[kamalmarhubi]

The --help taking an optional parameter seems to work!

[kamalmarhubi]

One thing I forgot to mention: the default I have in mind is to print to stdout (--write-mode=display). This forces the user to write -i to overwrite files, which seems safer as a default, especially if no backups is default.

[nrc]

Hmm, that is safer, but also less convenient - I imagine the most common use case is reformatting in place.

[kamalmarhubi]

That makes sense to me.

[kamalmarhubi]

@nrc should a new CLI be gated on being ready to switch the default write mode, or can I just wire this all up and make PR?

@marcusklaas any inputs on this?

[nrc]

I think you could probably just do it. Unless you think there is anything really major blocking the default write mode.

[marcusklaas]

                                                                                  Agreed.                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                         Van: Nick CameronVerzonden: donderdag 2 juni 2016 8:39 PMAan: rust-lang-nursery/rustfmtBeantwoorden: rust-lang-nursery/rustfmtCc: Marcus Klaas de Vries; MentionOnderwerp: Re: [rust-lang-nursery/rustfmt] Redesign CLI (#1028)I think you could probably just do it. Unless you think there is anything really major blocking the default write mode.

—You are receiving this because you were mentioned.Reply to this email directly, view it on GitHub, or mute the thread.

[killercup]

@kamalmarhubi looking at the --help output, some things are not really clear to me.

Can you maybe specify/show more clearly which options can be used together and which cannot be combined? E.g, I would be surprised if I can use --in-place and --diff at the same time (though that may be possible if it writes different files/to stdout). Also, what does --checkstyle output? A new file with some specific name?

[nielsle]

It would be great to have the option edit in place while also creating backups in a separate directory for safety.. #796

[nrc]

Closing in favour of #1976