Wrye-bash: List files in Clean Data & Sync From Data

[Utumno]

See: https://sourceforge.net/p/oblivionworks/enhancements/162/, https://sourceforge.net/p/oblivionworks/enhancements/113/, https://sourceforge.net/p/oblivionworks/enhancements/220/

Do see those for ideas etc

At least prompt before sync from data - quoting @saebel:

Data sync suggestion: don't just list the number of files that are going to be updated/deleted, show the file names. I accidentally agreed to a data sync that deleted files that it shouldn't have and would have caught had I seen the file names. Thank goodness I had made a backup.

From 162:

basically the "Clean data" command, but just gathering the file names and listing them instead of moving them.

From 220:

It would be nice to have a preview of the files that will be removed from the data folder when 'Clean Data' is executed from the Installers tab.
Possibly with an option to either once or permanently exclude some files from cleaning.

[Utumno]

@saebel - http://forums.bethsoft.com/topic/1599816-wrye-bash-thread-108/page-2#entry25116148

[warmfrost85]

@Infernio & @Utumno : This commit on today's nightly branch handles this need. It displays a sorted list of untracked files using checkboxes.
https://github.com/warmfrost85/wrye-bash/commit/67296957de2151e2a5801806fcd05957e95bb540
_It's based off nightly rather than dev because I'm using python 64 bit and haven't setup a virtual environment._

[Infernio]

Thanks! Left a preliminary review (code style only) on your commit.

[warmfrost85]

Pushed an update with requested changes. Probably should be squished later but preserving history now.
https://github.com/warmfrost85/wrye-bash/commit/3ad90a5b991154ffea04f521d21c255b104c84c8

[Infernio]

Left a few more nits, mostly line wrapping. Do you have your editor configured to show a 79-chars boundary? Very useful when writing WB code.

[warmfrost85]

I'm using PyCharm but was set to show a 120-char boundary so didn't see that when I scrolled. I've set it to 79 and now see all the offenders. I also see a lot of existing code exceeding that boundary. Those must be legacy and slowly be reworked.

_I personally think sometimes going a little beyond a 79 char boundary leads to more readable code or might incentivize using less descriptive names. For example, in bain.py (nightly branch) line's 2647 & 2648 exceed 79 chars and is more readable as-is IMO than wrapping._

        showInactive = conflicts_mode and bass.settings['bash.installers.conflictsReport.showInactive']
        showLower = conflicts_mode and bass.settings['bash.installers.conflictsReport.showLower']

However that's not the consensus in the python community and having said that I'll fix it to standards and not mention it again. :)

I see I didn't study the code enough to notice I could use self._askOK or better self._showOK and the other things you mentioned. Thanks for the insight and feedback @Infernio .

[Infernio]

Yup, code that goes beyond 79 is generally old. New code should always stick to PEP8.
And yeah, sometimes the 79 thing gets a bit unwieldy. I do find it to be worth it though - makes git diffs much easier to read :)

The bain example looks OK, but I think this alternative which fits into 79 chars isn't that much worse (the symmetry of having the string line up is actually pretty nice):

        showInactive = conflicts_mode and bass.settings[
            'bash.installers.conflictsReport.showInactive']
        showLower = conflicts_mode and bass.settings[
            'bash.installers.conflictsReport.showLower']
        showBSA = bass.settings[
            'bash.installers.conflictsReport.showBSAConflicts']

Anyway, yes this is off-topic chatter :)

[warmfrost85]

This commit should met your request. It needs to be squashed later.
https://github.com/warmfrost85/wrye-bash/commit/2c2c66bf1c8b10fda5515d0ebc1766f4eee8c1a5
I did not translate the new strings in the ListBox. When I ran Settings/Dump Translator I got:

Traceback (most recent call last):
  File "bash\balt.py", line 1793, in __Execute
    self.Execute()
  File "bash\basher\settings_links.py", line 498, in Execute
    outFile = dump_translator(outPath.s, bass.active_locale)
  File "bash\localize.py", line 177, in dump_translator
    args.extend(_find_all_bash_modules())
  File "bash\localize.py", line 161, in _find_all_bash_modules
    os.path.join(cur_dir, p[1]))
  File "bash\localize.py", line 151, in _find_all_bash_modules
    _files.extend([bash_path.join(m).s for m in os.listdir(cur_dir)
AttributeError: 'unicode' object has no attribute 'extend'

I assume there is something wrong with my local setup but stopped going down that rabbit hole after investigating a while. Does Dump Translator work off nightly for you?
I'm using: chardet in c:\python27lib\site-packages (3.0.4)

[Infernio]

No need to actually translate the strings, just surround them with an _() call. Dump Translator might well be broken, I'll check it out.

[Infernio]

Style looks fine now :)
When I run this on my Oblivion install, I get this result:

The two Docs results are in there because of a bug I fixed in c6575403d35126a40e0c83b63aaa55b91d8c10db (rebase on latest nightly to get it), but the remainder aren't actually valid results. They get skipped inside clean_data_dir:

https://github.com/wrye-bash/wrye-bash/blob/10e680cbb347b203f935c042145a5fc308b5f4c8/Mopy/bash/bosh/bain.py#L2557-L2560
https://github.com/wrye-bash/wrye-bash/blob/10e680cbb347b203f935c042145a5fc308b5f4c8/Mopy/bash/bosh/bain.py#L2565-L2566

So moving that part up into get_clean_data_dir_list would be a good idea, to not show the user any bogus results.

[warmfrost85]

Rebased on latest nightly, updated and ready for review.
https://github.com/warmfrost85/wrye-bash/commit/a30ab8bc8be4ff342b205d0a0fd12e657d113802

[Infernio]

We keep Bashed Patch docs for all BPs that are in the Data folder. So if you have a Bashed Patch, 0.esp and a Bashed Patch, Tweaks only.esp, only those docs will be kept.
Probably fine behavior, since you can't actually predict what the BP name is going to be (matching a regex like Bashed Patch.+\.esp is not going to do - the only thing identifying a BP is the Author field in the header, so you could name your BP something like My Cool BP.esp).

[warmfrost85]

I noticed my error after I posted that comment so I edited and removed. Bad practice to remove part of a comment once published since your comment now refers to "nothing". My bad. :( Next time I'll show EDIT: instead.

[Infernio]

Was just wondering why it was trying to remove ArchiveInvalidationInvalidated!.bsa in FO3, a file that comes with WB - turns out it was a typo:

diff --git a/Mopy/bash/game/fallout3/__init__.py b/Mopy/bash/game/fallout3/__init__.py
index c94abc3f1..f9a0d9396 100644
--- a/Mopy/bash/game/fallout3/__init__.py
+++ b/Mopy/bash/game/fallout3/__init__.py
@@ -106,7 +106,7 @@ class Xe(GameInfo.Xe):
     }
     SkipBAINRefresh = {u'fo3edit backups', u'fo3edit cache'}
     wryeBashDataFiles = GameInfo.wryeBashDataFiles | {
-        u'ArchiveInvalidationInvalidated!.bsa'
+        u'ArchiveInvalidationInvalidated!.bsa',
         u'Fallout - AI!.bsa'
     }
     ignoreDataFiles = {

You could include that in your commit as a quick fixup :)

[warmfrost85]

Nice catch. Technically that change is not directly related to the Clean Data code. It's a result of Clean Data now displaying files to the user. However I can make that change and include it in the commit if you want.

[Infernio]

You can include it and add a small note like this in the commit msg:

Mopy/bash/game/fallout3/__init__.py:
Tiny fixup, was cleaning out ArchiveInvalidationInvalidated!.bsa

That's generally what we do if we have a tiny commit that doesn't have much value on its own.

Edit: you can see an example here: 5c7772a0d297786ac0e8357014abbd9c6ae3f1c3

[warmfrost85]

Done. https://github.com/warmfrost85/wrye-bash/commit/387fe05c51d1c4bda5274dd8bb00ea9ba7a3b7f8

[Infernio]

Did you see my comments I left on https://github.com/warmfrost85/wrye-bash/commit/a30ab8bc8be4ff342b205d0a0fd12e657d113802? Once those are addressed, then I think this is ready for testing - I'll pull it over and stick it on nightly/307-beta6 here :)

[Utumno]

Let me do that - although turns out the difficult part is this: https://bugs.python.org/issue7195

[warmfrost85]

Sorry, I missed your comments. Now updated. Is there such a thing as Done. Done. ? :)
https://github.com/warmfrost85/wrye-bash/commit/4abca0acc9a848a3858c3aad1bff7973e785438d

[Infernio]

@Utumno Do you have your BP docs on a different drive? If so, checking if the Data folder is part of the BP doc path should be enough, right? If something isn't in the data folder, then BAIN has no business trying to clean it.

[Utumno]

Yes - good one :)

@@ -2590,2 +2590,3 @@ def get_clean_data_dir_list(self):
             if bp_doc: # path is absolute, convert to relative to the Data/ dir
+                print(u'%s %s' % (bp_doc,bass.dirs['mods'].s))
                 bp_doc = bp_doc.relpath(bass.dirs['mods'].s)


D:\GAMES\TESIV\Oblivion\Data\Docs\PBASH_refactor_307 on 312-patchers.html D:\GAMES\TESIV\Oblivion\Data
C:\GAMES\TESIV\Oblivion\Data\Docs\Bashed Patch, 2.txt D:\GAMES\TESIV\Oblivion\Data

What's even stranger though is that when I run pycharm debugger it will silently fail - won't even show the dialog. A heisenbug ? :D