Skip to content
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

parent directory stays selected after deleting a file #15

Closed
buxit opened this issue Apr 7, 2016 · 16 comments
Closed

parent directory stays selected after deleting a file #15

buxit opened this issue Apr 7, 2016 · 16 comments
Labels

Comments

@buxit
Copy link
Contributor

buxit commented Apr 7, 2016

  1. select a file in treemap
  2. right-click on the file in treemap
  3. delete (no way to undelete)
  4. yes
  5. now parent directory of the file is selected.
  6. in treemap select a different file in a different directory.
  7. right-click on the file in treemap
  8. delete (no way to undelete)

the parent directory of the previously deleted file is in the list of files to be deleted

this made me delete a directory i actually didn't intend to delete, so i consider this to be a grave bug.

@shundhammer
Copy link
Owner

I just managed to reproduce this.

It is intentional that after that cleanup action something is selected - in this case, the parent directory. Otherwise, you'd completely lose your context after the cleanup action: Since that directory subtree (the parent directory in this case) is re-read from disk, that entire subtree is deleted from the internal tree and rebuilt, i.e., all prior references inside that tree have become invalid, and the tree widget would scroll to some random location (typically the root of that tree); the user would have to search that directory again in case he'd want to remove another file (which is very common).

What happened in this case, however, is not that the parent directory was selected - that was intentional - but that it is not deselected when you select another object in the treemap. This is indeed a bug.

Trying to reproduce that scenario manually, if you select a directory in the tree view and then you select a file in the treemap (no matter if that file is inside or outside that same subtree), the directory is deselected first and then the file is selected. This is the correct behaviour.

Investigating why this directory remains selected.

Any yes, I know it's very little consolation that the confirmation popup explicitly asked for both objects, the directory and the file. Most users don't pay that much attention to those ubiquitous popups any more. Maybe that's another item that could be improved.

@shundhammer shundhammer added the bug label Apr 8, 2016
@shundhammer
Copy link
Owner

Step 1: Improved confirmation popup for this situation to make it better visible that not only a file, but a file and a directory are selected and will be affected by the cleanup action.

cleanup-confirmation-mixed

@shundhammer
Copy link
Owner

Fixed with c6db443 .

Thanks for testing!

@buxit
Copy link
Contributor Author

buxit commented Apr 11, 2016

retested, now works great.

also the confirmation popup is indeed a lot easier to parse for humans now. (though not completely consistent – it does not look the same when deleting only one directory)

@shundhammer
Copy link
Owner

Yes, it's intentional that it doesn't always look like this. Having such a "mixed" selection is typically a very exceptional situation, thus the extra highlighting.

@krbvroc1
Copy link

@shundhammer
Just installed this tool and was cleaning up (multiple selections using Ctrl-click) under my /home directory. Entire home directory was blown away. Since I had multi-selected > 30 subdirectory (like stuff under .cache and .local), I can't say I paid that close attention and lost everything (Restoring from backups now).

You mentioned something above about the next selection will deselect the parent directory. I am wondering if this is not the case when using Ctrl-click - it just adds it to the existing selection.

@shundhammer
Copy link
Owner

@krbvroc1 : Let me check this.

@krbvroc1
Copy link

The log file got erased with my home directory, but I did scroll back and noticed the first item was a directory and the second was the top level parent (/home/user) and then the remaining in the list were other subdirectories.

@shundhammer
Copy link
Owner

@krbvroc1 : I just checked: After selecting a couple of directories and deleting them in QDirStat, their parent directory is selected - just that one, nothing else.

Now if you got into the habit of always ctrl-clicking items even after the first such cleanup action, it might be possible that you just added to that selection which means that the parent directory (in your example: your home directory) remained selected and you just added more directories to delete. The next delete will then of course not only delete those subdirectories, it will also nuke the parent directory.

I do not say that this is what must have happened, but it is possible. Does this seem plausible to you?

@shundhammer
Copy link
Owner

The log normally resides in /tmp/qdirstat-$username . Is that gone as well?

@krbvroc1
Copy link

That log is still there. And yes I agree I think 99% chance of your suggeetion. I was cleaning up in my homedir in batches. After I did maybe 20-30 subdirectories I continued to the next group of 20-30. I never even though a parent selection would happen.. I am sure at some point I used Ctrl click even on first selection.

@shundhammer
Copy link
Owner

shundhammer commented Dec 21, 2017

Okay then. I don't think this is something that can be effectively prevented without hurting the user in other ways; ctrl-clicking to extend a selection is an important action in a tool like this. I really wouldn't like to restrict that for this kind of scenario.

Lessons learned:

  • Be careful when deleting things -- yes, of course you know that, but still ;-)

  • Do read the confirmation messages, even if it is inconvenient. Yes, I know, easier said than done. ;-)

  • Prefer deleting to trash over direct delete; unlike the direct delete, this can easily be undone. In particular for large cleanup actions this is definitely useful since things can easily get out of hand for many other reasons (most prominently the user getting confused - happens to me all the time), and the trash bin is a handy safety net for that. With QDirStat, you don't run the risk of getting huge directory trees being copied all over the place just for the sake of moving them to the trash bin; QDirStat does not do that. If it cannot be moved directly, you will simply get an error, and then you will have to make a decision how to proceed. So using the trash bin is also a quick action in QDirStat that will never use more disk space on your filesystem than before deleting to trash.

For a moment I considered a more explicit check for this situation: E.g. if the selection looks suspicious, i.e. items and their parents are selected, then maybe issue another warning. The problem is just that in many cases this might be an absolute legitimate thing to do, and it should clearly be limited to really dangerous cleanup actions.

Let me think about this some more; maybe that can integrate nicely into everything without becoming a major nuissance for most users in most cases. I don't promise anything, but sometimes good ideas come along after putting things on the backburner for some time.

@krbvroc1
Copy link

I am not familiar with the code but after a deletion what causes the parent to be selected and why is that a useful/good thing? In my thinking it is a flaw to select something the user did not select. I didn't take 'precautions' because I felt confident about what my fingers had clicked. I just finished restoring from backup - luckily 'only' a day of work lost...I will try again using some of your suggestions.

Thanks for the quick response too.

@shundhammer
Copy link
Owner

This is what this GitHub issue was originally all about - read my very first comment here for the reasoning: If nothing is selected after such a cleanup action, you are completely thrown off, you lose your context, you lose the keyboard focus; you have to manually navigate back to where your last action took place -- if you can even find it again. This was the state of affairs just after the Great Big Rewrite (i.e. when KDirStat became QDirStat), and it was confusing to the point of being barely usable.

@shundhammer
Copy link
Owner

Notice that this is only ever a problem for this very scenario: If you just always ctrl-click, no matter what. If you simply click normally (i.e. without ctrl), you won't ever notice the difference; it just comes naturally.

@krbvroc1
Copy link

I guess there is no way to save the position of the parent item and just move to that item in the qt list (move rather than select)? Alternatively, could you select the parent so it moves the view to that item and then deselect it. I would think that would handle both scenarios...Not losing your place but also not having something selected the user did not request. Just two suggestions I can think of.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants