Handle no selectionmodel on clear selection #8200
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Change `CompletionView.on_clear_completion_selection()` to call the Qt selection model getter, instead of our one. Since it can be called when the selection model has already been cleared and our one asserts that there is a selection model to return. Back in the distant past there was a change to handle the completion widget's selection model being None when the `on_clear_completion_selection()` slot was called: 88b522fa167e2f97b More recently a common getter for the selection model was added so we could have a single place to apply type narrowing to the returned object from the Qt getter (the type hints had been updated to be wrapped in `Optional`): 92dea988c01e745#diff-1559d42e246323bea35fa064d54d48c990999aaf4c732b09ccd448f994da74cf It seems this is one place where it does, and already did, handle that optional. So it didn't need to change to use the new getter. This is called from the `Command.on_mode_left` signal, not sure why the selection model is None here. Perhaps it already gets cleared by the effects of the `hide_cmd` signal that gets fired earlier, or perhaps even from the `self.hide()` on the line before. Anyway, this was working for several years and seems harmless enough.
rcorre
approved these changes
May 19, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
on_clear_completion_selection()back to call the Qt getter instead of our one that does anassert not Nonecheck, it already handles None being returned fineI didn't put much effort into reproducing it beyond manually hammering the completion in a temp basedir (possibly if the completion was slower because of more items it would be easier to reproduce). That commit history was pretty clear, we've always handled this situation (whatever it is) until adapting to type hints recently.
An alternate implementation is adding an
allow_noneor similar kwarg to our getter.Fixes: #7901