Change write_mode default to overwrite

[kamalmarhubi]

The current default of replace leaves behind a lot of .rs.bk files. Users must specify --write-mode overwrite to avoid it, or else set it in rustfmt.toml, where it doesn't quite belong—see #871 for more on changing that.

The current default is safer than overwrite, especially at this stage in rustfmt's development, and for users who do not commit to version control very frequently. However, eventually we will want to switch the default.

To make that change safe for users, we can do something like what git did when changing the default git push behavior between 1.x and 2.0:

• provide a user configuration setting in the user config to pick the write_mode
• in the run-up to changing the default, warn the users who do not have the write_mode set in their config that the default will be changing (see below for example of git's warning)
• after the change, warn the users without write_mode in their config that the default has changed, and that to get the old behavior they can change the setting (see below for example of git's warning)
• eventually remove the warning (git 2.8, coming out soon, will remove the warning)

Git's pre-change warning:

warning: push.default is unset; its implicit value is changing in 
Git 2.0 from 'matching' to 'simple'. To squelch this message 
and maintain the current behavior after the default changes, use: 

  git config --global push.default matching

To squelch this message and adopt the new behavior now, use: 

  git config --global push.default simple

Git's post-change warning:

warning: push.default is unset; its implicit value has changed in
Git 2.0 from 'matching' to 'simple'. To squelch this message
and maintain the traditional behavior, use:

  git config --global push.default matching

To squelch this message and adopt the new behavior now, use:

  git config --global push.default simple

When push.default is set to 'matching', git will push local branches
to the remote branches that already exist with the same name.

Since Git 2.0, Git defaults to the more conservative 'simple'
behavior, which only pushes the current branch to the corresponding
remote branch that 'git pull' uses to update the current branch.

See 'git help config' and search for 'push.default' for further information.
(the 'simple' mode was introduced in Git 1.7.11. Use the similar mode
'current' instead of 'simple' if you sometimes use older versions of Git)

fatal: The current branch basic-line-ranges has no upstream branch.
To push the current branch and set the remote as upstream, use

    git push --set-upstream origin basic-line-ranges

[marcusklaas]

While I am undecided on the proposed change, please note that replace is the current default. It seems you have mixed up the behaviour of replace and overwrite.

[kamalmarhubi]

@marcusklaas ah good point! Fixed. (A related issue for me is that I find those setting names confusable!)

[kamalmarhubi]

I also just opened #888 :-)

[nrc]

I'm keen to see us change on this. I think the staging proposed is a good path forward, and it'd be great to start that process.

One further possibility is to change the default to overwrite if we detect version control, and replace otherwise. I'm not sure how easy it is to detect VC, but looking for .git or .hg files should be fairly straightforard (not sure if that is cross-platform or not).

[kamalmarhubi]

One further possibility is to change the default to overwrite if we detect version control, and replace otherwise.

This makes some scarily strong assumptions about people's VCS usage. I think playing it safe is better.

[kamalmarhubi]

I'm keen to see us change on this. I think the staging proposed is a good path forward, and it'd be great to start that process.

I'm happy to take this on. I might use it as an excuse to do some refactoring of the CLI too... we'll see!

[kamalmarhubi]

@nrc is there a good idea of how many showstopper bugs exist in rustfmt that would prevent overwrite being a safe default? I figure we should only start actually warning users when the change is actually on the horizon.

[nrc]

This makes some scarily strong assumptions about people's VCS usage. I think playing it safe is better.

This would be playing it safer than always overwriting without backups though, i.e, it is less safe than today, but more safe than the proposed alternative.

I'm happy to take this on. I might use it as an excuse to do some refactoring of the CLI too... we'll see!

Awesome, thanks!

@nrc is there a good idea of how many showstopper bugs exist in rustfmt that would prevent overwrite being a safe default? I figure we should only start actually warning users when the change is actually on the horizon.

I don't really know. I think 'showstopper' changes definition from person to person though. I'd be happy taking a bit of a risk here, since I hear a lot of complaints about backups by default. I feel like now is a good time to start advertising the change, with the intention of changing the default in 2-3 months time.

We should probably track issues which would prevent us turning this on. I'll create a new label to do so.

[nrc]

I created the overwrite-blocker label.

[kamalmarhubi]

In implementing --flie-lines, I did a bit more comparison with how clang-format options work. For this case, it outputs diffs by default, and will overwrite if passed a -iflag.

[chpio]

whats about adding an global config file (eg: ~/.rustfmt.toml) and merge it with the project specific config, so we can set write_mode = "Overwrite" in there.

[topecongiro]

Closed via #1803.