[FEATURE] Cleanup of overrides that no longer need to exist

[OdatNurd]

Summary

There should be a command which would perform a check similar to a Bulk Diff of all overrides in a single or all packages (possibly implemented via those commands or having common code factored out) to find all overrides that don't have any differences from the underlying file so that they can be cleaned up.

Optionally via a configuration option, this would be able to remove any folders in the Packages directory that become empty as a result of removing files from them. Any folders that were already empty would be left alone under the theory that we don't know why they're there and they might be important to the package in question.

While determining what files should be cleaned up, this should skip over any unpacked package folders that are symlinks or contain a control file/directory that indicates that they're under source control (e.g. .svn, .hg, .git) as in those cases these folders may be part of something external being used for overrides such as someone working on changes to the default packages.

What gets skipped over should be configurable so that it can be tweaked to the user's preferences. It may also be interesting/useful to have a counter option that allows you to include such a folder even if it would otherwise be ignored.

Questions

Should this use a diff to determine if there are any differences, or calculate a CRC32 and compare it to the version in the sublime-package file to ensure that the file is fully identical, or possibly be configured one way or the other?

It may or may not be possible for some plugin file to be overridden and be e.g. sensitive to the line endings used, which would not be compared via a simple diff.

---

For something that is going to have such repercussions (removing multiple files and folders) it doesn't seem right to just go ahead and remove the files without confirming with the user. Additionally the list could easily be so large as to make a confirmation dialog unreadably large if it contained all of the files.

To this end the command could potentially generate a new style of report that would show you in context exactly what would be removed (much like the OverrideList currently does) and either offer a context sensitive command via the menu/context menu/command palette to "commit" to the action or trigger a Yes/No/Cancel dialog when you close it (note: I'm not sure if you can cancel a close that's in progress so that may or may not be doable).

[keith-hall]

I wonder if calculating a CRC32 checksum would be less resource intensive than calculating the diff - because really, the diff itself isn't necessary in this case, just to have a mechanism to know that it's unchanged is enough. If so, I'd vote for the CRC32, with no need to make it configurable...

Good thinking about the confirmation dialog becoming unreadably large, but the problem I could foresee with another report, would be that the overrides or packed-package contents could change meanwhile, since the report was generated, so we'd need to think about how to tackle that...

[OdatNurd]

Offhand I think it would be less resource intensive, yes. The diff has to keep both files in memory, calculate the sequence of changes, and then collate that into the final unified diff, which is at least more memory intensive if not more processor intensive as well.

The thing that made me consider that as a configurable item is that there is a (longer-term) plan of expanding on how the diffs currently work so that they could treat lines as similar if the only changes are leading/trailing white space (per extension).

In such a case it may or may not be beneficial to be able to consider some files as identical when their CRC's would not be. The case that comes to mind is package source files that use tabs for indenting being opened and turned into overrides by people that use spaces for indenting. In such cases the diffs as they stand are harder to decipher directly.

Of course, I also like the idea of making the user vet such files to be absolutely certain that they're identical, which would be a lot easier if the diff was able to deal with that situation better. It's also likely an issue that could be covered at a later time as well.

[OdatNurd]

I hadn't thought of the wrinkle of changes happening in the background; that does make things a little sticky. Wouldn't that still be the case even if there was a confirmation dialog?

[keith-hall]

I guess it could still be the case even for a confirmation dialog - I guess I was thinking most people interact with those much more instantly, but that may not always be true.

Ah I see, good idea about whitespace, in which case maybe just always go for a diff instead of a CRC to keep the implementation easier to maintain?

[OdatNurd]

I'm not sure where that might fall in the implementation order list since it seems to require more or less writing a custom context diff calculation.

I guess without whitespace insensitive diffing, CRC's are going to think the file is different and so are regular diffs, so at the moment either way is still going to make someone unhappy.

If it was just always diff, later when the ability to diff w/regard to whitespace changes is implemented it could be configurable if you wanted to use that sort of diff where possible or only a more "exact" diff, just to give that extra level of protection to people that might want the assurance that only truly identical files are being automatically wiped away?