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.

[NickelCoating]

@ayazhafiz If that can mean that typing rustfmt alone can print info then, then I will have a closer look at the code finally.

And regarding this printing of info through just typing cargo, rustup, and perhaps rustfmt, too. Correct me if I'm wrong, but that's an "idiomatic" way for CLI software in general, to be able to do this. My old Quake devs tools collecting dust, so all light, qbsp, and vis exes, they always print usage information in the terminal when used without any input or options, too.

So, whether it's just my OCD or perfectionism, this should be fixed, frankly, either way if possible. I will have a look.

[calebcartwright]

@NickelCoating - I opened this issue specifically to address updating documentation, but I feel like this is going way off topic with the conversation veering back towards when help text should be displayed and the-V for displaying version information.

You've expressed your thoughts on what you think should happen, but the current behavior of rustfmt allowing users to provide text via stdin is a feature, not a bug. Removing the current feature to instead provide help text is an entirely different discussion (and a breaking change) that should happen elsewhere.

Please keep comments on this thread on topic, namely augmenting the existing help text content for the current behavior

[NickelCoating]

@calebcartwright , yes, I'm not viewing this as a bug. I just need a full picture, and I like to write down such thoughts of the whys. The ripples such a feature change would cause is a such a reason why one may want to stick to just changing the docs instead, indeed. So, I agree with you, @calebcartwright lets not blow things out.

However, unfortunately I added new reasoning to this story that I started with the last issue I opened. Reasoning only mentioned here, and not there. The new reasoning for a blank rustfmt being able to print info is the "idiomatic terminal behavior", that ALWAYS prints info, or usage information when no input is given. I made an example with the Quake level design dev tools that behave all the same way.

So, let's just start here. My claim that a blank command prints usage information is idiomatic terminal behavior, it needs to be challenged. Because if that is true, only a hobby project would ignore such "rules", and not professional software like Rust. Meaning if it possible to maintain the feature, and do the info, too, then writing docs now may only get you to rewrite the docs later when someone else points out this flaw!? The docs really can wait for me at least. I already know what I did run into, for what it's worth.

So, what does everybody think about this idiomatic claim of mine?

[calebcartwright]

@NickelCoating - Please respect my previous ask that you not continue taking this thread off topic. I am marking these comments as off topic, and am not going to allow a continued orthogonal debate here. If you really want to request a change in behavior you are welcome to open a new issue, but this topic has been asked and answered multiple times already.

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