diff options
author | toofar <toofar@spalge.com> | 2023-12-22 17:18:26 +1300 |
---|---|---|
committer | toofar <toofar@spalge.com> | 2024-01-06 20:50:59 +1300 |
commit | ba9aee92f45f8a183c1e1d70420b853cb00871e5 (patch) | |
tree | 8efdf1392c09a12e9b85fe73100bfbd386038149 | |
parent | fb6345aa1bc4ceac073c475b60c831cd27332da1 (diff) | |
download | qutebrowser-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.py | 42 |
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): |