From af8af1bee9da5f2a940f23fe3d94b49427c37979 Mon Sep 17 00:00:00 2001 From: darrenburns Date: Tue, 20 Dec 2022 11:17:11 +0000 Subject: [PATCH] Ensure watcher isn't called on first_set when init is False and value doesn't change (#1367) * Ensure watcher not called when value doesnt change, even on first set * Update CHANGELOG.md --- CHANGELOG.md | 5 +++++ src/textual/reactive.py | 7 +------ tests/test_reactive.py | 6 ++++-- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ca146acce..02d40d932 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,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 @@ -34,6 +38,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 diff --git a/src/textual/reactive.py b/src/textual/reactive.py index 88b10cd04..d13039cde 100644 --- a/src/textual/reactive.py +++ b/src/textual/reactive.py @@ -175,15 +175,11 @@ class Reactive(Generic[ReactiveType]): 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 @@ -200,7 +196,6 @@ class Reactive(Generic[ReactiveType]): 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. diff --git a/tests/test_reactive.py b/tests/test_reactive.py index 6ccfa0656..5824f26f4 100644 --- a/tests/test_reactive.py +++ b/tests/test_reactive.py @@ -88,8 +88,6 @@ async def test_watch_async_init_true(): 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) @@ -102,6 +100,10 @@ async def test_watch_init_false_always_update_false(): 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():