Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Retain sort order for undo completion model #5964

Merged

Conversation

toofar
Copy link
Member

@toofar toofar commented Dec 16, 2020

Because 10 shouldn't be higher than 2.

image

@toofar toofar requested a review from rcorre as a code owner December 16, 2020 08:42
Because 10 shouldn't be higher than 2.
@toofar toofar force-pushed the fix/dont_sort_undo_completion branch from b19b7df to f0ed206 Compare December 16, 2020 09:03
Copy link

@rcorre rcorre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@The-Compiler The-Compiler merged commit 3307df4 into qutebrowser:master Dec 22, 2020
@The-Compiler
Copy link
Member

Thanks @toofar and @rcorre!

toofar added a commit that referenced this pull request Mar 10, 2024
I've made TreeUndoEntry a subclass of UndoEntry. But UndoEntry sets a
default value for the `created_at` attribute, which means if any of the
extra attributes defined on the child class don't define a default an
error is thrown complaining about non-keyword arguments after a keyword
only one. Python 3.10 adds a kw_only argument you can pass to the
dataclass decorator but it looks like we currently support python 3.8+.
See https://www.trueblade.com/blogs/news/python-3-10-new-dataclass-features

So a more compatible way to do it is to set `init=False` on the
attribute with a default, if you don't need to pass it in the
constructor. Which we don't because it's only set by the caller in
tests. Hence changing the tests to set it a different way. A bit uglier
but should be fine.

Also fix my "retains_sort_order" test from #5964 that wasn't actually
being run...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants