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
localization: do not crash on failed parsing of X layout (#1836047) #2633
localization: do not crash on failed parsing of X layout (#1836047) #2633
Conversation
Also adds unit test for the case and more localed wrapper testing (67% -> 98% coverage of localed.py)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
layouts.append(layout) | ||
variants.append(variant) | ||
|
||
if not layouts and parsing_failed: | ||
return | ||
|
||
layouts_str = ",".join(layouts) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What will happen here if parsing will be successful but there won't be any layouts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, I think it that by the logic here, it can't happen.
Or if you mean the case where the method is called with no nonempty layout_variants, then the behaviour should be preserved (which I think is effectively unsetting the X Layouts by supplying empty strings to the SetX11Keyboard). I don't intend to change it in this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, thanks for explanation.
jenkins, test this please |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
jenkins, test this please |
1 similar comment
jenkins, test this please |
Also adds unit test for the case and more localed wrapper testing
(67% -> 98% coverage of localed.py)