Refactor FileList in FileCommander

Issue #657 resolved
prl created an issue

FileList and MultiFileSelectList in FileCommander.FileListmod are largely the same as their corresponding classes in Screens, but have "inherited" from them by copy-paste-modify, leading to a lot of redundant code and the FileCommander not keeping up with changes to the corresponding classes in the main code. For example, there are a number of EXTENSIONS entries for music, picture and movie file extensions that are in Compionents.FileList, but are not in FileCommander.FileListmod.

The substantive differences between FileCommander.FileListmod the corresponding main code module are:

  • Different definitions of FileEntryComponent() and MultiFileSelectEntryComponent().
  • Substantial differences in changeDir() in both FileList and MultiFileSelectList to the corresponding main code.
  • Substantial differences in MultiFileSelectList.changeSelectionState().
  • Differences in MultiFileSelectList.getSelectedList().
  • Added methods getParentDirectory() and getSelectionID() in MultiFileSelectList.

The refactor should inherit from Component.FileList classes and only override different functions and methods. It should also construct its own EXTENSIONS dict by copying from Component.FileList.EXTENSIONS and adding its own additional extension-to-file-type mappings.

There is also some code cleanup that would be useful: using resolveFilename(SCOPE_PLUGINS, ...) to create plugin-private pixmap pathnames instead of fixed pathnames, changing some if statements to use Boolean variables instead and use the same import conventions for importing from os and os.path as the main code (so their differences can be more easily compared), and remove unused imports.

The refactor of FileCommander.FileListmod was motivated by similar patterns of "inheritance copy-paste" noted in issue #656.

Comments (7)

  1. Peter Urbanec

    Fix issue #657: Refactor FileList in FileCommander

    Rebase Plugins.Extensions.FileCommander.FileListmod from Components.MenuList to Components.FileList and eliminate duplicate of functionally equivalent methods from FileListmod.

    Construct FileCommander.FileListmod.EXTENSIONS from the media extensions in Components.FileList and additional extension types specific to FileCommander.FileListmod.

    Components.FileList holds self.matchingPattern as a compiled RE, while FileCommander.FileListmod held it as a string, so remove RE compilation method calls when self.matchingPattern is used.

    Remove now-unused imports.

    → <<cset b4891d845a3d>>

  2. Peter Urbanec

    Fix issue #657: Refactor FileList in FileCommander - import cleanup

    Change import conventions for os and os.path to match those in Components.FileList to make comparison of FileCommander.FileListmod and Components.FileList simpler.

    → <<cset 2682e80c40b6>>

  3. Peter Urbanec

    Fix issue #657: Refactor FileList in FileCommander - image load cleanup

    Load images relative to SCOPE_PLUGINS instead of from embedded paths.

    Use the default for cached (use cache) setting in LoadPixmap instead of setting it explicitly.

    Use os.path.join() to join image path components.

    → <<cset 001e5fb45b41>>

  4. Peter Urbanec

    Fix issue #657: Refactor FileList in FileCommander - getParentDir() fixes

    Implement the setting of self.parent_directory in FileCommander.MultiFileSelectList so that MultiFileSelectList.getParentDir() works correctly.

    Always set self.parent_directory to None when it is invalid (not sometimes None and sometimes False).

    Change FileCommanderScreen to test for the new invalid value.

    This is necessary to test the next step, and to allow the implementation of navigation in MultiFileSelectList if that is wanted at a later time.

    This is not possible to test without modifying FileCommanderScreen, but it has been tested that way.

    → <<cset d9541f403b3a>>

  5. Peter Urbanec

    Fix issue #657: Refactor FileList in FileCommander - FileListmod logic cleanup

    Clean up some of the logic in FileListmod:

    Replace if statements with the use of Boolean variables or test results and changing some explicit looping tests to x in list tests.

    Fix the logic of alreadySelected in the if self.showFiles part of MultiFileSelectList.changeDir() so that full pathnames are compared instead of basenames.

    Reorder & simplfy isDir/isLink tests in FileEntryComponent() and MultiFileSelectEntryComponent().

    This is not possible to test the changes in MultiFileSelectList.changeDir() that test for alreadySelected without modifying FileCommanderScreen, but it has been tested that way.

    → <<cset 362845a53a50>>

  6. Peter Urbanec

    Fix issue #657: Refactor FileList in FileCommander - module name cleanup

    Rename module Plugins.Extensions.FileCommander.FileListmod to Plugins.Extensions.FileCommander.FileList.

    → <<cset 2856f6171bbd>>

  7. Log in to comment