Merge pull request #705 from glubsy/exclude_list

Add Exclusion Filters

core/app.py

@@ -26,11 +26,13 @@ from .pe.photo import get_delta_dimensions
 from .util import cmp_value, fix_surrogate_encoding
 from . import directories, results, export, fs, prioritize
 from .ignore import IgnoreList
+from .exclude import ExcludeDict as ExcludeList
 from .scanner import ScanType
 from .gui.deletion_options import DeletionOptions
 from .gui.details_panel import DetailsPanel
 from .gui.directory_tree import DirectoryTree
 from .gui.ignore_list_dialog import IgnoreListDialog
+from .gui.exclude_list_dialog import ExcludeListDialogCore
 from .gui.problem_dialog import ProblemDialog
 from .gui.stats_label import StatsLabel
 
@@ -137,7 +139,8 @@ class DupeGuru(Broadcaster):
             os.makedirs(self.appdata)
         self.app_mode = AppMode.Standard
         self.discarded_file_count = 0
-        self.directories = directories.Directories()
+        self.exclude_list = ExcludeList()
+        self.directories = directories.Directories(self.exclude_list)
         self.results = results.Results(self)
         self.ignore_list = IgnoreList()
         # In addition to "app-level" options, this dictionary also holds options that will be
@@ -155,6 +158,7 @@ class DupeGuru(Broadcaster):
         self.directory_tree = DirectoryTree(self)
         self.problem_dialog = ProblemDialog(self)
         self.ignore_list_dialog = IgnoreListDialog(self)
+        self.exclude_list_dialog = ExcludeListDialogCore(self)
         self.stats_label = StatsLabel(self)
         self.result_table = None
         self.deletion_options = DeletionOptions()
@@ -587,6 +591,9 @@ class DupeGuru(Broadcaster):
         p = op.join(self.appdata, "ignore_list.xml")
         self.ignore_list.load_from_xml(p)
         self.ignore_list_dialog.refresh()
+        p = op.join(self.appdata, "exclude_list.xml")
+        self.exclude_list.load_from_xml(p)
+        self.exclude_list_dialog.refresh()
 
     def load_directories(self, filepath):
         # Clear out previous entries
@@ -779,6 +786,8 @@ class DupeGuru(Broadcaster):
         self.directories.save_to_file(op.join(self.appdata, "last_directories.xml"))
         p = op.join(self.appdata, "ignore_list.xml")
         self.ignore_list.save_to_xml(p)
+        p = op.join(self.appdata, "exclude_list.xml")
+        self.exclude_list.save_to_xml(p)
         self.notify("save_session")
 
     def save_as(self, filename):

core/directories.py

