summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authortoofar <toofar@spalge.com>2023-12-22 17:18:26 +1300
committertoofar <toofar@spalge.com>2024-01-06 20:50:59 +1300
commitba9aee92f45f8a183c1e1d70420b853cb00871e5 (patch)
tree8efdf1392c09a12e9b85fe73100bfbd386038149
parentfb6345aa1bc4ceac073c475b60c831cd27332da1 (diff)
downloadqutebrowser-ba9aee92f45f8a183c1e1d70420b853cb00871e5.tar.gz
qutebrowser-ba9aee92f45f8a183c1e1d70420b853cb00871e5.zip
Remove non-specific IndexError
Previously there was a `tree_prefix is empty!` error being printed in a couple of situations. 1) at startup 2) when hiding a tree group with multiple tabs in it. The main problem with the existing code is that it was catching a generic IndexError without having a strong indication of what the actual cause of this error was. We get called from `update_tab_titles()` in the base TabWidget with a valid tab index. If we don't have information for that tab index in our tree structure that could be an indication of a real error and we should go to some effort to identify and prevent the cause. This commit addresses cause 1 (at startup) by checking if the tree is empty and returning early if so. Ideally we would have an even more specific indication of being in startup (eg self.initialised == False) but we don't have have a variable indicating a startup is in progress currently (we do have one for shutting down!). TODO: figure out when the tree is constructed vs when the tab widgets are inserted and figure out if we can construct the tree earlier. The second part of the commit identifies and documents the cause of 2 (collapsing a group) but leaves the error messages in for now because I want to do a more proper fix that would let us continue to identify this situation if we got here via another call path in the future. This error was only being printed when hiding a group, not showing it, because when showing a group there was more entries in the tree than in the widget. So the function was still returning wrong data, it just wasn't being verified at all. The two remaining error messages are using a {foo=} f-string feature that I don't think is available in all the python versions we support.
-rw-r--r--qutebrowser/mainwindow/treetabwidget.py42
1 files changed, 32 insertions, 10 deletions
diff --git a/qutebrowser/mainwindow/treetabwidget.py b/qutebrowser/mainwindow/treetabwidget.py
index 36dc81ce4..d3d77d0eb 100644
--- a/qutebrowser/mainwindow/treetabwidget.py
+++ b/qutebrowser/mainwindow/treetabwidget.py
@@ -32,20 +32,42 @@ class TreeTabWidget(TabWidget):
"""Add tree field data to normal tab field data."""
fields = super().get_tab_fields(idx)
- tab = self.widget(idx)
- fields['collapsed'] = '[...] ' if tab.node.collapsed else ''
+ if len(self.tree_root.children) == 0:
+ # Presumably the window is still being initialized
+ log.misc.vdebug(f"Tree root has no children. Are we starting up? fields={fields}")
+ return fields
- # we remove the first two chars because every tab is child of tree
- # root and that gets rendered as well
rendered_tree = self.tree_root.render()
- try:
- pre, _ = rendered_tree[idx+1]
- tree_prefix = pre[2:]
- except IndexError: # window or first tab are not initialized yet
- tree_prefix = ""
- log.misc.error("tree_prefix is empty!")
+ # We can be called when the count of tabs in the widget is different
+ # to the size of the rendered tree. This is known to happen when
+ # hiding a tree group with multiple tabs in it. render() will reflect
+ # the final state right away but we get called for every removal or
+ # insertion from the tab widget while update_tree_tab_visibility() is
+ # traversing through the tree group to update the widget.
+ # There may be other cases when this happens that we would be
+ # swallowing here. To avoid that, since we get called via
+ # update_tab_titles() possibly it would be cleanest to add an
+ # attribute to TabWidget (or a context manager) to disabled tab title
+ # updates and set that while calling
+ # update_tree_tab_{visibility,positions} in tree_tab_update().
+ miscount = len(rendered_tree) - 1 - self.count()
+ if miscount < 0:
+ log.misc.error(f"Less nodes in tree than widget. Are we collapsing tabs? {idx=} {miscount=} {fields['current_url']=}")
+ return fields
+ elif miscount > 0:
+ log.misc.error(f"More nodes in tree than widget. Are we revealing tabs? {idx=} {miscount=} {fields['current_url']=}")
+ return fields
+
+ # we remove the first two chars because every tab is child of tree
+ # root and that gets rendered as well
+ pre, _ = rendered_tree[idx+1]
+ tree_prefix = pre[2:]
fields['tree'] = tree_prefix
+
+ tab = self.widget(idx)
+ fields['collapsed'] = '[...] ' if tab.node.collapsed else ''
+
return fields
def update_tree_tab_positions(self):