Semantics of "Discard All Changes" in Source Control panel considered harmful

[thierer]

I was curious what happened to the reporter of #32405 so I did some tests, and while he might have some self-inflicted problem being taken serious, I also think there's more to it.

This is vscode 1.14.2 on Linux x86_64.

Steps to Reproduce:

mkdir vs-code-vc-test
cd vs-code-vc-test
ls >test-1.txt
ls >test-2.txt

2. In vscode: "File" -> "Open Folder" -> Open the directory just created

3. On the "Source Control" Tab: Click "Initialize Repository" (but, see below)

This is displayed like this in vscode

and the state of the repository is reported by git status like this, as expected

4. Stage file test-1.txt and commit it

5. Make some random change to the content of file test-1.txt

This is shown like this in vscode

and reported like this by git status

6. Select "Discard All Changes" from the overflow menu (three dots in in the upper right corner) of the "Source Control" Tab and confirm the warning dialog that appears:

7. Result: test-1.txt is reset to the version that was committed (as expected), and test-2.txt is deleted although it was never added to git!

There are multiple things that I consider problematic:

1. Given the two menu entries "Unstage all changes" and "Discard all changes" I would have expected "Unstage ..." to do the equivalent of git reset --mixed (what it seems to do) and "Discard ..." to just do the equivalent of git reset --hard, but the latter really also does a git clean which removes all untracked files from the working tree! That's such a dangerous command that I argue there shouldn't even be an UI entry for it.
2. There doesn't seem to be a equivalent to just git reset --hard in the menu, which I would consider
   (more) useful and might give a hint that "Discard changes" is really even more dangerous. The thing
   about the warning is: If I want to do a git reset --hard I expect a warning (and would therefore confirm it) because it is dangerous.
3. Even if that's really how "Discard changes" is supposed to work, then the warning should not just read
   "Are you sure you want to discard ALL changes?" (is an untracked file really a "change"?) but clearly state the fact that untracked files will be removed and maybe name (some of) them.
4. Minor nitpick: I think the safer option of the two ("Unstage...") should come first in the menu.

