Merge pull request #8093 from qutebrowser/tree/fix-lint

Fix lint and tests on tree tabs integration branch

7 files changed, 62 insertions, 46 deletions

diff --git a/doc/treetabs.md b/doc/treetabs.md
index 319997efb..f32ee5449 100644
--- a/doc/treetabs.md
+++ b/doc/treetabs.md
@@ -5,7 +5,7 @@
 Tree style tabs allow you to group and manage related tabs together. Related
 tabs will be shown in a hierarchical fashion in the tab bar when it is on the
 left or right side of the browser window. It can be enabled by setting
-`tabs.tree_tabs` to `true`. That setting only applies to new windows create
+`tabs.tree_tabs` to `true`. That setting only applies to new windows created
 after it is enabled (including via saving and loading a session or
 `:restart`).
 
@@ -38,10 +38,10 @@ todo: add animated illustrations?
 
 You can change how tabs relate to each other after they are created too.
 
-* `:open`, as described in the into, has picked up some new behaviour to
+* `:open`, as described in the intro, has picked up some new behavior to
   decide where in relation to the current tab a new one should go. It has a
-  new `--sibling` argument and the `--related` argument, as well as `--tab` and
-  `-background`, has some additional meaning.
+  new `--sibling` argument and the existing arguments `--related`, `--tab` and
+  `--background` have picked up some additional meaning to help with that.
 * `:tab-move` will move a tab and its children within the tree
     * With a `+` or `-` argument tabs will only move within their siblings
       (wrapping at the top or bottom)
@@ -90,7 +90,7 @@ commands introduced to take advantage of the tab grouping feature.
 
 ## Settings
 
-There are some existing settings who's behaviour will be modified when tree
+There are some existing settings that will have modified behavior when tree
 tabs are enabled:
 
 * `tabs.new_position.related`: this is essentially replaced by
@@ -144,7 +144,7 @@ either the parent or children of a node via property setters. Beyond those two
 setters nodes have `promote()` and `demote()` helper functions used by the
 corresponding commands.
 
-Beyond those four methods to manipulate tree the tree structures nodes have
+Beyond those four methods to manipulate the tree structure nodes have
 methods for:
 
 * traversing the tree:
@@ -153,9 +153,9 @@ methods for:
   * `depth()` return depth in tree
 * collapsing a node
   * this just sets an attribute on a node, the traversal function respects it
-  * but beyond that it's up to callers to know that an un-collapsed node may
+    but beyond that it's up to callers to know that an un-collapsed node may
     be hidden if a parent node is collapsed, there are a few pieces of
-    calling code which do implement different behaviour for collapsed nodes
+    calling code which do implement different behavior for collapsed nodes
 * rendering unicode tree segments to be used in tab titles
   * our tab bar itself doesn't understand the tree structure for now, it's
     just being represented by drawing unicode line and angle characters to
diff --git a/qutebrowser/browser/commands.py b/qutebrowser/browser/commands.py
index d7212d6ce..6784f0c1d 100644
--- a/qutebrowser/browser/commands.py
+++ b/qutebrowser/browser/commands.py
@@ -496,7 +496,6 @@ class CommandDispatcher:
 
         # second pass: copy tree structure over
         newroot = tabbed_browser.widget.tree_root
-        oldroot = self._tabbed_browser.widget.tree_root
         for node in traversed:
             if node.parent.uid in uid_map:
                 uid = uid_map[node.uid]
diff --git a/qutebrowser/mainwindow/tabbedbrowser.py b/qutebrowser/mainwindow/tabbedbrowser.py
index 89b43285d..c07b9e1ad 100644
--- a/qutebrowser/mainwindow/tabbedbrowser.py
+++ b/qutebrowser/mainwindow/tabbedbrowser.py
@@ -616,6 +616,7 @@ class TabbedBrowser(QWidget):
             background: bool = None,
             related: bool = True,
             idx: int = None,
