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

Shredder: Can't seem to make "Render script from Selected" work with partial selections #375

Open
ferdnyc opened this issue Jul 29, 2019 · 6 comments

Comments

@ferdnyc
Copy link

ferdnyc commented Jul 29, 2019

I have a two-directory compare that I ran with the following settings:

  • Must match tagged
  • Keep all tagged
  • Remove duplicates
  • Do not cross mount points
  • No path limitations
  • Traversal depth: 6
  • Size limits: 1 MB to 8 GB

Then I tagged one of the two trees (my master copy), selected both trees (master copy and working copy), and let the comparison run. It returned a large set of duplicates, as expected, and automatically marked everything in the working copy to delete, that's also present and marked to preserve in the master copy. Again, as expected, and all good so far.

If I "Render script from All", no problems, I get a script that'll delete every dup in the working copy.

But I don't necessarily want to do that. I want to pick and choose some parts of the working copy to delete. And that's the part I can't seem to make work.

No matter how I make a partial selection, the resulting script comes up with the AUTOGENERATED OUTPUT section completely empty. I've tried:

  • Selecting a collapsed directory in the working (delete) tree
  • Selecting that collapsed directory, and the corresponding collapsed directory in the master (preserve) tree that contains the same set of files
  • Opening the collapsed working (delete) tree directory and selecting individual files
  • Doing the above, and also selecting the same individual files in the master (preserve) tree
  • Selecting the directory heading AND its individual file contents, in the delete tree
  • Selecting the (matching) directory heading AND individual file contents in both trees

And so on. Nothing works, unless I select the ENTIRE results tree, and then I get the exact same script that "Render script from All" would produce. Even if I then deselect some files/directories, following a select-all, those files are still included in the generated script, same as if I'd just done "Render script from All". (This is presumably because the very root of the tree is still selected — but whenever that root node is not selected, I get empty scripts.)

At least with the comparison settings and directory selections I'm working with, it's not clear to me how "Render script from Selected" can be convinced to do anything at all.

@sahib sahib added the bug label Jul 31, 2019
@sahib
Copy link
Owner

sahib commented Jul 31, 2019

It should also work when you select the root node and generate from that. Or when you generate from a directory that is self-contained, i.e., all originals and duplicates are in it.

Anyways, there was some buggy behavior mixed with surprises. Here's what's happening: When you click Generate from Selected shredder should go over all nodes recursively that you selected and build a map out of that. It only selected the first selected node though due to a mistake on my side. This should be fixed in deaa2fd - please test this.

@ferdnyc
Copy link
Author

ferdnyc commented Jul 31, 2019

It should also work when you select the root node and generate from that.

Oh, it did, but that's just "Render script from All", so I already had a way to do that. 😄

This should be fixed in deaa2fd - please test this.

Sure will, re-running that compare now. Thanks!

@ferdnyc
Copy link
Author

ferdnyc commented Jul 31, 2019

Things are greatly improved with the new version! I can now make partial selections of individual files or directories, and generate a script processing only those files. Thanks for the quick attention on that!

There are still two areas where things are a little different than might be expected, though they're not critical at all compared to the basic selected-items functionality which is now working well.

  1. When making positive selections, in the setup I described in my OP (where I compare two completely separate trees, with one tagged as the master copy and the "Must match tagged" / "Keep all tagged" options enabled), in order to get files included in the script it's necessary to select both the file(s)/folder(s) in the to-be-deleted tree, and their corresponding matches in the tagged (keep) tree.

    It'd be convenient if selecting only the to-be-deleted matches, at least, automatically implied the selection of their "keep" counterparts as well, and still processed the selected files for deletion. But, I can understand how any sort of implied selections, especially in a tool like rmlint, would have implications that might not be desirable, so maybe the current functionality is preferred as the safer option.

  2. When making negative selections, deselecting a file/folder subtree after performing a select-all DOESN'T remove those files from the script if any of their parents are still selected.

    This one strikes me as more of an actual undesirable-behavior thing, with the potential to surprise users with scripts that perform actions they weren't intending. (In fact, had explicitly intended not to include.) I'd think that, if you explicitly deselect anything, that should override any implicit selection it would inherit from its a parent node(s).

Other than those two aspects, everything seems to be working exactly as expected. I'll play with it a bit more just to see if I encounter any other unexpected results. Thanks again!

@sahib
Copy link
Owner

sahib commented Jul 31, 2019