What might make things worse (and the scenario from #32405 more plausible) is that I think that during my tests vscode twice initialized a git repository in the current directory on its own, that is, without me clicking "Initialize Repository" or running "git init" on the command line. I haven't really been able to reproduce that, though, so it might be just me being confused. It just mention it in case someone also encountered that before or comes up with a scenario where this might happen.

• VSCode Version: Code 1.14.2 (cb82feb, 2017-07-19T23:26:08.116Z)
• OS Version: Linux x64 4.12.4-1-ARCH
• Extensions:

Extension	Author (truncated)	Version
EditorConfig	Edi	0.9.4
vscode-eslint	dba	1.2.11
python	don	0.7.0
json-tools	eri	1.0.2
Go	luk	0.6.63
cpptools	ms-	0.12.2
polymer-ide	pol	0.6.0

[thierer]

Just found #31479, where this was apparently already discussed. Still, I don't agree with the answer given there: It's not "what git does" - it's what git does when you run git clean which you normally would not...

[joaomoreno]

@thierer As we discuss this, keep in mind that most users who hit the "omg where are all my files" issue come into following these three simple steps:

1. Oh what is this? Yes, let's initialize, git sounds awesome, people use it.
2. Oh what is this? All my files appear in this list... but they didn't before. It looks like its touching my files, I don't like that, how to I get them out of here?
3. Oh what is this? Discard all changes sounds a lot like what I want to do. 💀

People don't hit this issue because we have that action; they hit it because they want to clear that list. And they will go through great lengths to get there, even if not knowing what their doing or after warned that they are in dangerous waters.

---

Given the two menu entries "Unstage all changes" and "Discard all changes" I would have expected "Unstage ..." to do the equivalent of git reset --mixed (what it seems to do) and "Discard ..." to just do the equivalent of git reset --hard, but the latter really also does a git clean which removes all untracked files from the working tree! That's such a dangerous command that I argue there shouldn't even be an UI entry for it.

I don't agree. I use it all the time and I know you do to. You start coding up a feature, change a few files, add a few others and... oops, this is all wrong. Discard all changes discards all those changes. I definitely want to keep that.

What might make things worse (and the scenario from #32405 more plausible) is that I think that during my tests vscode twice initialized a git repository in the current directory on its own, that is, without me clicking "Initialize Repository" or running "git init" on the command line.

I'm pretty sure Code doesn't do that automatically. Maybe you have an extension doing that?

---

As I propose the following solution, keep in mind that I'm trying to minimize power user frustration while protecting inexperienced users from the dangers of this action.

Here's what I think we should do:

• Change the confirmation message for the simple Discard action for a single, untracked file to be more like Are you sure you want to delete the XXX file?.
• Change the behavior of Discard All to depend on a new setting git.discardAll, with values all | tracked | prompt, which defaults to prompt:
  • When prompt and there are untracked files: it will prompt There are X untracked files in this repository which will be deleted if discarded. Do you also want to delete these files?. Answers are Yes, No, Always and Never; these last two will change the setting accordingly.
  • When all, the same behavior as today occurs. We can add a sentence This will ALSO delete all X untracked files or something like that.
  • When tracked, no git clean will happen.

Let me know what you think.

[thierer]

I agree that some affected users should do their homework first, but I also think that as it is now makes it just too easy to lose files. This could have totally affected me to some extend, because I would never have expected "Discard all changes" to delete untracked files.

I don't agree. I use it all the time and I know you do to. You start coding up a feature, change a few files, add a few others and... oops, this is all wrong. Discard all changes discards all those changes.

I use git reset to get rid of changes to files that were already added to the repository, but I don't think I ever used git clean in a directory where I do active development in. Honestly, I'm equally surprised that you do :) The only real use I saw until now is after running a build in a cloned repository as an easy way to get rid of files created during the build. OTOH I often have files with snippets and logs that I don't want to commit but like to keep for the moment and don't bother to add to .gitignore. These files would be deleted by git clean / "Discard all changes". Obviously you're more organized than me :)

I'm pretty sure Code doesn't do that automatically. Maybe you have an extension doing that?

I have no idea. As I wrote, I wouldn't take it too serious. I'll keep an eye on it, but if it never happened to anybody else it was probably just my imagination...

Change the confirmation message for the simple Discard action for a single, untracked file to be more like Are you sure you want to delete the XXX file?.
Change the behavior of Discard All to depend on a new setting git.discardAll, with values all | tracked | prompt, which defaults to prompt:
When prompt and there are untracked files: it will prompt There are X untracked files in this repository which will be deleted if discarded. Do you also want to delete these files?. Answers are Yes, No, Always and Never; these last two will change the setting accordingly.
When all, the same behavior as today occurs. We can add a sentence This will ALSO delete all X untracked files or something like that.
When tracked, no git clean will happen.

1. What would be the difference between "all" and "prompt"? Just that "prompt" would give a more verbose message in the warning dialog? If so, why not just use that message for "all" and only use [all | tracked]?
2. I would make "tracked" the default, as this is the save option. Even though it changes the current behavior. I guess power users who really want their untracked files to be deleted would find the setting and change it to their needs. OTOH, like you mentioned, inexperienced users will always misunderstand what the message says. The prompt "... Do you also want to delete these files?" could well be interpreted as "Do you want to delete these files [from the repository / from this place where they show up where I don't want them / ...]?".
3. Instead of changing the message for single, untracked files, why not remove the command for these files altogether? As I wrote, a message can always be misinterpreted and this is basically just a duplicate to "delete" in the file browser (except that the file browser actually does move the file to the trash!).

[joaomoreno]

1. all means the user has pressed Always. It makes the action behave as today. prompt makes it prompt the user all the time, whenever there are untracked files laying around.
2. I will only do this if prompt is the default option. Using tracked will change the behavior for current users. Using all won't change much for brand new users. I don't want to make power users have to find the option, I will configure it for them based on the Always, Never choice. This has been successfully applied in other parts of the product.
3. I won't remove the command since many people use it.

[thierer]

I see. As the prompt could decide the behavior for all future "discards" it should perhaps be even more explicit then, something like "There are X untracked files in this repository which will be permanently deleted from disk if discarded" to make it clear that we're not talking about the repository here, but that's just me nitpicking...

In any case, I think that's a good solution. Thanks for the quick help!

[mohd-akram]

I don't think that having the equivalent of rm -rf as a GUI option is a good idea under any circumstances. If users want to delete their files, they can do so themselves manually. This will force them to think about every file/folder they're deleting to avoid any accidents. I personally don't remember ever using git clean.

[Brunorscc]

Seriously devs, just accept that the text box message IS misleading and doesn't mention all the files recursively of the entire folder. So, please change it.

I won't remove the command since many people use

You may know how many ppl use the command, but how can you how many of them deleted files by mistake?
Instead of using this command, isn't it better if each one configure its own .gitignore files and dont let undesired files into version control? Because thats what I think is the intent of someone constantly using this 'Discard ALL' option.

Anyway, adding a few words on the text box message (and not actually removing the 'discard ALL' command) explaining its more than a simple 'discard changes' would be (at least) of great help i guarantee.

[Mohjive]

I think @Brunorscc makes a valid assumption in that many users use the discard option to remove generated files they do not want in vcs instead of adding it to .gitignore.

Personally I'd assume that discard would only do the equivalent of git-reset. Never would I guess that it did git-clean as well. Even as it says it is IRREVERSIBLE I would assume that only my changes to the files added to the vcs would be lost. Not the files I never added. Git should not touch the files not added to vcs in my opinion. I have never used git-clean myself, instead I just do "rm -rf *" and "git checkout ." and I'm good to go.

[weinand]

Just don't ship VS Code with the git extension built in.
Users who know what they are doing will install git immediately.
Others will not and they don't run into this issue.

[ionut-botizan]

I never had this issue because, well, I know what I'm doing, but for the more superficial ones, wouldn't be simpler to have 2 separate buttons?

1. Revert all changes - resets modified files
2. Delete all untracked files - I think it's pretty clear what it does

It would take a couple more clicks for me and others like me, but it would avoid the bad publicity generated by those that can't be bothered to learn their tools first...

[carun]

If a git repo has not been created, then why allow the discard operation after all?

I would also attribute this to the messy git commands, which was inturn not well understood by the VS Code developer (not his problem, git CLI is arcane that are known only to the git developers.)

Sent from my Verizon SM-G900V using FastHub

[cdock1029]

I think a lot of pain could be avoided if the word "Discard" was changed to "Delete" since... that's what it does, while Discard has a much more nuanced and overloaded meaning.

[luxzeitlos]

But it's not the same, and it is a discard! If you only deleted files discarding the changes will restore them and not delete anything!

And I use git clean - xfd quite often. I know that's very dangerous, but the best way to get a totally clean workdir.

[Mohjive]

@luxferresum: that's interesting. Can you describe why you do it and what type of files that actin removes for you?

I think there are different views of what "changes" mean. For me it's everything git lists as "modified", i.e. files only added to git at one time would be affected by discard. It's apparent others think of "changes" from the checked in status, i.e. everything different from what was last committed.

In this case of a [very] destructive action I think the best solution would be to take the safe option, if there only was to be one function doing both. Splitting up the actions, as suggested by @ionut-botizan, is even better.

@joaomoreno: are you saying that you never ship breaking changes? Unless you never do I interpret your reluctance to change this as putting your own preference and convenience before user friendliness.

[luxzeitlos]

@Mohjive: well, git clean -xfd is something I often do after a git checkout to clean all caches (node_modules), build artifacts, etc. Or after a git commit before I rebuild everything to check my build will work on a clean environment before I git push. Also if I have some fishy behavior, a git clean -xfd will ensure that its not a cache issue.
Also if you have some files that are in your .gitignore, but the same files are checked in and not in the .gitignore on another branch a git clean -xfd will help you to get a clean environment.
Years ago before I know git clean -xfd I was always just deleting the entire folder and then doing a fresh git clone.

It seems also like @joaomoreno is very willing to change the message with his proposal!
The primary problem is that people need this button, so removing it is actually against user friendliness. So the only thing that could be done is to change the message. And I hope no-one will confirm a message that says it will delete N files permanently without thinking first. Maybe its even a good idea to do a dry-run first, and list what exactly will happen.

[Mohjive]

@luxferresum thank you, but would you consider caches and build artifacts to be "changes" that you "discard" with git-clean? Or are they just what you said, artifacts, that you clean/remove from your working directory? To me this distinction is important as I see this as a discussion about the semantics of "discard changes" and not the usefulness of a function that create a pristine working directory.

In the context of git I believe it's quite clear what discard changes should mean. It's referenced under the --hard option for git-reset (https://www.git-scm.com/docs/git-reset): "Resets the index and working tree. Any changes to tracked files in the working tree since are discarded."

git-clean (https://git-scm.com/docs/git-clean) otoh is very explicit about deleting files: "Cleans the working tree by recursively removing files that are not under version control, starting from the current directory.". This command also requires you to specify a force option to actually do something, else it quits with an error.

I don't doubt there's a need for a function to clean the working directory, but when adding such feature one should really be avoid using terminology already assigned to meaning something else.

[arcticcacti]

+1 to the idea that this shouldn't affect untracked files at all.

As a git user I'd expect "unstage changes" to move any tracked files out of the staging area, without touching the files themselves. I'd expect "discard changes" to completely revert any changes to tracked files, effectively performing a git reset to the last commit.

At no point would I expect git to start deleting files it's not meant to be managing. It doesn't even make any sense - if I have an untracked file, and after several commits I hit "discard changes", why should that file be deleted? It's not a change, it's been there the whole time - it exists outside of the commit history, that's the whole point. Doesn't help that "unstage all changes" uses the exact same wording, but in this case "all changes" naturally refers to tracked files only, reinforcing this (incorrect) expectation that "discard all changes" will affect the exact same files.

If you want to offer a git clean option then fine, but don't refer to 'changes' - I'd argue it's confusing and misleading for both newbies and git users expecting convenient reset options, with catastrophic results. Basically I agree with everything the OP brought up

[sandersaares]

I do expect any modification of my git-tracked directory to count as a change, whether I have staged them or not. Unstaged files are still part of my repository, just ones that I have not decided to add to any commit yet.

[arcticcacti]

What if you've made a decision not to add them to any commit, say because they're temporary files that need to be in the workspace, but you're planning to delete them later without ever including them in the commit history?

If you added them since the last commit then sure, that looks like a change to the repo. But if they've been there for a long time (maybe before you even initialised that folder as a repository) then it makes less sense to call those files' continued existence a change.

I get why you might look at it that way (treating the last commit as the "true" repo state, and any differences in the working tree as a "change") but for new people it doesn't fit the idea of an "undo" to how things were before. And for people familiar with git, it implies a normal reset affecting tracked files, the way git generally works. In both cases it's surprising behaviour in the worst kind of way, because you completely lose everything you thought would be unaffected. If it were implemented as git reset --hard and you were expecting git clean, the worst that could happen is you'd have to delete some files yourself (and in that case you probably know enough to run the command in a terminal)

[sandersaares]

In the case you describe - of intentionally local files that hang around for some time - I will add those files to .gitignore and I expect tools to ignore those files entirely (which I believe VS Code does).

[ryenus]

prefer stash over clean?

Maybe not just for me, most of the time when I want to discard current
changes, I use git stash, so that I can get the changes back via
git stash pop, and when I really want to throw the changes away, I
do git stash clear. Also this has the benefit that untracked files
are NOT touched.

In the context of GUI I think it should be more lousy/explicit when
files are to be deleted.

In case someone really want git clean being used for discarding, I
think there can be a settings entry to toggle the behavior, but the
default should be to use git stash.

[arcticcacti]

@sandersaares sure, but there are also times where I don't want to change my .gitignore just for some temporary files, and a new user probably wouldn't even be aware it exists and what it affects.

Point is this option is unclear about exactly what it does - the menu wording implies reset --hard/undo behaviour, the warning does nothing to suggest otherwise, but it actually performs a much more dangerous and destructive action. The fact your files happen to be 'safe' if they're listed in .gitignore isn't really the issue.

[KallDrexx]

Just as another anecdote, I do probably 80% of all my Git work in GUIs (Git Extensions and SmartGit). Both treat Discard and Delete of Untracked files as two totally separate operations, with an option to do both at the same time (but you have to check the box each time).

[moeffju]

Excuse me for not using 👍 on another comment, I didn’t find any that were close enough to what I wanted to say.

I’m a UX designer, used to be a developer, have a background also in linguistics, and from multiple perspectives I find this problematic. The name of the option is misleading; so is the wording in the dialog box. I don’t think I’ve used git clean in over 15 years of coding, I certainly wouldn’t expect “discard changes” to delete files that were not under version control, especially not if in a newly initialized repository.

Some suggestions on different wording: “Discard everything not in source control”, “Discard changes and untracked files”, “Clean working copy…”. But honestly, I would change this option to “fail softer”, i.e. run git reset --hard without the clean, and maybe add another option specifically to “Remove untracked files [from the working tree]” (quote from git-clean man page), but probably even hidden away under some “Advanced Git” submenu or something. The warning dialog should IMO also state clearly that “This will remove all untracked files from the working directory” and ideally show a few of the files (e.g. “This will remove all untracked files from the working directory, for example: file1.c file2.h file3.pl (+209 more files). This action is irreversible.”) and maybe have a blocking countdown on the “Oh yeah trash my files” button.

[ricardobeat]

As an author of git tooling, the fact that git clean is invoked when the intention is to discard changes only is indeed surprising. As mentioned before, the meaning of change within git is clear, and does not include untracked files.

It seems to me that the whole issue stems from introducing new terminology on top of git - this operation should use wording analog to 'hard reset' and 'soft reset'. In the meantime, it could also do one of the following for damage control:

git stash save --include-untracked "VSCode - Discard all changes"

or

git add .
git stash create # => f827d63... save for later
# proceed with current reset + clean procedure

The first one is equivalent to reset --hard + clean, but undoable with git stash apply and visible to users independently of VScode.

The latter returns a dangling commit hash, not visible in the stash list, which can be used for an undo feature in VScode alone, and will be GCed eventually.

[darthdeus]

One thing probably worth pointing out is that a lot of git GUIs have a git reset button, but I have never ever seen a git clean button. I've been using git for almost 10 years, mostly command line, and tried lots of GUIs, but holyshit I would never imagine anything but git reset --hard for "Discard all changes".

[ionut-botizan]

This discussion is getting ridiculous and I get this feeling that some of the people commenting on this issue don't even use code and are not familiar with the visual context of the "Discard all changes" button; they probably came from HN or something and decided to leave their UX "professional" opinion on a product they most likely know nothing about. For God's sake, it's a list of file names (each of them labeled U, M or D) titled "CHANGES" with a button at the top to revert ALL changes. If you click on that button, you would undoubtedly expect ALL the listed changes to disappear from that list, not just some of them. Stop thinking in git cli terms and look at the UI; that action, albeit dangerous, makes sense!

Also, a new file in a repository is a change you made to that repository and you have 2 choices: commit it or ignore it! There is no such thing as "Oh, I want to ignore it, but just for a while, so I'm not committing it to git, but I'm not ignoring it either". Did it dirtied your repo? Then YES, it counts as a change to that repo, even if it's yet untracked and you should be expecting for it to be removed if you click a "Discard all changes" button! (Hell, when I started using git, before I learned about git clean, I was baffled by the fact that git reset --hard doesn't do that; I mean, do you need it to be --very-hard, or what?!)

---

Anyway, if there are going to be any changes, I have another suggestion (complementary to my first one) for the VSCode devs: why not use the same expandable tree views as in the "Explorer" and separate the modified files from the untracked files in two distinct lists? It should be dead obvious then that you're removing untracked files or you're just undoing changes to tracked files. Something like this:

[moeffju]

@ionut-botizan Git has a clear concept of what constitutes a “change”; untracked files are listed separately for good reason. Like you yourself say, git reset --hard does not touch untracked files, because untracked files are not git’s domain to control. Additionally, since this data loss also happens when you have an existing directory, initialize a repo in it, then “discard changes”, the “new files are changes you made” does not hold.

None of this is to say that any of your points are wrong – I think that would be a fruitless discussion to have – but the argument I and others are making is that this behavior is needlessly unexpected for a user with no, little, or “average” git experience because both the wording and the behavior differ from what is custom and expected in git commandline, other git tools, and even most other popular version control systems. In general, UX that is unexpected is something that should be scrutinized; when there is a risk of irreversible data loss, doubly so.

(I like your suggestion of visually separating the changes from the untracked files btw, it is more aligned with git’s normal working model, and seems like something that should be done. But I don’t think that changes the problem of “discard all changes” doing something unexpected.)

[joaomoreno]

Hi guys. Thanks for all the discussion and passion. Although we've seen many interesting ideas and alternatives, we're not going to dramatically change this feature as a result of this situation: the goal here is to leave the feature intact while attempting to minimize the number of users who delete their files unknowingly. Note that many of the suggestions which appeared here can be easily found as existing issues in the repository.

---

A further iteration brought us here:

This dialog will let you discard the whole change set (like before) as well as only the tracked files, leaving the untracked files untouched.

[samveen]

A couple of questions:

1. In case I run git init, the change is the creation of a .git and initializition of an empty repository. Discarding this change would probably amount to the removal of the .git directory.
2. In case I run a git add ., the change is the staging of all currently unstaged files, whether tracked or untracked. Therefore undoing that change would amount to a git reset --soft instead of --hard

Can I suggest renaming the option from discard all changes to discard all source changes or something more appropriate, as the intent of this option doesn't match it's name.

[joaomoreno]

We consider a change to be source changes by definition. Discarding changes doesn't mean undo in any sense. Although I can see how it can feel like it.