| Age | Commit message (Collapse) | Author |
|---|
|
These two values are already defaulted in the parent class, so we don't need
to set them in the child (and weren't doing it consistently anyway).
I'm trying to identify places where we can't keep the tree tabs
knowledge self contained and speculate how we could handle that for
actually external code like extensions. So, thinking about how we would
gracefully handle some non-core code adding tab title format
placeholders, especially when that code could be disabled (eg, an
extension):
TabWidget will throw a KeyError if you have a format specifier which isn't
filled in. So we would have to decide how to handle that. Log a warning and
ignore it? Silently set it to `''`? Leave it as the literal `{thing}`, curly
braces and all? Presumable that API would look something like
`register_title_format_placeholder(placeholder: str, getter:
Callable[AbstractTab])`.
The `FormatString` config type used by `tabs.title.format` would have to
change as well to pull allowed placeholders from some global registry instead
of having the specified ahead of time in configdata.yml.
Even then, if you disabled an extension so it didn't register its placeholder
and you have the placeholder configured in your format string, what should
happen?
PS: jinja defaults to `''` for placeholders without values.
ref: #30
|
|
The method in this function where added in: 411d39a2e6e "Add config
option to enable tree-tabs". The `update_tab_titles()` line has the
comment "must also be called when deactivating" because back then
it was right before something gated on `if tabs.tree_tabs:`. But:
1. tree_tab_update() already calls update_tab_titles()
2. we don't currently support toggling tree tab off and on on an existing
window
So I don't believe the calls in this override are needed with the current
design. `_init_config()` is called on `__init__()` and `tabs.*` config
items changing. To double check if these is needed for anything I've
tried a bunch of things like:
1. change tab format
2. show/hide tab group
3. change tabs.position
4. show/hide tab bar - huh, when the tab bar is re-shown it's scrolled
down so on the title of the active tab and below are shown, is this
new? But that happens with a flat tabs window too. It only happens
sometimes though. Oh, it's not happening on 6.4.2 but is on 6.6.0 and I
don't remember it happending on 6.7.0. Must be a Qt issue that
hopefully got fixed.
And it all seemed to behave normally. I'm not sure what else to check
as everything else tree tab specific that I can think of relates to
moving tabs, which should be updating things already.
|
|
I don't believe this call is required, and disconnecting something that was
connected in the parent class could be surprising.
This was added in commit b8d9622aa64324 which says it was done for performance
reasons because `update_tab_titles()` many time when `:tree-tab-collapse` was
called on large trees.
I agree that `update_tab_titles()` is called lots. The tree operations could
be a bit more atomic. I did some work previously to prevent title updates
during update (for consistency reasons, not performance reasons) in dcee69f050ff5
that helps with the `:tree-tab-collapse` case. I can see that
`update_tab_titles()` is called once for each tab being hidden or show but it
now doesn't do any work (`self._tab_title_update_disabled` is `True`) for any
of the calls except that last one.
Surprisingly the `tabMoved` signal doesn't actually seem to be fired when
hiding or showing a tab group. Possibly the refactoring in late 2019
(e795eac9e1a7ce and friends) changed it so we aren't trigging that case
anymore.
|
|
qutebrowser/tree/8076_fix_loading_collapsed_tabs_and_legacy_session_support
Fix loading collapsed tabs and legacy session support
|
|
I got a failure in CI on windows for the "Load a collapsed subtree"
where it got an index error when trying to load the active history entry
for a tab in a session. This is likely because the session was saved
before the tab was loaded. I can reproduce it locally by changing the
last test to wait for only the first tab, instead of the last.
|
|
In 3ec2000b2f1c I identified a few cases where we could be asked to call into
the tree structure based on a widget index with evidence that the tree and
widget where out of sync. In particular where there were a different amount of
items in each.
Previously I had ignored desyncs during session loads but now I've found
a case where the desync remains after the session was loaded, for a
short time. It can be reproduced (most of the time) with:
python3 -m qutebrowser -T -s tabs.tree_tabs true ":open about:blank?1" ":open -tr about:blank?2" ":open -tr about:blank?3" ":tab-focus 2" ":tree-tab-toggle-hide" ":cmd-later 200 session-save foo" ":cmd-later 300 session-load -c foo"
This only happens if the last set of tabs we open is a collapsed group. There
would have been no update event fired since we set the collapsed attribute.
If this issue shows up again a more robust approach might be to not set the
collapsed attribute in the loop, but load all the tabs visible then go and set
the collapsed attribute on all the nodes that need it and fire an update.
I initially reproduced this with the "Load a collapsed subtree" end2end test.
|
|
The first three tests make sure that the "collapsed" attribute makes it
through the session machinery and behaves as expected.
Third one is a bit sneaky because it tests a case that was causing issues, but
this test couldn't actually reproduce. The problem was that before we were
passing `related=False` to `tabopen()` the tab after a collapsed group would
be loaded hidden when it wasn't supposed to be. This was only in the UI
though, if you saved a session it would be correct, so we can't test it in
these e2e tests currently...
|
|
With, presumably, rare combinations of settings tab order in windows
loaded from session can be different from the window the session was
saved from.
In particular with `tabs.new_position.related=prev` the tabs would be
loaded in reverse order!
This commit changes tabs loaded from sessions to 1) override positioning
related kwargs to tabopen so that it doesn't look at user sessions 2)
provide an explicit index to use for new tabs.
This particular test passes with either one of the newly passed kwargs.
We don't strictly need both of them, but while being explicit we may as
well override everything to be extra clear.
While looking at this code you may be asking why background is set to
True for all the tabs when only one of them is going to be focused? Good
question, I don't think it should be, see https://github.com/qutebrowser/qutebrowser/issues/6983#issuecomment-1025011641
But since that is explicitly set it should probably be a separate PR to
change it. Looks like the original PR to fix that got caught up in wider
scope and trying to do a new style of e2e-but-python tests. It should be
easy enough to write an e2e that loads a page with JS that reports that
visibility state.
TODO:
* add a similar test for tree tabs when the scaffolding is there (with
`new_position.tree.new_toplevel=prev` too)
* override sibling too?
|
|
Looks like when we refactored the session support to be backwards
compatible we forgot to assign tab_to_focus so it was ending up focusing
the last tab in the session for some reason. I guess that was just the
last one added to the tab.
|
|
We have one (1) use case where a caller wants to get all tabs from a
tabbed browser, without caring whether they are mounted in the tab bar
or not. I was wondering what an API would look like that let callers ask
that without having to worry about whether the tabbed browser had tree
tabs enabled or not, while still maintaining the more common use case of
only getting widgets that are visible, or focusable, to the user.
Right now the best option I can come up with is this
`tabs(include_hidden=True)` method. This provides a clear API that makes
it obvious how to achieve both use cases (apart from the overloaded
meaning of "hidden" regarding widget visibility). I think referring
to "tabs" is better than "widgets" too, it requires you to hold less
information in your head about what these widgets in particular are.
Am I going to put any effort into attempting to deprecate `.widgets()`?
Nah.
Despite what I think is a fine API I do have some concerns:
*extensibility* One of the goals of this attempt at tree tabs and
productionising it is to see what extensions to core logic would look
like without having to change core classes. I'm not sure if this is a
good example or not because the core class just has the `.widgets()` which
is very much tied to the QTabBar. It doesn't have the concept of tabs
that exist and are children of a TabbedBrowser but aren't in the
QTabBar. So it's a fundamentally new piece of information to express and
we can't just unilaterally return tabs that aren't in the tab bar
because I can see it'll break some things.
So perhaps it would be more fitting with the "extensibility case study"
refactor goal to require the caller to have more knowledge. But I still
don't want callers iterating trees if they don't have to be, so maybe
something like:
class TabbedBrowser:
def widgets():
...
class TreeTabbedBrowser:
def widgets(include_hidden=False):
...
tabbed_browser = ...
if isinstance(tabbed_browser, TreeTabbedBrowser):
tabs = tabbed_browser.widgets(include_hidden=True)
else:
tabs = tabbed_browser.widgets()
Since we are going to be merging this into the core codebase I don't
want to be inflicting that on users of tabs in the extensions API (when
we add them), but perhaps it's a pattern we can recommend for actual
extensions, when they show up.
*ownership and hidden tabs* For tabs associated with a window but not
visible in the tab bar, who should they be owned by? Currently
TabbedBrowser doesn't attempt to keep track of tabs at all, that's all
handled by the TabWidget. But now we have tabs that the TabWidget
doesn't know about. They are only referenced by the tree structure under
TabbedBrowser. Should the browser be managing tabs and just telling the
tab bar what to show? (Which is essentially what is happening now but
only for the tree case.) Or should we be pushing the tab hiding
functionality into the TabWidget?
Having tabs not in the tab bar can cause issues, mainly because it
causes violates some assumptions in the code that all tabs have an index
in the tab bar. But putting the tabs in the tab bar that you are not
showing can cause usability issues with, for example, navigating to a
tab via count.
When we implement our own tab bar we are probably going to re-do the
displaying of the tree structure to be a bit more "native". When we do
that should we move the tab hiding into the tab bar too? I guess the API
would end up looking like it does with the tree case today, hidden tabs
wouldn't get indices and you would want ways to both iterate through
visible tabs and all tabs (and maybe hidden tabs too, but we don't have
that currently without traversing the tree yourself). I imagine in that
case the tree structure would be part of the tab bar and in the "flat"
case it would just always put stuff at the top level, and complain if
you tried to demote.
That rambling was mostly driven by so many callers going
`tabbed_browser.widget.currentTab()` etc. Which I don't think is a great
API. Beyond an attribute called "widget" being not very meaningful have
tab accessing and manipulating commands spread between and parent and a
child seems like it could be confusing. So I settled on the future logic
being in the tab widget (or in a tree in the tab widget), does that
change anything or dictate what they should be owned by? No. Oh well. I
kinda want to say tabs should be owned by the tabbed browser but I don't
know what practical implications that would have. The interface to
callers would remain predominantly the tabbed browser and the logic
would probably continue to be mixed between that and the tab widget.
|
|
We are now iteration over all tabs, including hidden ones that aren't in the
tab widget. So we can't use the index of the current tab within the tab widget
as a key into that list. Check the actual tab value instead.
I made `TabbedBrowser._current_tab()` public because it's useful, so why not! I
could have used `tabbed_browser.widget.currentWidget()` like some other places
do but if we where doing proper type checking in session.py we would also have
to do the None check that is already being done in TabbedBrowser. Also
accessing `some_other_object.widget` feels a little dirty, like it should be
private?
|
|
|
|
In a2f98aa78b52 the session format for tree tab windows was changed to be
backwards compatible with the prior format. After that change we can load
non-tree tab windows fine but the browser crashes on startup when trying to
load session in the now-legacy tree tab session format.
Since people have been using the now-legacy session for years and there is no
way for them to easily migrate I figure we have a few options:
* provide a script to help them migrate
* handle the error encountered (`KeyError: 'tabs`) when trying to load a
legacy session better, eg give them a nice apologetic message
* support loading the old style of sessions
It didn't take much effort to support the prior format, so I propose we do
that. The data is all the same, we just changed it to nodes are childs of tabs
instead of the other way around.
|
|
Tabs in collapsed tree groups aren't currently listed in
tabbed_browser.widgets(), so they weren't getting saved.
The tabs are still referenced by the tree structure though so grab them
from there if we are dealing with a tree tabs window.
I'm not too happy that we are now checking `if
tabbed_browser.is_treetabbedbrowser` twice in the same methods. Seems a
bit smelly, but oh well.
Things we could do to help makes iterating tabs less error prone:
* Make TreeTabbedBrowser.widgets() return widgets from the tree instead
of the tab widget (would that break other use cases that called that that
expected them to always be in the tab widget?)
* Have tests to make sure this functionality doesn't break again in the
future
|
|
Add end2end tests for tree tabs feature
|
|
|
|
So that we can use it from test functions. And add a docstring as a result.
|
|
Merges the pre-existing `check_open_tabs()` and the temporary
`check_open_tabs()`. There's lots of differences here:
1. the previous implementation of the existing method compared parsed parts
out of the expected strings and compared them with attributes of session
entries. Here we are converting session entries into the same strings as the
expected format and comparing them. I find the diff that pytest shows is
great for highlighting which parts of the strings differ and seeing the
whole string in context in that case is useful.
2. we've added the "(collapsed)" suffix, I resisted the urge to try to reduce
the number of variables we have related to the three suffixes
3. we now support tree tab windows by 1) forming a "tree" dict with a helper
from sessions.py 2) traversing the tree if available 3) supporting a prefix
on the string format and adding that based on the depth in the tree
4. updated a bunch of variable names and moved a few lines around for clarity,
eg tabs -> expected_tabs, session['windows'][0] -> window
The `normalize()` method sorts the suffixes so you can write them in any order
(the ordering for the string formed from session is based on the order of the
suffixes in the dict).
Lastly I updated the docstring :)
|
|
TODO:
* make it pass tabs.feature (normalize on both sides)
* clean up (variable names in particular)
* align with check_open_tabs, in particular see if that has better error
messages with it's more nuanced checking vs this attempt that compares
a whole list of strings
|
|
Recurse through the tree structure and build up a simplified string
representation of a window in a tree tab session so we can compare it to a
test fixture. It should look something like this:
- about:blank?one
- about:blank?child (collapsed)
- about:blank?grandchild
- about:blank?childtwo (active)
- about:blank?two
TODO:
* align with `check_open_tabs()` - I think we should be able to adapt that
one to be the same format as this. Eg construct a string from the session
and then compare the two strings line by line. Then the comparison
shouldn't need to care if it's a tree or not. `tree_to_str()` could maybe
be made more generic too, or at least pull the "flat tab to str" bit out of
it and then slap the tree bit on afterwards if needed
* now we've got three individual suffixes handling them one by one is a bit of
a pain, can we put them in a dict and iterate over that instead?
* add some sanity checks instead of blindly doing array indexing etc, I think
it's hard to get stack traces from generators?
|
|
Had a think about how to get tree structure things to compare. For now
I'm thinking we create a string of the format:
- about:blank?one
- about:blank?child (collapsed)
- about:blank?grandchild
- about:blank?childtwo (active)
- about:blank?two
From both a) the text fixture (we write it like that) and b) the session
data (generate it). Then we just compare the strings, hopefully much
like check_open_tabs and maybe we can even make one implementation work
for both.
The next bit to do is generate the above text from the session data. We
can start by loading the tree structure with
`SessionManager._reconstruct_tree_data()` (TODO: make that public) and
then we need to recurse through that. We can take inspiration from
`recursive_load_node()` in sessions.py
Then copy the rest of the implementation from `check_open_tabs()` and
adapt that.
And then look at `quteproc.compare_session()` as well I guess. But it
looks like it'll be a pain to write out the whole tree tab format
sessions...
|
|
Background section:
* use the new Given I open ... to open a new window after enabling tree
tabs, so we can actually test with a tree tabbed window
* an alternative is to change the "fresh instance" given step to take
-s settings, but that seems a little more expense
* move "clean up open tabs" to after we've opened a new window
Scenario section:
* change to opening related tabs so that they are nested and get closed
recursively (also "I open" doesn't know about siblings yet)
Other:
* add a test_treetabs_bdd.py file - don't we need these to run the
features? Maybe I'm missing something and have been doing it wrong...
TODO:
* do we need to clean up the tabs.tree_tabs setting? If so how? Or do we
get a new instance for each Feature file?
* why is `set tabs.tabs_are_windows to false` there? Do we need that?
Lets see in CI I guess
Co-authored-by: pylbrecht <pylbrecht@mailbox.org>
|
|
Previously only "I open" in the "When" (aka "act") block supported suffixes
like "in a new windows". The tree tabs setting only applies to new windows so
I want to open a new window after setting the setting.
So I changed the given `open_path` to call the other `open_path`, then had to
support the difference between them: the Given version always opens the path
in a new tab. I changed that to open in a new tab *by default*.
It was a bit tricky that just eg passing in a `default_new_tab` arg and then
passing `new_tab=True` to quteproc command because then if I ended up passing
`new_window` based on a suffix it rightly complained that those two args
conflicting. So I only really want to set `new_tab=True` if no suffixes are
detected. Instead of adding a new `suffix_found = True` line to every part of
the if chain I converted it to loop over a dict.
I didn't think too hard about the flexible typing of the values of the suffix
map. Just did something for every case I observed. It probably works fine.
|
|
|
|
|
|
Fix lint and tests on tree tabs integration branch
|
|
|
|
Various lint fixes. The ones we will likely revisit later are:
* unused sibling arg in parent TabbedBrowser class
* seperate-but-similar UndoEntry implementations
* variables with two leading underscores in notree
* `_tree_tab_give()` implementation
qutebrowser/browser/commands.py:499:9: F841 local variable 'oldroot' is assigned to but never used
In `_tree_tab_give()` there is a check to see if a node being moved needs to
be re parented. That used to use `is not oldroot` and I changed it in
c2bc549605a3c to be `parent.uid in uid_map`, eg is this a child node of the
tree being moved. Now we don't need oldroot anymore but I forgot to clean it
up.
PS: not sure why that check is so complicated. Won't it only ever be false for
the top node being moved? So we can just say `if node !=
current_widget.node:`? I guess that's irrelevant if that's all going to be
re-written anyhow.
qutebrowser/mainwindow/treetabbedbrowser.py:83:1: D202 No blank lines allowed after function docstring
qutebrowser/mainwindow/treetabwidget.py:77:22: C419 Unnecessary list comprehension passed to all() prevents short-circuiting - rewrite as a generator.
qutebrowser/mainwindow/treetabwidget.py:78:13: C1804: "self.widget(idx).url().toString() == ''" can be simplified to "not self.widget(idx).url().toString()", if it is striclty a string, as an empty string is falsey (use-implicit-booleaness-not-comparison-to-string)
qutebrowser/mainwindow/treetabwidget.py:132:16: R5501: Consider using "elif" instead of "else" then "if" to remove one indentation level (else-if-used)
Ran out of energy at the double negative. Hope that's okay.
qutebrowser/mainwindow/treetabbedbrowser.py:131:35: W0613: Unused argument 'idx' (unused-argument)
Add an unused-argument ignore line because, while we aren't using index (tab
undo entries remember their parents instead of their index) I think we'll be
pulling this method up to the parent class later which does use it.
qutebrowser/mainwindow/treetabbedbrowser.py:198:4: W0237: Parameter 'idx' has been renamed to 'sibling' in overriding 'TreeTabbedBrowser.tabopen' method (arguments-renamed)
Urghgh, I have no idea why this would be like this. I looked through the
codebase both for `sibling` and `tabopen` and didn't see this being called
with positional arguments beyond the url. So seems safe to change it.
Probably some linter will be complaining that it still doesn't match the
signature of the parent...
qutebrowser/mainwindow/treetabbedbrowser.py:19:0: W0611: Unused log imported from qutebrowser.utils (unused-import)
qutebrowser/mainwindow/treetabbedbrowser.py:214:0: I0021: Useless suppression of 'arguments-differ' (useless-suppression)
And I guess this is the complaint about the arguments differing. But it says we
don't need the suppression anymore? We'll, I'll add it to the parent and see
what that does.
qutebrowser/misc/notree.py:60:4: C0103: Class constant name "PRE" doesn't conform to snake_case naming style (invalid-name)
qutebrowser/misc/notree.py:61:4: C0103: Class constant name "POST" doesn't conform to snake_case naming style (invalid-name)
qutebrowser/misc/notree.py:62:4: C0103: Class constant name "POST_R" doesn't conform to snake_case naming style (invalid-name)
I assume these are copied from anytree. Not a huge fan, why are the variables
names so much less readable than the values?
qutebrowser/misc/notree.py:221:16: R5501: Consider using "elif" instead of "else" then "if" to remove one indentation level (else-if-used)
qutebrowser/misc/notree.py:289:12: W0719: Raising too general exception: Exception (broad-exception-raised)
`promote()` is only called from one place in commands.py currently. So we are
essentially validating config, so this error "shouldn't happen". If this was
going to be called from more places we should probably make an enum in notree
and maybe generate the config from that? Or validate it against the config (in
a test?) or something.
qutebrowser/misc/notree.py:256:4: W0238: Unused private member `Node.__add_child(self, node: 'Node[T]')` (unused-private-member)
qutebrowser/misc/notree.py:260:4: W0238: Unused private member `Node.__disown(self, value: 'Node[T]')` (unused-private-member)
qutebrowser/misc/notree.py:190:12: W0238: Unused private member `Node.__modified` (unused-private-member)
I assume these are copied from anytree. The internet says variables with two
leading underscores are mangled by the interpreter to be
`__classname__variable` so that you don't have to worry about child and parent
classes fighting over variable names.
We don't have any examples of this pattern in the codebase currently and this
Node class seems designed for composition, not inheritance. So I think I would
prefer to change them to have a single underscore. That seems like something
that people might want to discuss though, so for now silence pylint as it's
presumably comparing some mangled vs unmangled name (eg if I rename them it's
happy).
qutebrowser/misc/notree.py:68:0: I0021: Useless suppression of 'invalid-name' (useless-suppression)
|
|
In 8aa88b83e I added a dependency in the TabWidget in
`parent.is_shutting_down` (assuming parent is a TabbedBrowser, but
without updating the type hints...).
Since the benchmark test wasn't setting a parent it's been breaking
since then, oops.
That's going to be breaking all the branches based off of the tree tabs
branch, so we should probably cherry pick this back to the
tree-tabs-integration branch. And consider updating the
TabWidget parent kwarg type hint (and make the dummpy parent a
mock/spy/double of the TabbedBrowser I guess).
Not sure where the size of 640x480 came from before hand but when I
added a parent it went to 100x30.
|
|
|
|
|
|
|
|
The command was renamed a while back.
|
|
|
|
- Don't split the pattern in weird places used for str.join, instead
build up the individual groups and then "".join them, which seems more
readable to me.
- Don't reuse "val" for both the user-input pattern and the regex pattern.
|
|
With the change in #7955, if the regex did not match, it ended up retrying the
lookahead at every position of the string (expected), but then *also* repeated
that process for every position in the string. Thus, a non-matching pattern
ended up in O(n^2) performance (with n = length of URL).
Instead, anchor the pattern at the beginning of the string. This doesn't change
behaviour as we use .* at the beginning of every lookahead anyways, but it means
we end up with O(n) instead of O(n^2) performance.
Co-authored-by: toofar <toofar@spalge.com>
|
|
This reverts commit 6a149901fbd91ac2b8b6cb39fbc835a777319c2e.
|
|
This reverts commit 45d483479aebb6af5a53aea97d65228d8621ad86.
|
|
Typo fix
|
|
Trivial typo fix
|
|
This reverts commit dbd0bfbe8e19b173e39b1536a52907eb28557773.
See previous commit
|
|
This reverts commit fe1faa14b91253db99e9b4451ffb1a479feff1db, reversing
changes made to 04af4c657d3725fd2d3b556d9b88b44d3ada0be1.
For my setup with 310 quickmarks and 94 bookmarks, this makes qutebrowser hang
for around a minute on every keypress.
|
|
https://docs.pytest.org/en/7.4.x/deprecations.html#py-path-local-arguments-for-hooks-replaced-with-pathlib-path
https://docs.pytest.org/en/7.4.x/reference/reference.html#std-hook-pytest_ignore_collect
8.0.0 removes the "path" compatibility alias we used.
|
|
As shown in detail by toofar in dff25d10bcc8a398a77ef260c33e51ec5c70f482,
mesa 23.3 seems to trigger some new error messages.
On my setup locally, it also triggers the warning that's ignored here.
Why that's not the case on CI or for others is unclear to me, I gave up
trying to understand graphics stuff on Linux.
I'll just presume this is the same underlying cause, and refer to the
commit message of the commit above for more details.
|
|
Update dependencies
|
|
|
|
TODO:
* convert to asciidoc or make scripts/asciidoc2html.py support markdown
input too.
* for now I'm using `pandoc -f markdown -t html -o qutebrowser/html/doc/treetabs.html doc/treetabs.md`
* should be able to asciidoc with pandoc too
* the last section and the rest of the second to last
* review and refactor
* post on the PR and figure out how we can contribute to it collectively
(put on main branch so people can open PRs? create new integration
repo where people can open issues, discussions and PRs?)
* maybe add more examples showing the effect of various operations
* (when including in a release) consider removing the implementation
section, or refactoring it to be more high level broad strokes and
less low level stuff that will inevitably drift from the actual
implementation
These docs are not supposed to be in their final state. My plan for them
so far is to provide a guide to the changes for PR contributors. I think
it's currently difficult to hold the scope of the changes in your head
the description on the PR is currently very limited.
I would like to encourage discussion of usability and feature set as
well as highlighting some implementation details to point out upsides,
downsides, implications and alternatives.
I've modified asciidoc2html to copy all images over. It was only doing
specific ones before. But there are more images in there, why are they
in doc/img if they aren't images used in the documentation? Anyway,
storage is cheap, none of the images are sensitive and not having to
hardcode individual files names is good.
|
|
I was looking through some commit messages from last week and noticed I
was having trouble keeping track of what these traverse orders meant. So
I tried to adjust the descriptions to be more clear. Hopefully it
worked?
Probably could still use a visual aid though, or the enum value names
could even be changed to be more clear, like "parent-first",
"child-first", "child-first-reverse/-right".
The test I used to verify this description was:
simple_tree = Node('parent')
left = Node('left', simple_tree)
Node('grandleft-left', left)
Node('grandleft-right', left)
Node('middle', simple_tree)
right = Node('right', simple_tree)
Node('grandright', right)
# parent
# ├─ left
# │ ├─ grandleft-left
# │ └─ grandleft-right
# ├─ middle
# └─ right
# └─ grandright
@pytest.mark.parametrize('order, expected', [
(TraverseOrder.PRE, "parent left grandleft-left grandleft-right middle right grandright",),
(TraverseOrder.POST, "grandleft-left grandleft-right left middle grandright right parent",),
(TraverseOrder.POST_R, "grandright right middle grandleft-right grandleft-left left parent",),
])
def test_simple(order, expected):
assert expected == " ".join(
[n.value for n in simple_tree.traverse(order)]
)
|
|
When recursive closing a tree group you will be prompted if you want to
close any tabs that are pinned. Except we passed the topmost tab to the
prompt method every time instead of the tab we were going to close.
This fixes that. It does mean that if there are multiple pinned tabs in
a tree it gives you an identical prompt for each one, which no
indication that they are separate prompts. It looks like nothing is
happening when you answer it. But that's a separate problem (probably
the solution is to either prompt once or to put URLs and/or tab IDs in
the prompt).
|
|
Seems a step from the existing code path wasn't applied to the
new tree-specific code. I've pulled it out to be applied to both paths.
Previously if you ran gD (tab-give without a positional argument) it
would open a new window but you couldn't see it!
|
