Update documentation on stdin input with rustfmt

[calebcartwright]

rustfmt supports formatting files as well as rust code provided via stdin. However, the readme documentation and rustfmt --help description text could use some updates to make this explicitly clear, as well as call out that if you run rustfmt without specifying any files that rustfmt will be waiting for input from stdin.

This could be addressed by updating the running section in the readme, with more details and ideally some examples (e.g. echo "fn main ( ) { }" | rustfmt with stdout output from rustfmt), as well as updating the structopt info to provide more detail when users run rustfmt --help.

rustfmt/src/rustfmt/main.rs

Lines 44 to 47 in 27b54e9

	/// Format Rust code
	#[derive(Debug, StructOpt, Clone)]
	#[structopt(name = "rustfmt", version = include_str!(concat!(env!("OUT_DIR"),"/version-info.txt")))]
	struct Opt {

https://docs.rs/structopt/0.3.15/structopt/#how-to-derivestructopt

If anyone is interested in working on this please leave a comment or ping us on Discord!

[ayazhafiz]

I just started working on this, do you want me to leave it for a new contributor?

[calebcartwright]

No worries! Feel free to work on this or any issue really, I'm just trying to be more mindful about tagging ones that even net-new users could potentially work on

[NickelCoating]

I would like to give the note part a shot. Once I figured out how to make pull requests form fork to forked from.

[NickelCoating]

Regarding examples, I personally wouldn't even go that far perhaps.

Regarding what I suggested, it may be still not foolproof. The tricky parts is that although most options may need <file>, some may not, like -V for version. And the idea that one may keep the input stdin input out actually still remains although for technical reasons not true. Conveying this dynamic, and trying to keeping it short and snappy without starting the sentence like "YoU mUsT nOt UsE rustfmt FoR pRiNtiNg HeLp", is tricky a bit.

As a side note for the designe choice regarding <file>. I figure <file> wasn't attached to an actual option because it is used by most options anyways, and it's the whole purpose of rustfmt to format that. However, since nobody every, not even I, would use an option without reading it up in the docs/info what it actually does, that actually would clear up this issue. And also allow typing rustfmt for printing help like the others. Except of course, this would be a much bigger change than the docs and introduce legacy code/behavior. But that is not needed, it is just a side note about the design choice here of how <file> functions. My 2 cents.

I closed my previous pull request, this needs a notch more of thought. But I've got an idea.

[ayazhafiz]

I think all users of rustfmt have the same goal: just to get the app to format some code. How to tell rustfmt to do this feels secondary to that goal, and while it is important, IMO being able to provide good documentation for the app and maintaining compatibility with existing usages is much more important.

UX on the granularity of CLI flags and default inputs expected by a program can be disagreed on by reasonable people; I think it is unlikely a productive conclusion would be made of this. imho we should focus on where we already are and find ways to better support information discovery.

[NickelCoating]

Current Readme:

You can run Rustfmt by just typing rustfmt filename if you used cargo install. This runs rustfmt on the given file, if the file includes out of line modules, then we reformat those too. So to run on a whole module or crate, you just need to run on the root file (usually mod.rs or lib.rs). Rustfmt can also read data from stdin. Alternatively, you can use cargo fmt to format all binary and library targets of your crate.

You can run rustfmt --help for information about available arguments.

When running with --check, ...

Take 2. Mind the lack of "Rustfmt can also read data from stdin.", since it is later mentioned.

You can run Rustfmt by just typing rustfmt filename if you used cargo install. This runs rustfmt on the given file, if the file includes out of line modules, then we reformat those too. So to run on a whole module or crate, you just need to run on the root file (usually mod.rs or lib.rs). Alternatively, you can use cargo fmt to format all binary and library targets of your crate.

Running rustfmt needs either a file, or a source via stdin directly, and is mandatory. Except, exclusively with options -V for printing the version and --help for printing option usage information.

When running with --check, ...

For the record, it's bugging me that <file> looks like mandatory only to not be that really. It's of course logical that printing version and help doesn't need any file to format. But it's not logical to look mandatory when from a very technical point, it's actually not. You know 1+1=2.5. I'm not sure how to tackle this issue, unless it doesn't sound too stupid to mention that -V and --help don't need data. For what it's worth, it makes it clear though that all other options need it without exception, it could be worth a hint, and of course a mere rustfmt would be discouraged with that wording.

But, somebody must correct me if that's not even true, that all other options need <file>. It's just the impression I have currently by reading the --help content through.

The new for <file> could be <filename.rs or source via stdin>.

[ayazhafiz]

it's bugging me that <file> looks like mandatory only to not be that really

This would be a good issue to file upstream to structopt. The relevant code is here:

rustfmt/src/rustfmt/main.rs

Lines 130 to 132 in 27b54e9

	// Positional arguments.
	#[structopt(parse(from_os_str))]
	files: Vec<PathBuf>,

Imo Vec<T> should be printed as [files]* in the args, not <files>. Maybe they will say that files should be Option<Vec<T>>, but then you get the error message Option<Vec<T>> type is meaningless for positional argument.

[calebcartwright]

@ayazhafiz - let me know if you're still interested in working on this one and if so i'll drop the help-wanted label. Feel free to revamp the running section altogether, as it's a bit outdated with rustfmt 2.0 anyway (for example --recursive flag would be required to format submods).

I'd like to lead with cargo fmt as that's the most common use case and easiest usage pattern for folks, and then call out the files & stdin options with rustfmt itself. There's a portion of rustfmt users that make use of the stdin feature pretty extensively, but we can certainly do a better job broadcasting that feature and use cases.

As far as the inline help text, understood we may be limited in some ways around what's available via structopt, but I have to imagine we can make some improvements. Even something as marginal as changing Format Rust code to something like Format Rust code in files or from stdin would be helpful.

[ayazhafiz]

Yeah I can take this, will do in the next day or so.

…

________________________________ От: Caleb Cartwright <notifications@github.com> Отправлено: Tuesday, July 21, 2020 1:36:24 PM Кому: rust-lang/rustfmt <rustfmt@noreply.github.com> Копия: hafiz <ayaz.hafiz.1@gmail.com>; Mention <mention@noreply.github.com> Тема: Re: [rust-lang/rustfmt] Update documentation on stdin input with rustfmt (#4336) @ayazhafiz<https://github.com/ayazhafiz> - let me know if you're still interested in working on this one and if so i'll drop the help-wanted label. Feel free to revamp the running section altogether, as it's a bit outdated with rustfmt 2.0 anyway (for example --recursive flag would be required to format submods). I'd like to lead with cargo fmt as that's the most common use case and easiest usage pattern for folks, and then call out the files & stdin options with rustfmt itself. There's a portion of rustfmt users that make use of the stdin feature pretty extensively, but we can certainly do a better job broadcasting that feature and use cases. As far as the inline help text, understood we may be limited in some ways around what's available via structopt, but I have to imagine we can make some improvements. Even something as marginal as changing Format Rust code to something like Format Rust code in files or from stdin would be helpful. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub<#4336 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AE6GL6XQR5UUMQXKDRTYBZLR4XN2RANCNFSM4PDA4D5A>.

[calebcartwright]

Awesome, thank you!

[ayazhafiz]

@calebcartwright we should reopen this until the follow up PRs go in?

[calebcartwright]

Yeah good catch. I thought the PR description was update to remove the keyword but presumably some commit message had it in there