Skip to content

Commit

Permalink
Don't special-case user_value for choice symbols set to y
Browse files Browse the repository at this point in the history
Previously, setting a choice symbol to y would only update
user_selection on the parent choice and not the symbol's own user_value.
Now both are updated.

The point of the old behavior was to remember the m mode selections of a
choice when it was switched back and forth between m and y mode, which
was a feature I thought the C implementation had. On closer inspection,
the C implementation never had that feature, though it might appear like
it if you only make "lucky" changes (if you never select any symbols in
y mode that were n in m mode).

The new behavior is simpler and easier to understand: Symbol.user_value
now always matches the value assigned in a .config file or via
set_value(), provided the value was well-formed. This might avoid some
special-casing in scripts too.

The loss in usability is pretty minimal.
  • Loading branch information
ulfalizer committed Feb 8, 2018
1 parent 2abc0bb commit e8b4ecb
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 33 deletions.
27 changes: 16 additions & 11 deletions kconfiglib.py
Original file line number Diff line number Diff line change
Expand Up @@ -2700,9 +2700,8 @@ def set_value(self, value):
'assignable' will cause Symbol.user_value to differ from
Symbol.str/tri_value (be truncated down or up).
Setting a choice symbol to 2 (y) only updates Choice.user_selection on
the parent choice and not Symbol.user_value itself. This gives the
expected behavior when a choice is switched between different modes.
Setting a choice symbol to 2 (y) sets Choice.user_selection to the
choice symbol in addition to setting Symbol.user_value.
Choice.user_selection is considered when the choice is in y mode (the
"normal" mode).
Expand All @@ -2727,9 +2726,14 @@ def set_value(self, value):
value of the symbol. For other symbol types, check whether the
visibility is non-n.
"""
if value == self.user_value:
# We know the value must be valid if it was successfully set
# previously
# If the new user value matches the old, nothing changes, and we can
# save some work.
#
# This optimization is skipped for choice symbols: Setting a choice
# symbol's user value to y might change the state of the choice, so it
# wouldn't be safe (symbol user values always match the values set in a
# .config file or via set_value(), and are never implicitly updated).
if value == self.user_value and not self.choice:
self._was_set = True
return True

Expand Down Expand Up @@ -2763,16 +2767,17 @@ def set_value(self, value):
if self.orig_type in (BOOL, TRISTATE) and value in ("n", "m", "y"):
value = STR_TO_TRI[value]

self.user_value = value

if self.choice and value == 2:
# Remember this as a choice selection only. Makes switching back
# and forth between choice modes work as expected, and makes the
# check for whether the user value is the same as before above
# safe.
# Setting a choice symbol to y makes it the user selection of the
# choice. Like for symbol user values, the user selection is not
# guaranteed to match the actual selection of the choice, as
# dependencies come into play.
self.choice.user_selection = self
self.choice._was_set = True
self.choice._rec_invalidate()
else:
self.user_value = value
self._was_set = True
self._rec_invalidate_if_has_prompt()

Expand Down
35 changes: 13 additions & 22 deletions testsuite.py
Original file line number Diff line number Diff line change
Expand Up @@ -1550,12 +1550,10 @@ def select_and_verify(sym):
sym.name + " should be the user selection of the choice")

verify(sym.tri_value == 2,
sym.name + " should be y when selected")
sym.name + " should have value y when selected")

verify(sym.user_value != 2,
sym.name + " should not have user value y, because choice "
"y mode selections are remembered on the choice "
"itself")
verify(sym.user_value == 2,
sym.name + " should have user value y when selected")

for sibling in choice.syms:
if sibling is not sym:
Expand Down Expand Up @@ -1628,28 +1626,21 @@ def verify_mode(choice_name, no_modules_mode, modules_mode):
# Test m mode selection

c.named_choices["TRISTATE"].set_value(1)

verify(c.named_choices["TRISTATE"].tri_value == 1,
"TRISTATE choice should have mode m after explicit mode assignment")

assign_and_verify_value("T_1", 0, 0)
assign_and_verify_value("T_2", 0, 0)
assign_and_verify_value("T_1", 1, 1)
assign_and_verify_value("T_2", 1, 1)
assign_and_verify_value("T_1", 2, 1)
assign_and_verify_value("T_2", 2, 1)

c.syms["T_1"].set_value(0) # Check that this is remembered later

# Switching to y mode should cause T_1 to become selected
# Switching to y mode should cause T_2 to become selected
c.named_choices["TRISTATE"].set_value(2)
verify_value("T_1", 2)
verify_value("T_2", 0)

# Switching back to m mode should restore the old values
c.named_choices["TRISTATE"].set_value(1)
verify_value("T_1", 0)
verify_value("T_2", 1)

assign_and_verify_value("TM_1", 1, 1)
assign_and_verify_value("TM_1", 2, 1) # Ignored
verify(c.named_choices["TRISTATE"].tri_value == 1,
"m-visible choice got invalid mode")

assign_and_verify_value("TM_1", 0, 0)
assign_and_verify_value("TM_1", 2, 0) # Ignored
verify_value("T_2", 2)

# Verify that choices with no explicitly specified type get the type of the
# first contained symbol with a type
Expand Down

0 comments on commit e8b4ecb

Please sign in to comment.