+            sibling: bool = False,  # pylint: disable=unused-argument
     ) -> browsertab.AbstractTab:
         """Open a new tab with a given URL.
 
diff --git a/qutebrowser/mainwindow/treetabbedbrowser.py b/qutebrowser/mainwindow/treetabbedbrowser.py
index 5e2f796f1..75a3c55f8 100644
--- a/qutebrowser/mainwindow/treetabbedbrowser.py
+++ b/qutebrowser/mainwindow/treetabbedbrowser.py
@@ -16,7 +16,6 @@ from qutebrowser.mainwindow.tabbedbrowser import TabbedBrowser
 from qutebrowser.mainwindow.treetabwidget import TreeTabWidget
 from qutebrowser.browser import browsertab
 from qutebrowser.misc import notree
-from qutebrowser.utils import log
 
 
 @dataclasses.dataclass
@@ -81,7 +80,6 @@ class TreeTabbedBrowser(TabbedBrowser):
 
     def _remove_tab(self, tab, *, add_undo=True, new_undo=True, crashed=False):
         """Handle children positioning after a tab is removed."""
-
         if not tab.url().isEmpty() and tab.url().isValid() and add_undo:
             idx = self.widget.indexOf(tab)
             self._add_undo_entry(tab, idx, new_undo)
@@ -128,7 +126,12 @@ class TreeTabbedBrowser(TabbedBrowser):
 
         self.widget.tree_tab_update()
 
-    def _add_undo_entry(self, tab, idx, new_undo):
+    def _add_undo_entry(
+        self,
+        tab,
+        idx,  # pylint: disable=unused-argument
+        new_undo,
+    ):
         """Save undo entry with tree information.
 
         This function was removed in tabbedbrowser, but it is still useful here because
@@ -199,8 +202,8 @@ class TreeTabbedBrowser(TabbedBrowser):
             self, url: QUrl = None,
             background: bool = None,
             related: bool = True,
-            sibling: bool = False,
             idx: int = None,
+            sibling: bool = False,
     ) -> browsertab.AbstractTab:
         """Open a new tab with a given url.
 
@@ -211,7 +214,6 @@ class TreeTabbedBrowser(TabbedBrowser):
                      focused tab.  Follows `tabs.new_position.tree.sibling`.
 
         """
-        # pylint: disable=arguments-differ
         # we save this now because super.tabopen also resets the focus
         cur_tab = self.widget.currentWidget()
         tab = super().tabopen(url, background, related, idx)
diff --git a/qutebrowser/mainwindow/treetabwidget.py b/qutebrowser/mainwindow/treetabwidget.py
index 082b7a079..92e18ae77 100644
--- a/qutebrowser/mainwindow/treetabwidget.py
+++ b/qutebrowser/mainwindow/treetabwidget.py
@@ -75,7 +75,7 @@ class TreeTabWidget(TabWidget):
         # empty_urls here is a proxy for "there is a session being loaded into
         # this window"
         empty_urls = all(
-            [self.widget(idx).url().toString() == "" for idx in range(self.count())]
+            not self.widget(idx).url().toString() for idx in range(self.count())
         )
         if empty_urls and is_hidden:
             # All tabs will be added to the tab widget during session load
@@ -121,24 +121,25 @@ class TreeTabWidget(TabWidget):
         for node in self.tree_root.traverse():
             if node.value is None:
                 continue
-            if any(ancestor.collapsed for ancestor in node.path[:-1]):
-                if self.indexOf(node.value) != -1:
-                    # node should be hidden but is shown
-                    cur_tab = node.value
-                    idx = self.indexOf(cur_tab)
-                    if idx != -1:
-                        self.removeTab(idx)
-            else:
-                if self.indexOf(node.value) == -1:
-                    # node should be shown but is hidden
-                    parent = node.parent
-                    tab = node.value
-                    name = tab.title()
-                    icon = tab.icon()
-                    if node.parent is not None:
-                        parent_idx = self.indexOf(node.parent.value)
-                    self.insertTab(parent_idx + 1, tab, icon, name)
-                    tab.node.parent = parent  # insertTab resets node
+
+            should_be_hidden = any(ancestor.collapsed for ancestor in node.path[:-1])
+            is_shown = self.indexOf(node.value) != -1
+            if should_be_hidden and is_shown:
+                # node should be hidden but is shown
+                cur_tab = node.value
+                idx = self.indexOf(cur_tab)
+                if idx != -1:
+                    self.removeTab(idx)
+            elif not should_be_hidden and not is_shown:
+                # node should be shown but is hidden
+                parent = node.parent
+                tab = node.value
+                name = tab.title()
+                icon = tab.icon()
+                if node.parent is not None:
+                    parent_idx = self.indexOf(node.parent.value)
+                self.insertTab(parent_idx + 1, tab, icon, name)
+                tab.node.parent = parent  # insertTab resets node
 
     def tree_tab_update(self):
         """Update titles and positions."""
