menuconfig: avoid crashing when leaving menu not in parent #4
Conversation
stephanosio
force-pushed
the
pr/menuconfig/fix_leaving_menu
branch
from
7e4048c to
bdee7ed
Compare
|
Rebased |
tejlmand
requested review from
tejlmand
and removed request for
jackrosenthal
tejlmand
left a comment
tejlmand
left a comment
There was a problem hiding this comment.
I prefer to handle the situation gracefully, so that other (unrelated) ValueErrors are not caught in the process.
menuconfig.py
Outdated
| _sel_node_i = _shown.index(_cur_menu) | ||
| except ValueError: | ||
| # The parent actually does not contain the current menu (e.g., symbol | ||
| # search). So we jump to the first node instead. |
There was a problem hiding this comment.
this is not really jumping anywhere.
_sel_node_i is the index of the currently selected node.
That said, pointing to index 0 (first node in menu) should be fine.
menuconfig.py
Outdated
| try: | ||
| _sel_node_i = _shown.index(_cur_menu) | ||
| except ValueError: | ||
| # The parent actually does not contain the current menu (e.g., symbol | ||
| # search). So we jump to the first node instead. | ||
| _sel_node_i = 0 |
There was a problem hiding this comment.
I would prefer to have proper handling so that errors are still propagated, so that other ValueErrors are not unexpectedly caught here.
For example like this instead:
| try: | |
| _sel_node_i = _shown.index(_cur_menu) | |
| except ValueError: | |
| # The parent actually does not contain the current menu (e.g., symbol | |
| # search). So we jump to the first node instead. | |
| _sel_node_i = 0 | |
| if _cur_menu in _shown: | |
| _sel_node_i = _shown.index(_cur_menu) | |
| else: | |
| # `_cur_menu` is not shown, this can happen when an invisible choice is | |
| # extending a choice defined elsewhere. In this case, point to node at index 0. | |
| _sel_node_i = 0 |
There was a problem hiding this comment.
Alright, I adapted the fix.
leandrolanzieri
force-pushed
the
pr/menuconfig/fix_leaving_menu
branch
from
bdee7ed to
0a132f2
Compare
| parent = _parent_menu(_cur_menu) | ||
| _shown = _shown_nodes(parent) | ||
| _sel_node_i = _shown.index(_cur_menu) | ||
| _cur_menu = parent |
There was a problem hiding this comment.
sorry for not noticing in first round, but why are you removing this line ?
_cur_menu = parent
There was a problem hiding this comment.
Oops, sorry that shouldn't be the case. I accidentally removed it in the last change. I'll add it back.
This handles a case of leaving a menu which is not shown by the parent menu. This might occur, for instance, when searching for the symbol of a named choice, which is extended by an invisible symbol somewhere else.
leandrolanzieri
force-pushed
the
pr/menuconfig/fix_leaving_menu
branch
from
0a132f2 to
d25a433
Compare
|
Thanks for the review @tejlmand. It's nice to finally see this in. |
3 participants
This adds a catch to an exception that might occur when leaving a menu which is not shown by the parent menu. This might occur, for instance, when searching for the symbol of a named choice.
Originally ulfalizer/Kconfiglib#94
See ulfalizer/Kconfiglib#93 for an example of the bug.