@@ -54,10 +54,11 @@ class Directories:
     """
 
     # ---Override
-    def __init__(self):
+    def __init__(self, exclude_list=None):
         self._dirs = []
         # {path: state}
         self.states = {}
+        self._exclude_list = exclude_list
 
     def __contains__(self, path):
         for p in self._dirs:
@@ -76,39 +77,62 @@ class Directories:
 
     # ---Private
     def _default_state_for_path(self, path):
+        # New logic with regex filters
+        if self._exclude_list is not None and self._exclude_list.mark_count > 0:
+            # We iterate even if we only have one item here
+            for denied_path_re in self._exclude_list.compiled:
+                if denied_path_re.match(str(path.name)):
+                    return DirectoryState.Excluded
+            # return # We still use the old logic to force state on hidden dirs
         # Override this in subclasses to specify the state of some special folders.
-        if path.name.startswith("."):  # hidden
+        if path.name.startswith("."):
             return DirectoryState.Excluded
 
     def _get_files(self, from_path, fileclasses, j):
         for root, dirs, files in os.walk(str(from_path)):
             j.check_if_cancelled()
-            root = Path(root)
-            state = self.get_state(root)
+            rootPath = Path(root)
+            state = self.get_state(rootPath)
             if state == DirectoryState.Excluded:
                 # Recursively get files from folders with lots of subfolder is expensive. However, there
                 # might be a subfolder in this path that is not excluded. What we want to do is to skim
                 # through self.states and see if we must continue, or we can stop right here to save time
-                if not any(p[: len(root)] == root for p in self.states):
+                if not any(p[: len(rootPath)] == rootPath for p in self.states):
                     del dirs[:]
             try:
                 if state != DirectoryState.Excluded:
-                    found_files = [
-                        fs.get_file(root + f, fileclasses=fileclasses) for f in files
-                    ]
+                    # Old logic
+                    if self._exclude_list is None or not self._exclude_list.mark_count:
+                        found_files = [fs.get_file(rootPath + f, fileclasses=fileclasses) for f in files]
+                    else:
+                        found_files = []
+                        # print(f"len of files: {len(files)} {files}")
+                        for f in files:
+                            found = False
+                            for expr in self._exclude_list.compiled_files:
+                                if expr.match(f):
+                                    found = True
+                                    break
+                            if not found:
+                                for expr in self._exclude_list.compiled_paths:
+                                    if expr.match(root + os.sep + f):
+                                        found = True
+                                        break
+                            if not found:
+                                found_files.append(fs.get_file(rootPath + f, fileclasses=fileclasses))
                     found_files = [f for f in found_files if f is not None]
                     # In some cases, directories can be considered as files by dupeGuru, which is
                     # why we have this line below. In fact, there only one case: Bundle files under
                     # OS X... In other situations, this forloop will do nothing.
                     for d in dirs[:]:
-                        f = fs.get_file(root + d, fileclasses=fileclasses)
+                        f = fs.get_file(rootPath + d, fileclasses=fileclasses)
                         if f is not None:
                             found_files.append(f)
                             dirs.remove(d)
                     logging.debug(
                         "Collected %d files in folder %s",
                         len(found_files),
-                        str(from_path),
+                        str(rootPath),
                     )
                     for file in found_files:
                         file.is_ref = state == DirectoryState.Reference
@@ -194,8 +218,14 @@ class Directories:
         if path in self.states:
             return self.states[path]
         state = self._default_state_for_path(path) or DirectoryState.Normal
+        # Save non-default states in cache, necessary for _get_files()
+        if state != DirectoryState.Normal:
+            self.states[path] = state
+            return state
+
         prevlen = 0
         # we loop through the states to find the longest matching prefix
+        # if the parent has a state in cache, return that state
         for p, s in self.states.items():
             if p.is_parent_of(path) and len(p) > prevlen:
                 prevlen = len(p)

core/exclude.py

@@ -0,0 +1,499 @@
+# This software is licensed under the "GPLv3" License as described in the "LICENSE" file,
+# which should be included with this package. The terms are also available at
+# http://www.gnu.org/licenses/gpl-3.0.html
+
+from .markable import Markable
+from xml.etree import ElementTree as ET
+# TODO: perhaps use regex module for better Unicode support? https://pypi.org/project/regex/
+# also https://pypi.org/project/re2/
+# TODO update the Result list with newly added regexes if possible
+import re
+from os import sep
+import logging
+import functools
+from hscommon.util import FileOrPath
+from hscommon.plat import ISWINDOWS
+import time
+
+default_regexes = [r"^thumbs\.db$",  # Obsolete after WindowsXP
+                   r"^desktop\.ini$",  # Windows metadata
+                   r"^\.DS_Store$",  # MacOS metadata
+                   r"^\.Trash\-.*",  # Linux trash directories
+                   r"^\$Recycle\.Bin$",  # Windows
+                   r"^\..*",  # Hidden files on Unix-like
+                   ]
+# These are too broad
+forbidden_regexes = [r".*", r"\/.*", r".*\/.*", r".*\\\\.*", r".*\..*"]
+
+
+def timer(func):
+    @functools.wraps(func)
+    def wrapper_timer(*args):
+        start = time.perf_counter_ns()
+        value = func(*args)
+        end = time.perf_counter_ns()
+        print(f"DEBUG: func {func.__name__!r} took {end - start} ns.")
+        return value
+    return wrapper_timer
+
+
+def memoize(func):
+    func.cache = dict()
+
+    @functools.wraps(func)
+    def _memoize(*args):
+        if args not in func.cache:
+            func.cache[args] = func(*args)
+        return func.cache[args]
+    return _memoize
+
+
+class AlreadyThereException(Exception):
+    """Expression already in the list"""
+    def __init__(self, arg="Expression is already in excluded list."):
+        super().__init__(arg)
+
+
+class ExcludeList(Markable):
+    """A list of lists holding regular expression strings and the compiled re.Pattern"""
+
+    # Used to filter out directories and files that we would rather avoid scanning.
+    # The list() class allows us to preserve item order without too much hassle.
+    # The downside is we have to compare strings every time we look for an item in the list
+    # since we use regex strings as keys.
+    # If _use_union is True, the compiled regexes will be combined into one single
+    # Pattern instead of separate Patterns which may or may not give better
+    # performance compared to looping through each Pattern individually.
+
+    # ---Override
+    def __init__(self, union_regex=True):
+        Markable.__init__(self)
+        self._use_union = union_regex
+        # list([str regex, bool iscompilable, re.error exception, Pattern compiled], ...)
+        self._excluded = []
+        self._excluded_compiled = set()
+        self._dirty = True
+
+    def __iter__(self):
+        """Iterate in order."""
+        for item in self._excluded:
+            regex = item[0]
+            yield self.is_marked(regex), regex
+
+    def __contains__(self, item):
+        return self.isExcluded(item)
+
+    def __len__(self):
+        """Returns the total number of regexes regardless of mark status."""
+        return len(self._excluded)
+
+    def __getitem__(self, key):
+        """Returns the list item corresponding to key."""
+        for item in self._excluded:
+            if item[0] == key:
+                return item
+        raise KeyError(f"Key {key} is not in exclusion list.")
+
+    def __setitem__(self, key, value):
+        # TODO if necessary
+        pass
+
+    def __delitem__(self, key):
+        # TODO if necessary
+        pass
+
+    def get_compiled(self, key):
+        """Returns the (precompiled) Pattern for key"""
+        return self.__getitem__(key)[3]
+
+    def is_markable(self, regex):
+        return self._is_markable(regex)
+
+    def _is_markable(self, regex):
+        """Return the cached result of "compilable" property"""
+        for item in self._excluded:
+            if item[0] == regex:
+                return item[1]
+        return False  # should not be necessary, the regex SHOULD be in there
+
+    def _did_mark(self, regex):
+        self._add_compiled(regex)
+
+    def _did_unmark(self, regex):
+        self._remove_compiled(regex)
+
+    def _add_compiled(self, regex):
+        self._dirty = True
+        if self._use_union:
+            return
+        for item in self._excluded:
+            # FIXME probably faster to just rebuild the set from the compiled instead of comparing strings
+            if item[0] == regex:
+                # no need to test if already present since it's a set()
+                self._excluded_compiled.add(item[3])
+                break
+
+    def _remove_compiled(self, regex):
+        self._dirty = True
+        if self._use_union:
+            return
+        for item in self._excluded_compiled:
+            if regex in item.pattern:
+                self._excluded_compiled.remove(item)
+                break
+
+    # @timer
+    @memoize
+    def _do_compile(self, expr):
+        try:
+            return re.compile(expr)
+        except Exception as e:
+            raise(e)
+
+    # @timer
+    # @memoize  # probably not worth memoizing this one if we memoize the above
+    def compile_re(self, regex):
+        compiled = None
+        try:
+            compiled = self._do_compile(regex)
+        except Exception as e:
+            return False, e, compiled
+        return True, None, compiled
+
+    def error(self, regex):
+        """Return the compilation error Exception for regex.
+        It should have a "msg" attr."""
+        for item in self._excluded:
+            if item[0] == regex:
+                return item[2]
+
+    def build_compiled_caches(self, union=False):
+        if not union:
+            self._cached_compiled_files =\
+                [x for x in self._excluded_compiled if not has_sep(x.pattern)]
+            self._cached_compiled_paths =\
+                [x for x in self._excluded_compiled if has_sep(x.pattern)]
+            return
+        marked_count = [x for marked, x in self if marked]
+        # If there is no item, the compiled Pattern will be '' and match everything!
+        if not marked_count:
+            self._cached_compiled_union_all = []
+            self._cached_compiled_union_files = []
+            self._cached_compiled_union_paths = []
+        else:
+            # HACK returned as a tuple to get a free iterator and keep interface
+            # the same regardless of whether the client asked for union or not
+            self._cached_compiled_union_all =\
+                (re.compile('|'.join(marked_count)),)
+            files_marked = [x for x in marked_count if not has_sep(x)]
+            if not files_marked:
+                self._cached_compiled_union_files = tuple()
+            else:
+                self._cached_compiled_union_files =\
+                    (re.compile('|'.join(files_marked)),)
+            paths_marked = [x for x in marked_count if has_sep(x)]
+            if not paths_marked:
+                self._cached_compiled_union_paths = tuple()
+            else:
+                self._cached_compiled_union_paths =\
+                    (re.compile('|'.join(paths_marked)),)
+
+    @property
+    def compiled(self):
+        """Should be used by other classes to retrieve the up-to-date list of patterns."""
+        if self._use_union:
+            if self._dirty:
+                self.build_compiled_caches(True)
+                self._dirty = False
+            return self._cached_compiled_union_all
+        return self._excluded_compiled
+
+    @property
+    def compiled_files(self):
+        """When matching against filenames only, we probably won't be seeing any
+        directory separator, so we filter out regexes with os.sep in them.
+        The interface should be expected to be a generator, even if it returns only
+        one item (one Pattern in the union case)."""
+        if self._dirty:
+            self.build_compiled_caches(True if self._use_union else False)
+            self._dirty = False
+        return self._cached_compiled_union_files if self._use_union\
+            else self._cached_compiled_files
+
+    @property
+    def compiled_paths(self):
+        """Returns patterns with only separators in them, for more precise filtering."""
+        if self._dirty:
+            self.build_compiled_caches(True if self._use_union else False)
+            self._dirty = False
+        return self._cached_compiled_union_paths if self._use_union\
+            else self._cached_compiled_paths
+
+    # ---Public
+    def add(self, regex, forced=False):
+        """This interface should throw exceptions if there is an error during
+        regex compilation"""
+        if self.isExcluded(regex):
+            # This exception should never be ignored
+            raise AlreadyThereException()
+        if regex in forbidden_regexes:
+            raise Exception("Forbidden (dangerous) expression.")
+
+        iscompilable, exception, compiled = self.compile_re(regex)
+        if not iscompilable and not forced:
+            # This exception can be ignored, but taken into account
+            # to avoid adding to compiled set
+            raise exception
+        else:
+            self._do_add(regex, iscompilable, exception, compiled)
+
+    def _do_add(self, regex, iscompilable, exception, compiled):
+        # We need to insert at the top
+        self._excluded.insert(0, [regex, iscompilable, exception, compiled])
+
+    @property
+    def marked_count(self):
+        """Returns the number of marked regexes only."""
+        return len([x for marked, x in self if marked])
+
+    def isExcluded(self, regex):
+        for item in self._excluded:
+            if regex == item[0]:
+                return True
+        return False
+
+    def remove(self, regex):
+        for item in self._excluded:
+            if item[0] == regex:
+                self._excluded.remove(item)
+        self._remove_compiled(regex)
+
+    def rename(self, regex, newregex):
+        if regex == newregex:
+            return
+        found = False
+        was_marked = False
+        is_compilable = False
+        for item in self._excluded:
+            if item[0] == regex:
+                found = True
+                was_marked = self.is_marked(regex)
+                is_compilable, exception, compiled = self.compile_re(newregex)
+                # We overwrite the found entry
+                self._excluded[self._excluded.index(item)] =\
+                    [newregex, is_compilable, exception, compiled]
+                self._remove_compiled(regex)
+                break
+        if not found:
+            return
+        if is_compilable and was_marked:
+            # Not marked by default when added, add it back
+            self.mark(newregex)
+
+    # def change_index(self, regex, new_index):
+    # """Internal list must be a list, not dict."""
+    #     item = self._excluded.pop(regex)
+    #     self._excluded.insert(new_index, item)
+
+    def restore_defaults(self):
+        for _, regex in self:
+            if regex not in default_regexes:
+                self.unmark(regex)
+        for default_regex in default_regexes:
+            if not self.isExcluded(default_regex):
+                self.add(default_regex)
+            self.mark(default_regex)
+
+    def load_from_xml(self, infile):
+        """Loads the ignore list from a XML created with save_to_xml.
+
+        infile can be a file object or a filename.
+        """
+        try:
+            root = ET.parse(infile).getroot()
+        except Exception as e:
+            logging.warning(f"Error while loading {infile}: {e}")
+            self.restore_defaults()
+            return e
+
+        marked = set()
+        exclude_elems = (e for e in root if e.tag == "exclude")
+        for exclude_item in exclude_elems:
+            regex_string = exclude_item.get("regex")
+            if not regex_string:
+                continue
+            try:
+                # "forced" avoids compilation exceptions and adds anyway
+                self.add(regex_string, forced=True)
+            except AlreadyThereException:
+                logging.error(f"Regex \"{regex_string}\" \
+loaded from XML was already present in the list.")
+                continue
+            if exclude_item.get("marked") == "y":
+                marked.add(regex_string)
+
+        for item in marked:
+            self.mark(item)
+
+    def save_to_xml(self, outfile):
+        """Create a XML file that can be used by load_from_xml.
+        outfile can be a file object or a filename."""
+        root = ET.Element("exclude_list")
+        # reversed in order to keep order of entries when reloading from xml later
+        for item in reversed(self._excluded):
+            exclude_node = ET.SubElement(root, "exclude")
+            exclude_node.set("regex", str(item[0]))
+            exclude_node.set("marked", ("y" if self.is_marked(item[0]) else "n"))
+        tree = ET.ElementTree(root)
+        with FileOrPath(outfile, "wb") as fp:
+            tree.write(fp, encoding="utf-8")
+
+
+class ExcludeDict(ExcludeList):
+    """Exclusion list holding a set of regular expressions as keys, the compiled
+    Pattern, compilation error and compilable boolean as values."""
+    # Implemntation around a dictionary instead of a list, which implies
+    # to keep the index of each string-key as its sub-element and keep it updated
+    # whenever insert/remove is done.
+
+    def __init__(self, union_regex=False):
+        Markable.__init__(self)
+        self._use_union = union_regex
+        # { "regex string":
+        #   {
+        #       "index": int,
+        #       "compilable": bool,
+        #       "error": str,
+        #       "compiled": Pattern or None
+        #   }
+        # }
+        self._excluded = {}
+        self._excluded_compiled = set()
+        self._dirty = True
+
+    def __iter__(self):
+        """Iterate in order."""
+        for regex in ordered_keys(self._excluded):
+            yield self.is_marked(regex), regex
+
+    def __getitem__(self, key):
+        """Returns the dict item correponding to key"""
+        return self._excluded.__getitem__(key)
+
+    def get_compiled(self, key):
+        """Returns the compiled item for key"""
+        return self.__getitem__(key).get("compiled")
+
+    def is_markable(self, regex):
+        return self._is_markable(regex)
+
+    def _is_markable(self, regex):
+        """Return the cached result of "compilable" property"""
+        exists = self._excluded.get(regex)
+        if exists:
+            return exists.get("compilable")
+        return False
+
+    def _add_compiled(self, regex):
+        self._dirty = True
+        if self._use_union:
+            return
+        try:
+            self._excluded_compiled.add(self._excluded[regex]["compiled"])
+        except Exception as e:
+            logging.warning(f"Exception while adding regex {regex} to compiled set: {e}")
+            return
+
+    def is_compilable(self, regex):
+        """Returns the cached "compilable" value"""
+        return self._excluded[regex]["compilable"]
+
+    def error(self, regex):
+        """Return the compilation error message for regex string"""
+        return self._excluded.get(regex).get("error")
+
+    # ---Public
+    def _do_add(self, regex, iscompilable, exception, compiled):
+        # We always insert at the top, so index should be 0
+        # and other indices should be pushed by one
+        for value in self._excluded.values():
+            value["index"] += 1
+        self._excluded[regex] = {
+            "index": 0,
+            "compilable": iscompilable,
+            "error": exception,
+            "compiled": compiled
+        }
+
+    def isExcluded(self, regex):
+        if regex in self._excluded.keys():
+            return True
+        return False
+
+    def remove(self, regex):
+        old_value = self._excluded.pop(regex)
+        # Bring down all indices which where above it
+        index = old_value["index"]
+        if index == len(self._excluded) - 1:  # we start at 0...
+            # Old index was at the end, no need to update other indices
+            self._remove_compiled(regex)
+            return
+
+        for value in self._excluded.values():
+            if value.get("index") > old_value["index"]:
+                value["index"] -= 1
+        self._remove_compiled(regex)
+
+    def rename(self, regex, newregex):
+        if regex == newregex or regex not in self._excluded.keys():
+            return
+        was_marked = self.is_marked(regex)
+        previous = self._excluded.pop(regex)
+        iscompilable, error, compiled = self.compile_re(newregex)
+        self._excluded[newregex] = {
+            "index": previous["index"],
+            "compilable": iscompilable,
+            "error": error,
+            "compiled": compiled
+        }
+        self._remove_compiled(regex)
+        if was_marked and iscompilable:
+            self.mark(newregex)
+
+    def save_to_xml(self, outfile):
+        """Create a XML file that can be used by load_from_xml.
+
+        outfile can be a file object or a filename.
+        """
+        root = ET.Element("exclude_list")
+        # reversed in order to keep order of entries when reloading from xml later
+        reversed_list = []
+        for key in ordered_keys(self._excluded):
+            reversed_list.append(key)
+        for item in reversed(reversed_list):
+            exclude_node = ET.SubElement(root, "exclude")
+            exclude_node.set("regex", str(item))
+            exclude_node.set("marked", ("y" if self.is_marked(item) else "n"))
+        tree = ET.ElementTree(root)
+        with FileOrPath(outfile, "wb") as fp:
+            tree.write(fp, encoding="utf-8")
+
+
+def ordered_keys(_dict):
+    """Returns an iterator over the keys of dictionary sorted by "index" key"""
+    if not len(_dict):
+        return
+    list_of_items = []
+    for item in _dict.items():
+        list_of_items.append(item)
+    list_of_items.sort(key=lambda x: x[1].get("index"))
+    for item in list_of_items:
+        yield item[0]
+
+
+if ISWINDOWS:
+    def has_sep(x):
+        return '\\' + sep in x
+else:
+    def has_sep(x):
+        return sep in x

core/gui/exclude_list_dialog.py

@@ -0,0 +1,70 @@
+# Created On: 2012/03/13
+# Copyright 2015 Hardcoded Software (http://www.hardcoded.net)
+#
+# This software is licensed under the "GPLv3" License as described in the "LICENSE" file,
+# which should be included with this package. The terms are also available at
+# http://www.gnu.org/licenses/gpl-3.0.html
+
+# from hscommon.trans import tr
+from .exclude_list_table import ExcludeListTable
+import logging
+
+
+class ExcludeListDialogCore:
+    def __init__(self, app):
+        self.app = app
+        self.exclude_list = self.app.exclude_list  # Markable from exclude.py
+        self.exclude_list_table = ExcludeListTable(self, app)  # GUITable, this is the "model"
+
+    def restore_defaults(self):
+        self.exclude_list.restore_defaults()
+        self.refresh()
+
+    def refresh(self):
+        self.exclude_list_table.refresh()
+
+    def remove_selected(self):
+        for row in self.exclude_list_table.selected_rows:
+            self.exclude_list_table.remove(row)
+            self.exclude_list.remove(row.regex)
+        self.refresh()
+
+    def rename_selected(self, newregex):
+        """Renames the selected regex to ``newregex``.
+        If there's more than one selected row, the first one is used.
+        :param str newregex: The regex to rename the row's regex to.
+        """
+        try:
+            r = self.exclude_list_table.selected_rows[0]
+            self.exclude_list.rename(r.regex, newregex)
+            self.refresh()
+            return True
+        except Exception as e:
+            logging.warning(f"Error while renaming regex to {newregex}: {e}")
+        return False
+
+    def add(self, regex):
+        try:
+            self.exclude_list.add(regex)
+        except Exception as e:
+            raise(e)
+        self.exclude_list.mark(regex)
+        self.exclude_list_table.add(regex)
+
+    def test_string(self, test_string):
+        """Sets property on row to highlight if its regex matches test_string supplied."""
+        matched = False
+        for row in self.exclude_list_table.rows:
+            if self.exclude_list.get_compiled(row.regex).match(test_string):
+                matched = True
+                row.highlight = True
+            else:
+                row.highlight = False
+        return matched
+
+    def reset_rows_highlight(self):
+        for row in self.exclude_list_table.rows:
+            row.highlight = False
+
+    def show(self):
+        self.view.show()

core/gui/exclude_list_table.py

@@ -0,0 +1,98 @@
+# This software is licensed under the "GPLv3" License as described in the "LICENSE" file,
+# which should be included with this package. The terms are also available at
+# http://www.gnu.org/licenses/gpl-3.0.html
+
+from .base import DupeGuruGUIObject
+from hscommon.gui.table import GUITable, Row
+from hscommon.gui.column import Column, Columns
+from hscommon.trans import trget
+tr = trget("ui")
+
+
+class ExcludeListTable(GUITable, DupeGuruGUIObject):
+    COLUMNS = [
+        Column("marked", ""),
+        Column("regex", tr("Regular Expressions"))
+    ]
+
+    def __init__(self, exclude_list_dialog, app):
+        GUITable.__init__(self)
+        DupeGuruGUIObject.__init__(self, app)
+        self.columns = Columns(self)
+        self.dialog = exclude_list_dialog
+
+    def rename_selected(self, newname):
+        row = self.selected_row
+        if row is None:
+            return False
+        row._data = None
+        return self.dialog.rename_selected(newname)
+
+    # --- Virtual
+    def _do_add(self, regex):
+        """(Virtual) Creates a new row, adds it in the table.
+        Returns ``(row, insert_index)``."""
+        # Return index 0 to insert at the top
+        return ExcludeListRow(self, self.dialog.exclude_list.is_marked(regex), regex), 0
+
+    def _do_delete(self):
+        self.dalog.exclude_list.remove(self.selected_row.regex)
+
+    # --- Override
+    def add(self, regex):
+        row, insert_index = self._do_add(regex)
+        self.insert(insert_index, row)
+        self.view.refresh()
+
+    def _fill(self):
+        for enabled, regex in self.dialog.exclude_list:
+            self.append(ExcludeListRow(self, enabled, regex))
+
+    def refresh(self, refresh_view=True):
+        """Override to avoid keeping previous selection in case of multiple rows
+        selected previously."""
+        self.cancel_edits()
+        del self[:]
+        self._fill()
+        if refresh_view:
+            self.view.refresh()
+
+
+class ExcludeListRow(Row):
+    def __init__(self, table, enabled, regex):
+        Row.__init__(self, table)
+        self._app = table.app
+        self._data = None
+        self.enabled = str(enabled)
+        self.regex = str(regex)
+        self.highlight = False
+
+    @property
+    def data(self):
+        if self._data is None:
+            self._data = {"marked": self.enabled, "regex": self.regex}
+        return self._data
+
+    @property
+    def markable(self):
+        return self._app.exclude_list.is_markable(self.regex)
+
+    @property
+    def marked(self):
+        return self._app.exclude_list.is_marked(self.regex)
+
+    @marked.setter
+    def marked(self, value):
+        if value:
+            self._app.exclude_list.mark(self.regex)
+        else:
+            self._app.exclude_list.unmark(self.regex)
+
+    @property
+    def error(self):
+        # This assumes error() returns an Exception()
+        message = self._app.exclude_list.error(self.regex)
+        if hasattr(message, "msg"):
+            return self._app.exclude_list.error(self.regex).msg
+        else:
+            return message  # Exception object

core/gui/ignore_list_dialog.py

@@ -17,7 +17,7 @@ class IgnoreListDialog:
     def __init__(self, app):
         self.app = app
         self.ignore_list = self.app.ignore_list
-        self.ignore_list_table = IgnoreListTable(self)
+        self.ignore_list_table = IgnoreListTable(self)  # GUITable
 
     def clear(self):
         if not self.ignore_list:

core/tests/directories_test.py

@@ -12,6 +12,7 @@ import shutil
 from pytest import raises
 from hscommon.path import Path
 from hscommon.testutil import eq_
+from hscommon.plat import ISWINDOWS
 
 from ..fs import File
 from ..directories import (
@@ -20,6 +21,7 @@ from ..directories import (
     AlreadyThereError,
     InvalidPathError,
 )
+from ..exclude import ExcludeList, ExcludeDict
 
 
 def create_fake_fs(rootpath):
@@ -341,3 +343,200 @@ def test_default_path_state_override(tmpdir):
     d.set_state(p1["foobar"], DirectoryState.Normal)
     eq_(d.get_state(p1["foobar"]), DirectoryState.Normal)
     eq_(len(list(d.get_files())), 2)
+
+
+class TestExcludeList():
+    def setup_method(self, method):
+        self.d = Directories(exclude_list=ExcludeList(union_regex=False))
+
+    def get_files_and_expect_num_result(self, num_result):
+        """Calls get_files(), get the filenames only, print for debugging.
+        num_result is how many files are expected as a result."""
+        print(f"EXCLUDED REGEX: paths {self.d._exclude_list.compiled_paths} \
+files: {self.d._exclude_list.compiled_files} all: {self.d._exclude_list.compiled}")
+        files = list(self.d.get_files())
+        files = [file.name for file in files]
+        print(f"FINAL FILES {files}")
+        eq_(len(files), num_result)
+        return files
+
+    def test_exclude_recycle_bin_by_default(self, tmpdir):
+        regex = r"^.*Recycle\.Bin$"
+        self.d._exclude_list.add(regex)
+        self.d._exclude_list.mark(regex)
+        p1 = Path(str(tmpdir))
+        p1["$Recycle.Bin"].mkdir()
+        p1["$Recycle.Bin"]["subdir"].mkdir()
+        self.d.add_path(p1)
+        eq_(self.d.get_state(p1["$Recycle.Bin"]), DirectoryState.Excluded)
+        # By default, subdirs should be excluded too, but this can be overriden separately
+        eq_(self.d.get_state(p1["$Recycle.Bin"]["subdir"]), DirectoryState.Excluded)
+        self.d.set_state(p1["$Recycle.Bin"]["subdir"], DirectoryState.Normal)
+        eq_(self.d.get_state(p1["$Recycle.Bin"]["subdir"]), DirectoryState.Normal)
+
+    def test_exclude_refined(self, tmpdir):
+        regex1 = r"^\$Recycle\.Bin$"
+        self.d._exclude_list.add(regex1)
+        self.d._exclude_list.mark(regex1)
+        p1 = Path(str(tmpdir))
+        p1["$Recycle.Bin"].mkdir()
+        p1["$Recycle.Bin"]["somefile.png"].open("w").close()
+        p1["$Recycle.Bin"]["some_unwanted_file.jpg"].open("w").close()
+        p1["$Recycle.Bin"]["subdir"].mkdir()
+        p1["$Recycle.Bin"]["subdir"]["somesubdirfile.png"].open("w").close()
+        p1["$Recycle.Bin"]["subdir"]["unwanted_subdirfile.gif"].open("w").close()
+        p1["$Recycle.Bin"]["subdar"].mkdir()
+        p1["$Recycle.Bin"]["subdar"]["somesubdarfile.jpeg"].open("w").close()
+        p1["$Recycle.Bin"]["subdar"]["unwanted_subdarfile.png"].open("w").close()
+        self.d.add_path(p1["$Recycle.Bin"])
+
+        # Filter should set the default state to Excluded
+        eq_(self.d.get_state(p1["$Recycle.Bin"]), DirectoryState.Excluded)
+        # The subdir should inherit its parent state
+        eq_(self.d.get_state(p1["$Recycle.Bin"]["subdir"]), DirectoryState.Excluded)
+        eq_(self.d.get_state(p1["$Recycle.Bin"]["subdar"]), DirectoryState.Excluded)
+        # Override a child path's state
+        self.d.set_state(p1["$Recycle.Bin"]["subdir"], DirectoryState.Normal)
+        eq_(self.d.get_state(p1["$Recycle.Bin"]["subdir"]), DirectoryState.Normal)
+        # Parent should keep its default state, and the other child too
+        eq_(self.d.get_state(p1["$Recycle.Bin"]), DirectoryState.Excluded)
+        eq_(self.d.get_state(p1["$Recycle.Bin"]["subdar"]), DirectoryState.Excluded)
+        # print(f"get_folders(): {[x for x in self.d.get_folders()]}")
+
+        # only the 2 files directly under the Normal directory
+        files = self.get_files_and_expect_num_result(2)
+        assert "somefile.png" not in files
+        assert "some_unwanted_file.jpg" not in files
+        assert "somesubdarfile.jpeg" not in files
+        assert "unwanted_subdarfile.png" not in files
+        assert "somesubdirfile.png" in files
+        assert "unwanted_subdirfile.gif" in files
+        # Overriding the parent should enable all children
+        self.d.set_state(p1["$Recycle.Bin"], DirectoryState.Normal)
+        eq_(self.d.get_state(p1["$Recycle.Bin"]["subdar"]), DirectoryState.Normal)
+        # all files there
+        files = self.get_files_and_expect_num_result(6)
+        assert "somefile.png" in files
+        assert "some_unwanted_file.jpg" in files
+
+        # This should still filter out files under directory, despite the Normal state
+        regex2 = r".*unwanted.*"
+        self.d._exclude_list.add(regex2)
+        self.d._exclude_list.mark(regex2)
+        files = self.get_files_and_expect_num_result(3)
+        assert "somefile.png" in files
+        assert "some_unwanted_file.jpg" not in files
+        assert "unwanted_subdirfile.gif" not in files
+        assert "unwanted_subdarfile.png" not in files
+
+        if ISWINDOWS:
+            regex3 = r".*Recycle\.Bin\\.*unwanted.*subdirfile.*"
+        else:
+            regex3 = r".*Recycle\.Bin\/.*unwanted.*subdirfile.*"
+        self.d._exclude_list.rename(regex2, regex3)
+        assert self.d._exclude_list.error(regex3) is None
+        # print(f"get_folders(): {[x for x in self.d.get_folders()]}")
+        # Directory shouldn't change its state here, unless explicitely done by user
+        eq_(self.d.get_state(p1["$Recycle.Bin"]["subdir"]), DirectoryState.Normal)
+        files = self.get_files_and_expect_num_result(5)
+        assert "unwanted_subdirfile.gif" not in files
+        assert "unwanted_subdarfile.png" in files
+
+        # using end of line character should only filter the directory, or file ending with subdir
+        regex4 = r".*subdir$"
+        self.d._exclude_list.rename(regex3, regex4)
+        assert self.d._exclude_list.error(regex4) is None
+        p1["$Recycle.Bin"]["subdar"]["file_ending_with_subdir"].open("w").close()
+        eq_(self.d.get_state(p1["$Recycle.Bin"]["subdir"]), DirectoryState.Excluded)
+        files = self.get_files_and_expect_num_result(4)
+        assert "file_ending_with_subdir" not in files
+        assert "somesubdarfile.jpeg" in files
+        assert "somesubdirfile.png" not in files
+        assert "unwanted_subdirfile.gif" not in files
+        self.d.set_state(p1["$Recycle.Bin"]["subdir"], DirectoryState.Normal)
+        eq_(self.d.get_state(p1["$Recycle.Bin"]["subdir"]), DirectoryState.Normal)
+        # print(f"get_folders(): {[x for x in self.d.get_folders()]}")
+        files = self.get_files_and_expect_num_result(6)
+        assert "file_ending_with_subdir" not in files
+        assert "somesubdirfile.png" in files
+        assert "unwanted_subdirfile.gif" in files
+
+        regex5 = r".*subdir.*"
+        self.d._exclude_list.rename(regex4, regex5)
+        # Files containing substring should be filtered
+        eq_(self.d.get_state(p1["$Recycle.Bin"]["subdir"]), DirectoryState.Normal)
+        # The path should not match, only the filename, the "subdir" in the directory name shouldn't matter
+        p1["$Recycle.Bin"]["subdir"]["file_which_shouldnt_match"].open("w").close()
+        files = self.get_files_and_expect_num_result(5)
+        assert "somesubdirfile.png" not in files
+        assert "unwanted_subdirfile.gif" not in files
+        assert "file_ending_with_subdir" not in files
+        assert "file_which_shouldnt_match" in files
+
+    def test_japanese_unicode(self, tmpdir):
+        p1 = Path(str(tmpdir))
+        p1["$Recycle.Bin"].mkdir()
+        p1["$Recycle.Bin"]["somerecycledfile.png"].open("w").close()
+        p1["$Recycle.Bin"]["some_unwanted_file.jpg"].open("w").close()
+        p1["$Recycle.Bin"]["subdir"].mkdir()
+        p1["$Recycle.Bin"]["subdir"]["過去白濁物語～]_カラー.jpg"].open("w").close()
+        p1["$Recycle.Bin"]["思叫物語"].mkdir()
+        p1["$Recycle.Bin"]["思叫物語"]["なししろ会う前"].open("w").close()
+        p1["$Recycle.Bin"]["思叫物語"]["堂～ロ"].open("w").close()
+        self.d.add_path(p1["$Recycle.Bin"])
+        regex3 = r".*物語.*"
+        self.d._exclude_list.add(regex3)
+        self.d._exclude_list.mark(regex3)
+        # print(f"get_folders(): {[x for x in self.d.get_folders()]}")
+        eq_(self.d.get_state(p1["$Recycle.Bin"]["思叫物語"]), DirectoryState.Excluded)
+        files = self.get_files_and_expect_num_result(2)
+        assert "過去白濁物語～]_カラー.jpg" not in files
+        assert "なししろ会う前" not in files
+        assert "堂～ロ" not in files
+        # using end of line character should only filter that directory, not affecting its files
+        regex4 = r".*物語$"
+        self.d._exclude_list.rename(regex3, regex4)
+        assert self.d._exclude_list.error(regex4) is None
+        self.d.set_state(p1["$Recycle.Bin"]["思叫物語"], DirectoryState.Normal)
+        files = self.get_files_and_expect_num_result(5)
+        assert "過去白濁物語～]_カラー.jpg" in files
+        assert "なししろ会う前" in files
+        assert "堂～ロ" in files
+
+    def test_get_state_returns_excluded_for_hidden_directories_and_files(self, tmpdir):
+        # This regex only work for files, not paths
+        regex = r"^\..*$"
+        self.d._exclude_list.add(regex)
+        self.d._exclude_list.mark(regex)
+        p1 = Path(str(tmpdir))
+        p1["foobar"].mkdir()
+        p1["foobar"][".hidden_file.txt"].open("w").close()
+        p1["foobar"][".hidden_dir"].mkdir()
+        p1["foobar"][".hidden_dir"]["foobar.jpg"].open("w").close()
+        p1["foobar"][".hidden_dir"][".hidden_subfile.png"].open("w").close()
+        self.d.add_path(p1["foobar"])
+        # It should not inherit its parent's state originally
+        eq_(self.d.get_state(p1["foobar"][".hidden_dir"]), DirectoryState.Excluded)
+        self.d.set_state(p1["foobar"][".hidden_dir"], DirectoryState.Normal)
+        # The files should still be filtered
+        files = self.get_files_and_expect_num_result(1)
+        eq_(len(self.d._exclude_list.compiled_paths), 0)
+        eq_(len(self.d._exclude_list.compiled_files), 1)
+        assert ".hidden_file.txt" not in files
+        assert ".hidden_subfile.png" not in files
+        assert "foobar.jpg" in files
+
+
+class TestExcludeDict(TestExcludeList):
+    def setup_method(self, method):
+        self.d = Directories(exclude_list=ExcludeDict(union_regex=False))
+
+
+class TestExcludeListunion(TestExcludeList):
+    def setup_method(self, method):
+        self.d = Directories(exclude_list=ExcludeList(union_regex=True))
+
+
+class TestExcludeDictunion(TestExcludeList):
+    def setup_method(self, method):
+        self.d = Directories(exclude_list=ExcludeDict(union_regex=True))

core/tests/exclude_test.py

@@ -0,0 +1,282 @@
+# Copyright 2016 Hardcoded Software (http://www.hardcoded.net)
+#
+# This software is licensed under the "GPLv3" License as described in the "LICENSE" file,
+# which should be included with this package. The terms are also available at
+# http://www.gnu.org/licenses/gpl-3.0.html
+
+import io
+# import os.path as op
+
+from xml.etree import ElementTree as ET
+
+# from pytest import raises
+from hscommon.testutil import eq_
+from hscommon.plat import ISWINDOWS
+
+from .base import DupeGuru
+from ..exclude import ExcludeList, ExcludeDict, default_regexes, AlreadyThereException
+
+from re import error
+
+
+# Two slightly different implementations here, one around a list of lists,
+# and another around a dictionary.
+
+
+class TestCaseListXMLLoading:
+    def setup_method(self, method):
+        self.exclude_list = ExcludeList()
+
+    def test_load_non_existant_file(self):
+        # Loads the pre-defined regexes
+        self.exclude_list.load_from_xml("non_existant.xml")
+        eq_(len(default_regexes), len(self.exclude_list))
+        # they should also be marked by default
+        eq_(len(default_regexes), self.exclude_list.marked_count)
+
+    def test_save_to_xml(self):
+        f = io.BytesIO()
+        self.exclude_list.save_to_xml(f)
+        f.seek(0)
+        doc = ET.parse(f)
+        root = doc.getroot()
+        eq_("exclude_list", root.tag)
+
+    def test_save_and_load(self, tmpdir):
+        e1 = ExcludeList()
+        e2 = ExcludeList()
+        eq_(len(e1), 0)
+        e1.add(r"one")
+        e1.mark(r"one")
+        e1.add(r"two")
+        tmpxml = str(tmpdir.join("exclude_testunit.xml"))
+        e1.save_to_xml(tmpxml)
+        e2.load_from_xml(tmpxml)
+        # We should have the default regexes
+        assert r"one" in e2
+        assert r"two" in e2
+        eq_(len(e2), 2)
+        eq_(e2.marked_count, 1)
+
+    def test_load_xml_with_garbage_and_missing_elements(self):
+        root = ET.Element("foobar")  # The root element shouldn't matter
+        exclude_node = ET.SubElement(root, "bogus")
+        exclude_node.set("regex", "None")
+        exclude_node.set("marked", "y")
+
+        exclude_node = ET.SubElement(root, "exclude")
+        exclude_node.set("regex", "one")
+        # marked field invalid
+        exclude_node.set("markedddd", "y")
+
+        exclude_node = ET.SubElement(root, "exclude")
+        exclude_node.set("regex", "two")
+        # missing marked field
+
+        exclude_node = ET.SubElement(root, "exclude")
+        exclude_node.set("regex", "three")
+        exclude_node.set("markedddd", "pazjbjepo")
+
+        f = io.BytesIO()
+        tree = ET.ElementTree(root)
+        tree.write(f, encoding="utf-8")
+        f.seek(0)
+        self.exclude_list.load_from_xml(f)
+        print(f"{[x for x in self.exclude_list]}")
+        # only the two "exclude" nodes should be added,
+        eq_(3, len(self.exclude_list))
+        # None should be marked
+        eq_(0, self.exclude_list.marked_count)
+
+
+class TestCaseDictXMLLoading(TestCaseListXMLLoading):
+    def setup_method(self, method):
+        self.exclude_list = ExcludeDict()
+
+
+class TestCaseListEmpty:
+    def setup_method(self, method):
+        self.app = DupeGuru()
+        self.app.exclude_list = ExcludeList(union_regex=False)
+        self.exclude_list = self.app.exclude_list
+
+    def test_add_mark_and_remove_regex(self):
+        regex1 = r"one"
+        regex2 = r"two"
+        self.exclude_list.add(regex1)
+        assert(regex1 in self.exclude_list)
+        self.exclude_list.add(regex2)
+        self.exclude_list.mark(regex1)
+        self.exclude_list.mark(regex2)
+        eq_(len(self.exclude_list), 2)
+        eq_(len(self.exclude_list.compiled), 2)
+        compiled_files = [x for x in self.exclude_list.compiled_files]
+        eq_(len(compiled_files), 2)
+        self.exclude_list.remove(regex2)
+        assert(regex2 not in self.exclude_list)
+        eq_(len(self.exclude_list), 1)
+
+    def test_add_duplicate(self):
+        self.exclude_list.add(r"one")
+        eq_(1 , len(self.exclude_list))
+        try:
+            self.exclude_list.add(r"one")
+        except Exception:
+            pass
+        eq_(1 , len(self.exclude_list))
+
+    def test_add_not_compilable(self):
+        # Trying to add a non-valid regex should not work and raise exception
+        regex = r"one))"
+        try:
+            self.exclude_list.add(regex)
+        except Exception as e:
+            # Make sure we raise a re.error so that the interface can process it
+            eq_(type(e), error)
+        added = self.exclude_list.mark(regex)
+        eq_(added, False)
+        eq_(len(self.exclude_list), 0)
+        eq_(len(self.exclude_list.compiled), 0)
+        compiled_files = [x for x in self.exclude_list.compiled_files]
+        eq_(len(compiled_files), 0)
+
+    def test_force_add_not_compilable(self):
+        """Used when loading from XML for example"""
+        regex = r"one))"
+        try:
+            self.exclude_list.add(regex, forced=True)
+        except Exception as e:
+            # Should not get an exception here unless it's a duplicate regex
+            raise e
+        marked = self.exclude_list.mark(regex)
+        eq_(marked, False)  # can't be marked since not compilable
+        eq_(len(self.exclude_list), 1)
+        eq_(len(self.exclude_list.compiled), 0)
+        compiled_files = [x for x in self.exclude_list.compiled_files]
+        eq_(len(compiled_files), 0)
+        # adding a duplicate
+        regex = r"one))"
+        try:
+            self.exclude_list.add(regex, forced=True)
+        except Exception as e:
+            # we should have this exception, and it shouldn't be added
+            assert type(e) is AlreadyThereException
+        eq_(len(self.exclude_list), 1)
+        eq_(len(self.exclude_list.compiled), 0)
+
+    def test_rename_regex(self):
+        regex = r"one"
+        self.exclude_list.add(regex)
+        self.exclude_list.mark(regex)
+        regex_renamed = r"one))"
+        # Not compilable, can't be marked
+        self.exclude_list.rename(regex, regex_renamed)
+        assert regex not in self.exclude_list
+        assert regex_renamed in self.exclude_list
+        eq_(self.exclude_list.is_marked(regex_renamed), False)
+        self.exclude_list.mark(regex_renamed)
+        eq_(self.exclude_list.is_marked(regex_renamed), False)
+        regex_renamed_compilable = r"two"
+        self.exclude_list.rename(regex_renamed, regex_renamed_compilable)
+        assert regex_renamed_compilable in self.exclude_list
+        eq_(self.exclude_list.is_marked(regex_renamed), False)
+        self.exclude_list.mark(regex_renamed_compilable)
+        eq_(self.exclude_list.is_marked(regex_renamed_compilable), True)
+        eq_(len(self.exclude_list), 1)
+        # Should still be marked after rename
+        regex_compilable = r"three"
+        self.exclude_list.rename(regex_renamed_compilable, regex_compilable)
+        eq_(self.exclude_list.is_marked(regex_compilable), True)
+
+    def test_restore_default(self):
+        """Only unmark previously added regexes and mark the pre-defined ones"""
+        regex = r"one"
+        self.exclude_list.add(regex)
+        self.exclude_list.mark(regex)
+        self.exclude_list.restore_defaults()
+        eq_(len(default_regexes), self.exclude_list.marked_count)
+        # added regex shouldn't be marked
+        eq_(self.exclude_list.is_marked(regex), False)
+        # added regex shouldn't be in compiled list either
+        compiled = [x for x in self.exclude_list.compiled]
+        assert regex not in compiled
+        # Only default regexes marked and in compiled list
+        for re in default_regexes:
+            assert self.exclude_list.is_marked(re)
+            found = False
+            for compiled_re in compiled:
+                if compiled_re.pattern == re:
+                    found = True
+            if not found:
+                raise(Exception(f"Default RE {re} not found in compiled list."))
+            continue
+        eq_(len(default_regexes), len(self.exclude_list.compiled))
+
+
+class TestCaseDictEmpty(TestCaseListEmpty):
+    """Same, but with dictionary implementation"""
+    def setup_method(self, method):
+        self.app = DupeGuru()
+        self.app.exclude_list = ExcludeDict(union_regex=False)
+        self.exclude_list = self.app.exclude_list
+
+
+def split_union(pattern_object):
+    """Returns list of strings for each union pattern"""
+    return [x for x in pattern_object.pattern.split("|")]
+
+
+class TestCaseCompiledList():
+    """Test consistency between union or and separate versions."""
+    def setup_method(self, method):
+        self.e_separate = ExcludeList(union_regex=False)
+        self.e_separate.restore_defaults()
+        self.e_union = ExcludeList(union_regex=True)
+        self.e_union.restore_defaults()
+
+    def test_same_number_of_expressions(self):
+        # We only get one union Pattern item in a tuple, which is made of however many parts
+        eq_(len(split_union(self.e_union.compiled[0])), len(default_regexes))
+        # We get as many as there are marked items
+        eq_(len(self.e_separate.compiled), len(default_regexes))
+        exprs = split_union(self.e_union.compiled[0])
+        # We should have the same number and the same expressions
+        eq_(len(exprs), len(self.e_separate.compiled))
+        for expr in self.e_separate.compiled:
+            assert expr.pattern in exprs
+
+    def test_compiled_files(self):
+        # is path separator checked properly to yield the output
+        if ISWINDOWS:
+            regex1 = r"test\\one\\sub"
+        else:
+            regex1 = r"test/one/sub"
+        self.e_separate.add(regex1)
+        self.e_separate.mark(regex1)
+        self.e_union.add(regex1)
+        self.e_union.mark(regex1)
+        separate_compiled_dirs = self.e_separate.compiled
+        separate_compiled_files = [x for x in self.e_separate.compiled_files]
+        # HACK we need to call compiled property FIRST to generate the cache
+        union_compiled_dirs = self.e_union.compiled
+        # print(f"type: {type(self.e_union.compiled_files[0])}")
+        # A generator returning only one item... ugh
+        union_compiled_files = [x for x in self.e_union.compiled_files][0]
+        print(f"compiled files: {union_compiled_files}")
+        # Separate should give several plus the one added
+        eq_(len(separate_compiled_dirs), len(default_regexes) + 1)
+        # regex1 shouldn't be in the "files" version
+        eq_(len(separate_compiled_files), len(default_regexes))
+        # Only one Pattern returned, which when split should be however many + 1
+        eq_(len(split_union(union_compiled_dirs[0])), len(default_regexes) + 1)
+        # regex1 shouldn't be here either
+        eq_(len(split_union(union_compiled_files)), len(default_regexes))
+
+
+class TestCaseCompiledDict(TestCaseCompiledList):
+    """Test the dictionary version"""
+    def setup_method(self, method):
+        self.e_separate = ExcludeDict(union_regex=False)
+        self.e_separate.restore_defaults()
+        self.e_union = ExcludeDict(union_regex=True)
+        self.e_union.restore_defaults()

qt/app.py

@@ -27,6 +27,7 @@ from .result_window import ResultWindow
 from .directories_dialog import DirectoriesDialog
 from .problem_dialog import ProblemDialog
 from .ignore_list_dialog import IgnoreListDialog
+from .exclude_list_dialog import ExcludeListDialog
 from .deletion_options import DeletionOptions
 from .se.details_dialog import DetailsDialog as DetailsDialogStandard
 from .me.details_dialog import DetailsDialog as DetailsDialogMusic
@@ -86,11 +87,17 @@ class DupeGuru(QObject):
                 "IgnoreListDialog",
                 parent=self.main_window,
                 model=self.model.ignore_list_dialog)
-            self.ignoreListDialog.accepted.connect(self.main_window.onDialogAccepted)
+
+            self.excludeListDialog = self.main_window.createPage(
+                "ExcludeListDialog",
+                app=self,
+                parent=self.main_window,
+                model=self.model.exclude_list_dialog)
         else:
             self.ignoreListDialog = IgnoreListDialog(
-                parent=parent_window, model=self.model.ignore_list_dialog
-            )
+                parent=parent_window, model=self.model.ignore_list_dialog)
+            self.excludeDialog = ExcludeListDialog(
+                app=self, parent=parent_window, model=self.model.exclude_list_dialog)
 
         self.deletionOptions = DeletionOptions(
             parent=parent_window,
@@ -130,6 +137,7 @@ class DupeGuru(QObject):
                 tr("Clear Picture Cache"),
                 self.clearPictureCacheTriggered,
             ),
+            ("actionExcludeList", "", "", tr("Exclusion Filters"), self.excludeListTriggered),
             ("actionShowHelp", "F1", "", tr("dupeGuru Help"), self.showHelpTriggered),
             ("actionAbout", "", "", tr("About dupeGuru"), self.showAboutBoxTriggered),
             (
@@ -223,6 +231,10 @@ class DupeGuru(QObject):
     def showResultsWindow(self):
         if self.resultWindow is not None:
             if self.use_tabs:
+                if self.main_window.indexOfWidget(self.resultWindow) < 0:
+                    self.main_window.addTab(
+                        self.resultWindow, "Results", switch=True)
+                    return
                 self.main_window.showTab(self.resultWindow)
             else:
                 self.resultWindow.show()
@@ -267,19 +279,25 @@ class DupeGuru(QObject):
 
     def ignoreListTriggered(self):
         if self.use_tabs:
-            # Fetch the index in the TabWidget or the StackWidget (depends on class):
-            index = self.main_window.indexOfWidget(self.ignoreListDialog)
-            if index < 0:
-                # we have not instantiated and populated it in their internal list yet
-                index = self.main_window.addTab(
-                    self.ignoreListDialog, "Ignore List", switch=True)
-            # if not self.main_window.tabWidget.isTabVisible(index):
-            self.main_window.setTabVisible(index, True)
-            self.main_window.setCurrentIndex(index)
-            return
-        else:
+            self.showTriggeredTabbedDialog(self.ignoreListDialog, "Ignore List")
+        else:  # floating windows
             self.model.ignore_list_dialog.show()
 
+    def excludeListTriggered(self):
+        if self.use_tabs:
+            self.showTriggeredTabbedDialog(self.excludeListDialog, "Exclusion Filters")
+        else:  # floating windows
+            self.model.exclude_list_dialog.show()
+
+    def showTriggeredTabbedDialog(self, dialog, desc_string):
+        """Add tab for dialog, name the tab with desc_string, then show it."""
+        index = self.main_window.indexOfWidget(dialog)
+        # Create the tab if it doesn't exist already
+        if index < 0:  # or (not dialog.isVisible() and not self.main_window.isTabVisible(index)):
+            index = self.main_window.addTab(dialog, desc_string, switch=True)
+        # Show the tab for that widget
+        self.main_window.setCurrentIndex(index)
+
     def openDebugLogTriggered(self):
         debugLogPath = op.join(self.model.appdata, "debug.log")
         desktop.open_path(debugLogPath)
@@ -344,15 +362,15 @@ class DupeGuru(QObject):
             # or simply delete it on close which is probably cleaner:
             self.details_dialog.setAttribute(Qt.WA_DeleteOnClose)
             self.details_dialog.close()
-            # self.details_dialog.setParent(None)  # seems unnecessary
+            # if we don't do the following, Qt will crash when we recreate the Results dialog
+            self.details_dialog.setParent(None)
         if self.resultWindow is not None:
             self.resultWindow.close()
-            self.resultWindow.setParent(None)
+            # This is better for tabs, as it takes care of duplicate items in menu bar
+            self.resultWindow.deleteLater() if self.use_tabs else self.resultWindow.setParent(None)
         if self.use_tabs:
             self.resultWindow = self.main_window.createPage(
                 "ResultWindow", parent=self.main_window, app=self)
-            self.main_window.addTab(
-                self.resultWindow, "Results", switch=False)
         else:  # We don't use a tab widget, regular floating QMainWindow
             self.resultWindow = ResultWindow(self.directories_dialog, self)
             self.directories_dialog._updateActionsState()

qt/dg.qrc

@@ -10,5 +10,6 @@
     <file alias="zoom_out">../images/old_zoom_out.png</file>
     <file alias="zoom_original">../images/old_zoom_original.png</file>
     <file alias="zoom_best_fit">../images/old_zoom_best_fit.png</file>
+    <file alias="error">../images/dialog-error.png</file>
 </qresource>
 </RCC>

qt/directories_dialog.py

@@ -137,6 +137,7 @@ class DirectoriesDialog(QMainWindow):
         self.menuView.addAction(self.app.actionDirectoriesWindow)
         self.menuView.addAction(self.actionShowResultsWindow)
         self.menuView.addAction(self.app.actionIgnoreList)
+        self.menuView.addAction(self.app.actionExcludeList)
         self.menuView.addSeparator()
         self.menuView.addAction(self.app.actionPreferences)
 

qt/exclude_list_dialog.py

@@ -0,0 +1,167 @@
+# This software is licensed under the "GPLv3" License as described in the "LICENSE" file,
+# which should be included with this package. The terms are also available at
+# http://www.gnu.org/licenses/gpl-3.0.html
+
+import re
+from PyQt5.QtCore import Qt, pyqtSlot
+from PyQt5.QtWidgets import (
+    QPushButton, QLineEdit, QVBoxLayout, QGridLayout, QDialog,
+    QTableView, QAbstractItemView, QSpacerItem, QSizePolicy, QHeaderView
+)
+from .exclude_list_table import ExcludeListTable
+
+from core.exclude import AlreadyThereException
+from hscommon.trans import trget
+tr = trget("ui")
+
+
+class ExcludeListDialog(QDialog):
+    def __init__(self, app, parent, model, **kwargs):
+        flags = Qt.CustomizeWindowHint | Qt.WindowTitleHint | Qt.WindowSystemMenuHint
+        super().__init__(parent, flags, **kwargs)
+        self.app = app
+        self.specific_actions = frozenset()
+        self._setupUI()
+        self.model = model  # ExcludeListDialogCore
+        self.model.view = self
+        self.table = ExcludeListTable(app, view=self.tableView)  # Qt ExcludeListTable
+        self._row_matched = False  # test if at least one row matched our test string
+        self._input_styled = False
+
+        self.buttonAdd.clicked.connect(self.addStringFromLineEdit)
+        self.buttonRemove.clicked.connect(self.removeSelected)
+        self.buttonRestore.clicked.connect(self.restoreDefaults)
+        self.buttonClose.clicked.connect(self.accept)
+        self.buttonHelp.clicked.connect(self.display_help_message)
+        self.buttonTestString.clicked.connect(self.onTestStringButtonClicked)
+        self.inputLine.textEdited.connect(self.reset_input_style)
+        self.testLine.textEdited.connect(self.reset_input_style)
+        self.testLine.textEdited.connect(self.reset_table_style)
+
+    def _setupUI(self):
+        layout = QVBoxLayout(self)
+        gridlayout = QGridLayout()
+        self.buttonAdd = QPushButton(tr("Add"))
+        self.buttonRemove = QPushButton(tr("Remove Selected"))
+        self.buttonRestore = QPushButton(tr("Restore defaults"))
+        self.buttonTestString = QPushButton(tr("Test string"))
+        self.buttonClose = QPushButton(tr("Close"))
+        self.buttonHelp = QPushButton(tr("Help"))
+        self.inputLine = QLineEdit()
+        self.testLine = QLineEdit()
+        self.tableView = QTableView()
+        triggers = (
+            QAbstractItemView.DoubleClicked
+            | QAbstractItemView.EditKeyPressed
+            | QAbstractItemView.SelectedClicked
+        )
+        self.tableView.setEditTriggers(triggers)
+        self.tableView.setSelectionMode(QTableView.ExtendedSelection)
+        self.tableView.setSelectionBehavior(QTableView.SelectRows)
+        self.tableView.setShowGrid(False)
+        vheader = self.tableView.verticalHeader()
+        vheader.setSectionsMovable(True)
+        vheader.setVisible(False)
+        hheader = self.tableView.horizontalHeader()
+        hheader.setSectionsMovable(False)
+        hheader.setSectionResizeMode(QHeaderView.Fixed)
+        hheader.setStretchLastSection(True)
+        hheader.setHighlightSections(False)
+        hheader.setVisible(True)
+        gridlayout.addWidget(self.inputLine, 0, 0)
+        gridlayout.addWidget(self.buttonAdd, 0, 1, Qt.AlignLeft)
+        gridlayout.addWidget(self.buttonRemove, 1, 1, Qt.AlignLeft)
+        gridlayout.addWidget(self.buttonRestore, 2, 1, Qt.AlignLeft)
+        gridlayout.addWidget(self.buttonHelp, 3, 1, Qt.AlignLeft)
+        gridlayout.addWidget(self.buttonClose, 4, 1)
+        gridlayout.addWidget(self.tableView, 1, 0, 6, 1)
+        gridlayout.addItem(QSpacerItem(0, 0, QSizePolicy.Minimum, QSizePolicy.Expanding), 4, 1)
+        gridlayout.addWidget(self.buttonTestString, 6, 1)
+        gridlayout.addWidget(self.testLine, 6, 0)
+
+        layout.addLayout(gridlayout)
+        self.inputLine.setPlaceholderText(tr("Type a python regular expression here..."))
+        self.inputLine.setFocus()
+        self.testLine.setPlaceholderText(tr("Type a file system path or filename here..."))
+        self.testLine.setClearButtonEnabled(True)
+
+    # --- model --> view
+    def show(self):
+        super().show()
+        self.inputLine.setFocus()
+
+    @pyqtSlot()
+    def addStringFromLineEdit(self):
+        text = self.inputLine.text()
+        if not text:
+            return
+        try:
+            self.model.add(text)
+        except AlreadyThereException:
+            self.app.show_message("Expression already in the list.")
+            return
+        except Exception as e:
+            self.app.show_message(f"Expression is invalid: {e}")
+            return
+        self.inputLine.clear()
+
+    def removeSelected(self):
+        self.model.remove_selected()
+
+    def restoreDefaults(self):
+        self.model.restore_defaults()
+
+    def onTestStringButtonClicked(self):
+        input_text = self.testLine.text()
+        if not input_text:
+            self.reset_input_style()
+            return
+        # if at least one row matched, we know whether table is highlighted or not
+        self._row_matched = self.model.test_string(input_text)
+        # FIXME There is a bug on Windows (7) where the table rows don't get
+        # repainted until the table receives a mouse click event.
+        self.tableView.update()
+
+        input_regex = self.inputLine.text()
+        if not input_regex:
+            self.reset_input_style()
+            return
+        try:
+            compiled = re.compile(input_regex)
+        except re.error:
+            self.reset_input_style()
+            return
+        match = compiled.match(input_text)
+        if match:
+            self._input_styled = True
+            self.inputLine.setStyleSheet("background-color: rgb(10, 200, 10);")
+        else:
+            self.reset_input_style()
+
+    def reset_input_style(self):
+        """Reset regex input line background"""
+        if self._input_styled:
+            self._input_styled = False
+            self.inputLine.setStyleSheet(self.styleSheet())
+
+    def reset_table_style(self):
+        if self._row_matched:
+            self._row_matched = False
+            self.model.reset_rows_highlight()
+        self.tableView.update()
+
+    def display_help_message(self):
+        self.app.show_message(tr("""\
+These (case sensitive) python regular expressions will filter out files during scans.<br>\
+Directores will also have their <strong>default state</strong> set to Excluded \
+in the Directories tab if their name happen to match one of the regular expressions.<br>\
+For each file collected two tests are perfomed on each of them to determine whether or not to filter them out:<br>\
+<li>1. Regular expressions with no path separator in them will be compared to the file name only.</li>
+<li>2. Regular expressions with no path separator in them will be compared to the full path to the file.</li><br>
+Example: if you want to filter out .PNG files from the "My Pictures" directory only:<br>\
+<code>.*My\\sPictures\\\\.*\\.png</code><br><br>\
+You can test the regular expression with the test string feature by pasting a fake path in it:<br>\
+<code>C:\\\\User\\My Pictures\\test.png</code><br><br>
+Matching regular expressions will be highlighted.<br>\
+If there is at least one highlight, the path tested will be ignored during scans.<br><br>\
+Directories and files starting with a period '.' are filtered out by default.<br><br>"""))

qt/exclude_list_table.py

@@ -0,0 +1,77 @@
+# This software is licensed under the "GPLv3" License as described in the "LICENSE" file,
+# which should be included with this package. The terms are also available at
+# http://www.gnu.org/licenses/gpl-3.0.html
+
+from PyQt5.QtCore import Qt
+from PyQt5.QtGui import QFont, QFontMetrics, QIcon, QColor
+
+from qtlib.column import Column
+from qtlib.table import Table
+from hscommon.trans import trget
+tr = trget("ui")
+
+
+class ExcludeListTable(Table):
+    """Model for exclude list"""
+    COLUMNS = [
+        Column("marked", defaultWidth=15),
+        Column("regex", defaultWidth=230)
+    ]
+
+    def __init__(self, app, view, **kwargs):
+        model = app.model.exclude_list_dialog.exclude_list_table  # pointer to GUITable
+        super().__init__(model, view, **kwargs)
+        font = view.font()
+        font.setPointSize(app.prefs.tableFontSize)
+        view.setFont(font)
+        fm = QFontMetrics(font)
+        view.verticalHeader().setDefaultSectionSize(fm.height() + 2)
+        # app.willSavePrefs.connect(self.appWillSavePrefs)
+
+    def _getData(self, row, column, role):
+        if column.name == "marked":
+            if role == Qt.CheckStateRole and row.markable:
+                return Qt.Checked if row.marked else Qt.Unchecked
+            if role == Qt.ToolTipRole and not row.markable:
+                return tr("Compilation error: ") + row.get_cell_value("error")
+            if role == Qt.DecorationRole and not row.markable:
+                return QIcon.fromTheme("dialog-error", QIcon(":/error"))
+            return None
+        if role == Qt.DisplayRole:
+            return row.data[column.name]
+        elif role == Qt.FontRole:
+            return QFont(self.view.font())
+        elif role == Qt.BackgroundRole and column.name == "regex":
+            if row.highlight:
+                return QColor(10, 200, 10)  # green
+        elif role == Qt.EditRole:
+            if column.name == "regex":
+                return row.data[column.name]
+        return None
+
+    def _getFlags(self, row, column):
+        flags = Qt.ItemIsEnabled
+        if column.name == "marked":
+            if row.markable:
+                flags |= Qt.ItemIsUserCheckable
+        elif column.name == "regex":
+            flags |= Qt.ItemIsEditable | Qt.ItemIsSelectable | Qt.ItemIsDragEnabled | Qt.ItemIsDropEnabled
+        return flags
+
+    def _setData(self, row, column, value, role):
+        if role == Qt.CheckStateRole:
+            if column.name == "marked":
+                row.marked = bool(value)
+                return True
+        elif role == Qt.EditRole:
+            if column.name == "regex":
+                return self.model.rename_selected(value)
+        return False
+
+    # def sort(self, column, order):
+    #     column = self.model.COLUMNS[column]
+    #     self.model.sort(column.name, order == Qt.AscendingOrder)
+
+    # # --- Events
+    # def appWillSavePrefs(self):
+    #     self.model.columns.save_columns()

qt/ignore_list_table.py

@@ -10,6 +10,8 @@ from qtlib.table import Table
 
 
 class IgnoreListTable(Table):
+    """ Ignore list model"""
+
     COLUMNS = [
         Column("path1", defaultWidth=230),
         Column("path2", defaultWidth=230),

qt/pe/details_dialog.py

@@ -9,7 +9,6 @@ from PyQt5.QtWidgets import (
     QAbstractItemView, QSizePolicy, QGridLayout, QSplitter, QFrame)
 from PyQt5.QtGui import QResizeEvent
 from hscommon.trans import trget
-from hscommon.plat import ISWINDOWS
 from ..details_dialog import DetailsDialog as DetailsDialogBase
 from ..details_table import DetailsTable
 from .image_viewer import (
@@ -102,14 +101,14 @@ class DetailsDialog(DetailsDialogBase):
         self.vController.updateBothImages()
 
     def show(self):
-        # Compute the maximum size the table view can reach
-        # Assuming all rows below headers have the same height
+        # Give the splitter a maximum height to reach. This is assuming that
+        # all rows below their headers have the same height
         self.tableView.setMaximumHeight(
             self.tableView.rowHeight(1)
             * self.tableModel.model.row_count()
             + self.tableView.verticalHeader().sectionSize(0)
-            # Windows seems to add a few pixels more to the table somehow
-            + (5 if ISWINDOWS else 0))
+            # looks like the handle is taken into account by the splitter
+            + self.splitter.handle(1).size().height())
         DetailsDialogBase.show(self)
         self.ensure_same_sizes()
         self._update()

qt/preferences_dialog.py

@@ -161,28 +161,31 @@ On MacOS, the tab bar will fill up the window's width instead."))
         self.ui_groupbox.setLayout(layout)
         self.displayVLayout.addWidget(self.ui_groupbox)
 
-        gridlayout = QFormLayout()
+        gridlayout = QGridLayout()
+        gridlayout.setColumnStretch(2, 2)
+        formlayout = QFormLayout()
         result_groupbox = QGroupBox("&Result Table")
         self.fontSizeSpinBox = QSpinBox()
         self.fontSizeSpinBox.setMinimum(5)
-        gridlayout.addRow(tr("Font size:"), self.fontSizeSpinBox)
+        formlayout.addRow(tr("Font size:"), self.fontSizeSpinBox)
         self._setupAddCheckbox("reference_bold_font",
                                tr("Use bold font for references"))
-        gridlayout.addRow(self.reference_bold_font)
+        formlayout.addRow(self.reference_bold_font)
 
         self.result_table_ref_foreground_color = ColorPickerButton(self)
-        gridlayout.addRow(tr("Reference foreground color:"),
+        formlayout.addRow(tr("Reference foreground color:"),
                           self.result_table_ref_foreground_color)
         self.result_table_ref_background_color = ColorPickerButton(self)
-        gridlayout.addRow(tr("Reference background color:"),
+        formlayout.addRow(tr("Reference background color:"),
                           self.result_table_ref_background_color)
         self.result_table_delta_foreground_color = ColorPickerButton(self)
-        gridlayout.addRow(tr("Delta foreground color:"),
+        formlayout.addRow(tr("Delta foreground color:"),
                           self.result_table_delta_foreground_color)
-        gridlayout.setLabelAlignment(Qt.AlignLeft)
+        formlayout.setLabelAlignment(Qt.AlignLeft)
 
         # Keep same vertical spacing as parent layout for consistency
-        gridlayout.setVerticalSpacing(self.displayVLayout.spacing())
+        formlayout.setVerticalSpacing(self.displayVLayout.spacing())
+        gridlayout.addLayout(formlayout, 0, 0)
         result_groupbox.setLayout(gridlayout)
         self.displayVLayout.addWidget(result_groupbox)
 
@@ -205,12 +208,13 @@ use the modifier key to drag the floating window around") if ISLINUX else
         self.details_dialog_titlebar_enabled.stateChanged.connect(
             self.details_dialog_vertical_titlebar.setEnabled)
         gridlayout = QGridLayout()
-        self.details_table_delta_foreground_color_label = QLabel(tr("Delta foreground color:"))
-        gridlayout.addWidget(self.details_table_delta_foreground_color_label, 4, 0)
+        formlayout = QFormLayout()
         self.details_table_delta_foreground_color = ColorPickerButton(self)
-        gridlayout.addWidget(self.details_table_delta_foreground_color, 4, 2, 1, 1, Qt.AlignLeft)
+        # Padding on the right side and space between label and widget to keep it somewhat consistent across themes
         gridlayout.setColumnStretch(1, 1)
-        gridlayout.setColumnStretch(3, 4)
+        formlayout.setHorizontalSpacing(50)
+        formlayout.addRow(tr("Delta foreground color:"), self.details_table_delta_foreground_color)
+        gridlayout.addLayout(formlayout, 0, 0)
         self.details_groupbox_layout.addLayout(gridlayout)
         details_groupbox.setLayout(self.details_groupbox_layout)
         self.displayVLayout.addWidget(details_groupbox)

qt/tabbed_window.py

@@ -18,6 +18,7 @@ from qtlib.util import moveToScreenCenter, createActions
 from .directories_dialog import DirectoriesDialog
 from .result_window import ResultWindow
 from .ignore_list_dialog import IgnoreListDialog
+from .exclude_list_dialog import ExcludeListDialog
 tr = trget("ui")
 
 
@@ -25,7 +26,7 @@ class TabWindow(QMainWindow):
     def __init__(self, app, **kwargs):
         super().__init__(None, **kwargs)
         self.app = app
-        self.pages = {}
+        self.pages = {}  # This is currently not used anywhere
         self.menubar = None
         self.menuList = set()
         self.last_index = -1
@@ -108,7 +109,7 @@ class TabWindow(QMainWindow):
         self.menuList.add(self.menuHelp)
 
     @pyqtSlot(int)
-    def updateMenuBar(self, page_index=None):
+    def updateMenuBar(self, page_index=-1):
         if page_index < 0:
             return
         current_index = self.getCurrentIndex()
@@ -141,6 +142,9 @@ class TabWindow(QMainWindow):
             and not page_type == "IgnoreListDialog" else False)
         self.app.actionDirectoriesWindow.setEnabled(
             False if page_type == "DirectoriesDialog" else True)
+        self.app.actionExcludeList.setEnabled(
+            True if self.app.excludeListDialog is not None
+            and not page_type == "ExcludeListDialog" else False)
 
         self.previous_widget_actions = active_widget.specific_actions
         self.last_index = current_index
@@ -157,7 +161,14 @@ class TabWindow(QMainWindow):
             parent = kwargs.get("parent", self)
             model = kwargs.get("model")
             page = IgnoreListDialog(parent, model)
-        self.pages[cls] = page
+            page.accepted.connect(self.onDialogAccepted)
+        elif cls == "ExcludeListDialog":
+            app = kwargs.get("app", app)
+            parent = kwargs.get("parent", self)
+            model = kwargs.get("model")
+            page = ExcludeListDialog(app, parent, model)
+            page.accepted.connect(self.onDialogAccepted)
+        self.pages[cls] = page  # Not used, might remove
         return page
 
     def addTab(self, page, title, switch=False):
@@ -173,7 +184,6 @@ class TabWindow(QMainWindow):
 
     def showTab(self, page):
         index = self.indexOfWidget(page)
-        self.setTabVisible(index, True)
         self.setCurrentIndex(index)
 
     def indexOfWidget(self, widget):
@@ -182,9 +192,6 @@ class TabWindow(QMainWindow):
     def setCurrentIndex(self, index):
         return self.tabWidget.setCurrentIndex(index)
 
-    def setTabVisible(self, index, value):
-        return self.tabWidget.setTabVisible(index, value)
-
     def removeTab(self, index):
         return self.tabWidget.removeTab(index)
 
@@ -202,7 +209,7 @@ class TabWindow(QMainWindow):
 
     # --- Events
     def appWillSavePrefs(self):
-        # Right now this is useless since the first spawn dialog inside the
+        # Right now this is useless since the first spawned dialog inside the
         # QTabWidget will assign its geometry after restoring it
         prefs = self.app.prefs
         prefs.mainWindowIsMaximized = self.isMaximized()
@@ -223,14 +230,11 @@ class TabWindow(QMainWindow):
             # menu or shortcut. But this is useless if we don't have a button
             # set up to make a close request anyway. This check could be removed.
             return
-        current_widget.close()
-        self.setTabVisible(index, False)
-        # self.tabWidget.widget(index).hide()
         self.removeTab(index)
 
     @pyqtSlot()
     def onDialogAccepted(self):
-        """Remove tabbed dialog when Accepted/Done."""
+        """Remove tabbed dialog when Accepted/Done (close button clicked)."""
         widget = self.sender()
         index = self.indexOfWidget(widget)
         if index > -1:
@@ -268,7 +272,7 @@ class TabBarWindow(TabWindow):
         self.verticalLayout.addLayout(self.horizontalLayout)
         self.verticalLayout.addWidget(self.stackedWidget)
 
-        self.tabBar.currentChanged.connect(self.showWidget)
+        self.tabBar.currentChanged.connect(self.showTabIndex)
         self.tabBar.tabCloseRequested.connect(self.onTabCloseRequested)
 
         self.stackedWidget.currentChanged.connect(self.updateMenuBar)
@@ -278,50 +282,48 @@ class TabBarWindow(TabWindow):
         self.restoreGeometry()
 
     def addTab(self, page, title, switch=True):
-        stack_index = self.stackedWidget.insertWidget(-1, page)
-        tab_index = self.tabBar.addTab(title)
+        stack_index = self.stackedWidget.addWidget(page)
+        self.tabBar.insertTab(stack_index, title)
 
         if isinstance(page, DirectoriesDialog):
             self.tabBar.setTabButton(
-                tab_index, QTabBar.RightSide, None)
+                stack_index, QTabBar.RightSide, None)
         if switch:  # switch to the added tab immediately upon creation
-            self.setTabIndex(tab_index)
-            self.stackedWidget.setCurrentWidget(page)
+            self.setTabIndex(stack_index)
         return stack_index
 
     @pyqtSlot(int)
-    def showWidget(self, index):
-        if index >= 0 and index <= self.stackedWidget.count() - 1:
+    def showTabIndex(self, index):
+        # The tab bar's indices should be aligned with the stackwidget's
+        if index >= 0 and index <= self.stackedWidget.count():
             self.stackedWidget.setCurrentIndex(index)
-            # if not self.tabBar.isTabVisible(index):
-            self.setTabVisible(index, True)
 
     def indexOfWidget(self, widget):
         # Warning: this may return -1 if widget is not a child of stackedwidget
         return self.stackedWidget.indexOf(widget)
 
     def setCurrentIndex(self, tab_index):
-        # The signal will handle switching the stackwidget's widget
         self.setTabIndex(tab_index)
+        # The signal will handle switching the stackwidget's widget
         # self.stackedWidget.setCurrentWidget(self.stackedWidget.widget(tab_index))
 
+    def setCurrentWidget(self, widget):
+        """Sets the current Tab on TabBar for this widget."""
+        self.tabBar.setCurrentIndex(self.indexOfWidget(widget))
+
     @pyqtSlot(int)
     def setTabIndex(self, index):
         if index is None:
             return
         self.tabBar.setCurrentIndex(index)
 
-    def setTabVisible(self, index, value):
-        return self.tabBar.setTabVisible(index, value)
-
     @pyqtSlot(int)
     def onRemovedWidget(self, index):
         self.removeTab(index)
 
     @pyqtSlot(int)
     def removeTab(self, index):
-        # No need to remove the widget here:
-        # self.stackedWidget.removeWidget(self.stackedWidget.widget(index))
+        """Remove the tab, but not the widget (it should already be removed)"""
         return self.tabBar.removeTab(index)
 
     @pyqtSlot(int)
@@ -348,13 +350,18 @@ class TabBarWindow(TabWindow):
 
     @pyqtSlot(int)
     def onTabCloseRequested(self, index):
-        current_widget = self.getWidgetAtIndex(index)
-        if isinstance(current_widget, DirectoriesDialog):
+        target_widget = self.getWidgetAtIndex(index)
+        if isinstance(target_widget, DirectoriesDialog):
             # On MacOS, the tab has a close button even though we explicitely
             # set it to None in order to hide it. This should prevent
             # the "Directories" tab from closing by mistake.
             return
-        current_widget.close()
-        self.stackedWidget.removeWidget(current_widget)
-        # In this case the signal will take care of the tab itself after removing the widget
-        # self.removeTab(index)
+        # target_widget.close()  # seems unnecessary
+        # Removing the widget should trigger tab removal via the signal
+        self.removeWidget(self.getWidgetAtIndex(index))
+
+    @pyqtSlot()
+    def onDialogAccepted(self):
+        """Remove tabbed dialog when Accepted/Done (close button clicked)."""
+        widget = self.sender()
+        self.removeWidget(widget)