Conversation
A SettingsQR that picks exactly one option gives a bare value rather than a list -- "seedlen=24" parses as an int. It passes parsing and the review screen, and then set_value() refuses it, so Apply ends on the unhandled-error screen with the keys ahead of it in the dict already applied and the navigation history cleared. Wrap a lone value in a list where the other multiselect shapes are cleaned, which covers the settings.json path as well. seed_word_lengths is the only int-valued multiselect today, but nothing about this was specific to it.
This branch has not been deployed
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Description
Problem or Issue being addressed
Applying a SettingsQR that picks exactly one seed word length —
seedlen=24, what a 24-word-only user would write — passes the review screen and then crashes on Apply to the unhandled-error screen, with navigation history cleared and the settings ahead of that key already applied.A multiselect setting given one value arrives as a bare value (here an int), and
set_value()requires a list.Solution
In
Settings.update(), where multiselect values are already cleaned, wrap a single non-list value in a list. That covers settings.json as well as SettingsQR.Nonestill falls back to the defaults.Additional Information
The generic test applies one value to every multiselect setting except the smartcard interfaces, which power USB ports and restart PCSC; it fails, rather than runs, if any setting tries to run a command.
Screenshots
N/A — no screen change.
This pull request is categorized as a:
Checklist
I ran
pytestlocallyFork CI's
test (3.10)andtest (3.12)lanes (the full suite, screenshots included) are green on this exact code. The desktop matrix was left to this repository's CI: the change touches no platform code.I included screenshots of any new or modified screens
I added or updated tests
I tested this PR hands-on on the following platform(s):
Not hardware-tested: the change is dictionary handling after the QR is decoded; scanning is unchanged.
I have reviewed these notes: