Skip to content
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

Ensure watcher isn't called on first_set when init is False and value doesn't change #1367

Merged
merged 3 commits into from
Dec 20, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/).

- Added `textual.actions.SkipAction` exception which can be raised from an action to allow parents to process bindings.

### Fixed

- Fixed watch method incorrectly running on first set when value hasnt changed and init=False https://github.com/Textualize/textual/pull/1367

## [0.7.0] - 2022-12-17

### Added
Expand All @@ -30,6 +34,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
- Fixed validator not running on first reactive set https://github.com/Textualize/textual/pull/1359
- Ensure only printable characters are used as key_display https://github.com/Textualize/textual/pull/1361


## [0.6.0] - 2022-12-11

### Added
Expand Down
7 changes: 1 addition & 6 deletions src/textual/reactive.py
Original file line number Diff line number Diff line change
Expand Up @@ -175,15 +175,11 @@ def __set__(self, obj: Reactable, value: ReactiveType) -> None:
current_value = getattr(obj, name)
# Check for validate function
validate_function = getattr(obj, f"validate_{name}", None)
# Check if this is the first time setting the value
first_set = getattr(obj, f"__first_set_{self.internal_name}", True)
# Call validate
if callable(validate_function):
value = validate_function(value)
# If the value has changed, or this is the first time setting the value
if current_value != value or first_set or self._always_update:
# Set the first set flag to False
setattr(obj, f"__first_set_{self.internal_name}", False)
if current_value != value or self._always_update:
# Store the internal value
setattr(obj, self.internal_name, value)
# Check all watchers
Expand All @@ -200,7 +196,6 @@ def _check_watchers(cls, obj: Reactable, name: str, old_value: Any):
obj (Reactable): The reactable object.
name (str): Attribute name.
old_value (Any): The old (previous) value of the attribute.
first_set (bool, optional): True if this is the first time setting the value. Defaults to False.
"""
_rich_traceback_omit = True
# Get the current value.
Expand Down
6 changes: 4 additions & 2 deletions tests/test_reactive.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,6 @@ async def watch_count(self, old_value: int, new_value: int) -> None:
assert app.watcher_new_value == OLD_VALUE # The value wasn't changed


@pytest.mark.xfail(
reason="Reactive watcher is incorrectly always called the first time it is set, even if value is same [issue#1230]")
async def test_watch_init_false_always_update_false():
class WatcherInitFalse(App):
count = reactive(0, init=False)
Expand All @@ -102,6 +100,10 @@ def watch_count(self, new_value: int) -> None:
async with app.run_test():
app.count = 0 # Value hasn't changed, and always_update=False, so watch_count shouldn't run
assert app.watcher_call_count == 0
app.count = 0
assert app.watcher_call_count == 0
app.count = 1
assert app.watcher_call_count == 1


async def test_watch_init_true():
Expand Down