-
Notifications
You must be signed in to change notification settings - Fork 27.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Semantics of "Discard All Changes" in Source Control panel considered harmful #32459
Comments
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 |
@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:
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.
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.
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:
Let me know what you think. |
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 use
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...
|
|
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! |
I don't think that having the equivalent of |
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.
You may know how many ppl use the command, but how can you how many of them deleted files by mistake? 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. |
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. |
Just don't ship VS Code with the git extension built in. |
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?
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... |
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 |
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. |
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 |
@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. |
@Mohjive: well, It seems also like @joaomoreno is very willing to change the message with his proposal! |
@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. |
+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 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 |
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. |
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 |
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). |
prefer stash over clean?Maybe not just for me, most of the time when I want to discard current In the context of GUI I think it should be more lousy/explicit when In case someone really want |
@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 |
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). |
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 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 |
As an author of git tooling, the fact that 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:
or
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. |
One thing probably worth pointing out is that a lot of git GUIs have a |
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 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: |
@ionut-botizan Git has a clear concept of what constitutes a “change”; untracked files are listed separately for good reason. Like you yourself say, 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.) |
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. |
A couple of questions:
Can I suggest renaming the option from |
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. |
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:
This is displayed like this in vscode
and the state of the repository is reported by
git status
like this, as expectedStage file test-1.txt and commit it
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
There are multiple things that I consider problematic:
git reset --mixed
(what it seems to do) and "Discard ..." to just do the equivalent ofgit reset --hard
, but the latter really also does agit 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.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."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.
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.
The text was updated successfully, but these errors were encountered: