Ignore *.bk files generated by rustfmt in cargo-generated .gitignore

[Amanieu]

This will also avoid having .bk files published to crates.io.

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @huonw (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

[steveklabnik]

I have always felt like these files were totally superflous. @nrc are there any plans to make rustfmt just stop generating these files?

[steveklabnik]

(If not, then huge 👍 from me)

[alexcrichton]

I personally prefer to be pretty conservative with gitignore directives (and very liberal with global gitignore directives), and I suspect that rustfmt is the tool here which may want to not generate *.bk files by default. Which I guess is another way of saying I'd rather not add this to Cargo just yet and see if we can perhaps tweak the experience of using rustfmt

[alexcrichton]

Closing due to my previous comment.

[gnzlbg]

@nrc any updates on this?

I actually don't mind whether rustfmt generates these files by default or not as long as there is an option to not generate them. The --write-mode flag seems to allow this with overwrite/replace. Is that right?

@alexcrichton the extension is not *.bk but rather *.rs.bk which is pretty rust specific. If it is possible to not generate these files already (even if that is not the default behavior), I think that a better option might be for cargo fmt to call rustfmt with the appropriate flag to not generate these files.

That would work on old and new projects without needing to modify the .gitignore file and without needing to change rustfmt default behavior.

[alexcrichton]

@gnzlbg yeah I'd be down for that! (still in favor of modifying rustfmt)

[nrc]

The plan is that we'll stop producing these files when we're confident that Rustfmt is not likely to screw up. I think we're almost there. I'd hoped to have a release for Rustfmt around now and change the default behviour then, but I found a pretty fundamental issue that needs a big fix :-(

I'd be happy to change the default behaviour of cargo fmt, it would be a pretty easy change if someone wants to submit a PR. Note that this doesn't entirely solve the problem because a lot of users use Rustfmt from their editor, rather than via Cargo.

You are correct that --write-mode allows for these files to be skipped.

[sanmai-NL]

@nrc, @alexcrichton: I'm encountering the issue that motivated this issue report even today. Reading this thread, I'm not satisified the issue has been addressed. Running a bare cargo fmt command still produces these files with rs.bk extension (see meta info below).

@alexcrichton: Could you please share your experiences why you would want to be conservative with gitignore directives, as added to blank projects created with cargo new? Having sensible default gitignore templates will of course not prevent someone who knows what he or she is doing from adding odd files to a Git repo.

Meta

cargo fmt -- --version

0.7.1 ()

cargo --version

cargo-0.18.0-nightly (fdfdb5f 2017-02-16)

[gnzlbg]

@nrc @alexcrichton I think rustfmt should spit the backup files forever. It's job is to format possibly broken rust code as good as it can. Since the code can be arbitrarily broken in subtle ways, it can screw up, so it should produce backups, always.

OTOH cargo fmt's job is to format your code without breaking anything. For this, it actually can 1) compile, test, what not, 2) reformat using rustfmt, 3) recompile, test what not, 4) either delete the backups if everything went fine, or revert rustfmt changes.

[alexcrichton]

@sanmai-NL

@nrc claimed above that these files won't be produced forever, and in the meantime it should be possible to add this to .gitignore fairly easily.

@gnzlbg I'll leave that up to @nrc, he works more on rustfmt.

[sanmai-NL]

Yes, I'm now adding src/**/*.rs.bk to my .gitignore in every project. Beyond that, I'm requesting an exclusion to the same effect be added to the default .gitignore for Cargo projects. That exclusion can be removed again later, after a transition period, after rustfmt has really stopped leaving those files. It is not certain that those files will always be cleaned up even if rustfmt gets improved. And in the meantime committers to Cargo-managed projects may inadvertently add those files to their Git repos. Also, my IDE hides those pesky backup files from my project dir tree - if only they are excluded in its .gitignore.

It's only a single, low-impact addition to a default .gitignore, so I request you do add that line for the time being and be a bit more expedient in this regard.

[nrc]

The plan is still to stop producing these files at some point (when we're confident we're not screwing up). I'm a bit less optimistic about when that might be now though. It might be that we produce them for the next year. In which case it seems worth adding to the .gitignore maybe?

[steveklabnik]

This is very offtopic, but couldn't we make these files opt-in? Given git exists, they've always felt superfluous to me.

…

On Wed, Feb 22, 2017 at 2:08 PM, Nick Cameron ***@***.***> wrote: The plan is still to stop producing these files at some point (when we're confident we're not screwing up). I'm a bit less optimistic about when that might be now though. It might be that we produce them for the next year. In which case it seems worth adding to the .gitignore maybe? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#2409 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABsij-GELEUaD8dkDjQ6-9mEctuna4Lks5rfIefgaJpZM4HhBxy> .

[nrc]

When I talk about whether we produce them or not I'm talking only about the defaults. Currently they are opt-out at some point we should change to being opt-in.

[alexcrichton]

We discussed this during tools triage yesterday and the conclusion was that let's just go ahead and add this to .gitignore. @sanmai-NL want to send a PR?

[sanmai-NL]

@Amanieu: what do you wish, do you want to redo your PR or do you leave it to me?

[Amanieu]

@sanmai-NL I'll leave it to you.

[gnzlbg]

Nice. I just wanted to add that, e.g., clang-format (which is already very old) still does require users to opt-into not emitting backup files, and I don't think that this behavior is ever going to change.

One can feed the tool arbitrary broken code, and if it doesn't realize it, it can break that code even more (e.g. think of feeding it a half commented file) from a text editor while editing.

So I think a "safe reformatting" utility in cargo still would be very useful (compile check -> format -> compile check) or even (build -> test -> format -> build -> test).