Good, good. To the other points:

  1. I think that's to be expected (well, I'm probably biased here since I'm not the most typical rmlint user), but you only get what you selected - with the caveat in point 2. That's also analogous to the --replay feature working behind the scene for that.

  2. I'm not sure if I got that right, so beware. When you deselect a folder, but have a folder above selected it will still be in the results. That's because the selection is determined recursively and this kind of makes sense to me. I don't want to select all items (which might be a huge number), but just the directories I'm interested in. Since we don't really have documentation on that I kind of agree though that expectations and reality can differ here.

@ferdnyc
Copy link
Author

ferdnyc commented Aug 1, 2019

  1. I think that's to be expected (well, I'm probably biased here since I'm not the most typical rmlint user), but you only get what you selected - with the caveat in point 2. That's also analogous to the --replay feature working behind the scene for that.

Fair enough.

  1. I'm not sure if I got that right, so beware. When you deselect a folder, but have a folder above selected it will still be in the results.

Not just a folder, but individual files as well. Deselect them, when they were previously selected, and if any of their parents are selected they'll still be "stealth" included in the selection. (I mean, it's not "stealth" in the sense that the parents are still visibly selected, but... it's at the very least counterintuitive. Like I said, you explicitly deselect something, you expect it's removed from the selection.

That's because the selection is determined recursively and this kind of makes sense to me.

From an implementation standpoint, I can see that. But I've thought about it a bunch more and from a UX standpoint I just can't agree. It's problematic both practically and theoretically/philosophically:

  1. On a practical level, one problem is that it makes "Select All" completely useless. You never actually need to "Select All", because you can just select the root node and everything under it will be selected. So, the only reason you'd perform a "Select All" is to immediately follow it by deselecting some of the items, because you want to create a mostly-inclusive partial selection.

    But, because of the parents' recursive selection overriding the individual item state, deselection of items is useless unless you're then willing to walk all the way up the tree and also manually deselect every parent as well.

  2. And that's the theoretical hurdle: Recursive container selection combined with the ability to select both container and children means that, effectively, an individual item can be selected multiple times — in fact, it can be redundantly selected as many times as it is levels deep in the tree. Which isn't ever what the user actually means, or intends to have happen.

    Just for the sake of comparing to another familiar implementation, Microsoft solved this on Windows with the tri-state checkbox. Anytime there are selectable items in a tree structure, they'll be enabled/disabled using checkboxes on each row. Checking a container row automatically checks all of its children, and the container remains checked only while ALL of its children are checked — uncheck any one of them, and the container's checkbox (and its container's, and so on and so on, potentially) reverts to a box containing a horizontal line, denoting a mix of checked and unchecked contents.

  3. This also leads to inconsistent behavior in other parts of the interface which don't implement the same form of recursive selection. For instance, if you select a folder and choose "Toggle selected" from the context menu, nothing happens. For consistency with other effects from folder selection, one would expect all of its contents to be toggled, but they're not.

Not to imply that the selection model, or the issues with it, are unique to rmlint, BTW. One place you'll find the same UX is in Nautilus' List View mode. If the "Allow folders to be expanded" preference is enabled, it turns Nautilus' list view into an expandable tree much like Shredder's. And it's the source of some pretty buggy behavior in Nautilus, too.

  • It's possible to select both a folder and some or all of its contents, separately but together.

  • If you select both a folder row and a file within that folder, then copy them somewhere else, Nautilus totally will copy that same file twice, once inside the folder and then a second time outside (alongside) it, even though it's hard to imagine that ever being what the user really intended to have happen.

  • But just in case anyone wanted to consider it intended/desirable behavior, or to defend it as "doing what the user asked", Nautilus makes sure it manifests as a bug by messing up its own progress meter for the copy operation. The meter will terminate prematurely because it fails to account for the second copy of the file.

    So it'll reach 100% when the folder finishes copying, and the second file copy will occur silently in the background. If that file's size is enough to fill up the disk, I imagine that would also take Nautilus by surprise and fail to trigger its normal warnings about available space for the copy operation.

I apologize for belaboring what feels like it should be a minor point of GUI implementation. But I know that every aspect of rmlint's filesystem operation was carefully considered, to the point that it was the topic of an entire treatise on the subject. (A document to which I still refer anyone who casually suggests adding "simple" file-syncing functionality to some network service or whatever. Because assuming you don't want to end up accidentally losing your users' data, there is no such thing as "simple" file syncing.)

@sahib
Copy link
Owner

sahib commented Aug 5, 2019

Long story short:
Would it help to make a row selection also (visually) select all children in case of directories?

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

No branches or pull requests

3 participants