diff --git a/qutebrowser/misc/notree.py b/qutebrowser/misc/notree.py
index 297673bfb..e12a8bf3f 100644
--- a/qutebrowser/misc/notree.py
+++ b/qutebrowser/misc/notree.py
@@ -57,15 +57,15 @@ class TraverseOrder(enum.Enum):
         POST_R: post-order-reverse: like POST but rightmost nodes first.
     """
 
-    PRE = 'pre-order'
-    POST = 'post-order'
-    POST_R = 'post-order-reverse'
+    PRE = 'pre-order'  # pylint: disable=invalid-name
+    POST = 'post-order'  # pylint: disable=invalid-name
+    POST_R = 'post-order-reverse'  # pylint: disable=invalid-name
 
 
 uid_gen = itertools.count(0)
 
 # generic type of value held by Node
-T = TypeVar('T')  # pylint: disable=invalid-name
+T = TypeVar('T')
 
 
 class Node(Generic[T]):
@@ -187,7 +187,7 @@ class Node(Generic[T]):
     def __set_modified(self) -> None:
         """If self is modified, every ancestor is modified as well."""
         for node in self.path:
-            node.__modified = True  # pylint: disable=protected-access
+            node.__modified = True  # pylint: disable=protected-access,unused-private-member
 
     def render(self) -> List[Tuple[str, 'Node[T]']]:
         """Render a tree with ascii symbols.
@@ -217,11 +217,10 @@ class Node(Generic[T]):
                     result += [subtree[0]]
                 else:
                     result += subtree
+            elif child is self.children[-1]:
+                result.append((CORNER, child))
             else:
-                if child is self.children[-1]:
-                    result.append((CORNER, child))
-                else:
-                    result.append((INTERSECTION, child))
+                result.append((INTERSECTION, child))
         self.__modified = False
         self.__rendered = list(result)
         return list(result)
@@ -253,11 +252,17 @@ class Node(Generic[T]):
         if order in [TraverseOrder.POST, TraverseOrder.POST_R]:
             yield self
 
-    def __add_child(self, node: 'Node[T]') -> None:
+    def __add_child(  # pylint: disable=unused-private-member
+        self,
+        node: 'Node[T]',
+    ) -> None:
         if node not in self.__children:
             self.__children.append(node)
 
-    def __disown(self, value: 'Node[T]') -> None:
+    def __disown(  # pylint: disable=unused-private-member
+        self,
+        value: 'Node[T]',
+    ) -> None:
         self.__set_modified()
         if value in self.__children:
             self.__children.remove(value)
@@ -286,7 +291,7 @@ class Node(Generic[T]):
 
         """
         if to not in ['first', 'last', 'next', 'prev']:
-            raise Exception("Invalid value supplied for 'to': " + to)
+            raise ValueError("Invalid value supplied for 'to': " + to)
         position = {'first': 0, 'last': -1}.get(to, None)
         diff = {'next': 1, 'prev': 0}.get(to, 1)
         count = times
diff --git a/tests/unit/mainwindow/test_tabwidget.py b/tests/unit/mainwindow/test_tabwidget.py
index a249a6d1c..59defeb50 100644
--- a/tests/unit/mainwindow/test_tabwidget.py
+++ b/tests/unit/mainwindow/test_tabwidget.py
@@ -11,6 +11,7 @@ import pytest
 from unittest.mock import Mock
 
 from qutebrowser.qt.gui import QIcon, QPixmap
+from qutebrowser.qt.widgets import QWidget
 from qutebrowser.mainwindow import tabwidget
 from qutebrowser.utils import usertypes
 
@@ -21,7 +22,14 @@ class TestTabWidget:
 
     @pytest.fixture
     def widget(self, qtbot, monkeypatch, config_stub):
-        w = tabwidget.TabWidget(0)
+        class DummyParent(QWidget):
+            def __init__(self):
+                super().__init__()
+                self.is_shutting_down = False
+                self.show()
+
+        w = tabwidget.TabWidget(0, parent=DummyParent())
+        w.resize(640, 480)
         qtbot.add_widget(w)
         monkeypatch.setattr(tabwidget.objects, 'backend',
                             usertypes.Backend.QtWebKit)