From 40664e298c969ce3ec6eebd79a99334a35364af1 Mon Sep 17 00:00:00 2001 From: AntoinePrv Date: Fri, 27 Jan 2023 13:46:39 +0100 Subject: [PATCH 1/4] Add comments around the update protocol --- include/xwidgets/xcommon.hpp | 17 +++++++++++------ include/xwidgets/xtransport.hpp | 6 +++++- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/include/xwidgets/xcommon.hpp b/include/xwidgets/xcommon.hpp index 75a1d2e..e37752b 100644 --- a/include/xwidgets/xcommon.hpp +++ b/include/xwidgets/xcommon.hpp @@ -118,19 +118,24 @@ namespace xw xeus::buffer_sequence buffers; xwidgets_serialize(value, state[name], buffers); + // A current message is set, which implies that this change is triggered due to + // a change from the frontend. if (m_hold != nullptr) { const auto& hold_state = m_hold->content()["data"]["state"]; const auto& hold_buffers = m_hold->buffers(); - auto it = hold_state.find(name); - if (it != hold_state.end()) + // The change that triggered this function is the same as the change coming from + // the frontend so we need not send back an update to the frontend + auto const it = hold_state.find(name); + if ((it != hold_state.end()) && same_patch(name, *it, hold_buffers, state[name], buffers)) { - if (same_patch(name, *it, hold_buffers, state[name], buffers)) - { - return; - } + return; } + // On the contrary, the update could differ from the change in the frontend. + // For instance, if a validator adjusted the value of a property from the one + // recieved from the frontend. + // In this case, we need to send an update back to the frontend. } send_patch(std::move(state), std::move(buffers)); diff --git a/include/xwidgets/xtransport.hpp b/include/xwidgets/xtransport.hpp index 4472d3d..43e8046 100644 --- a/include/xwidgets/xtransport.hpp +++ b/include/xwidgets/xtransport.hpp @@ -193,12 +193,16 @@ namespace xw const nl::json& state = data["state"]; const auto& buffers = message.buffers(); const nl::json& buffer_paths = data["buffer_paths"]; + // Set the current message in ``xcommom::hold()`` + // As a result ``x::commom`` will know that the change is coming from the frontend this->hold() = std::addressof(message); - ; insert_buffer_paths(const_cast(state), buffer_paths); + // This will potentially change the properties of the widget. + // As a result, ``xp::observed`` will call ``xcommon::notify` /*D*/ this->derived_cast().apply_patch(state, buffers); /*D*/ + // Clear current message. this->hold() = nullptr; } else if (method == "request_state") From df03f154eb4707a72539eae59f3ad9cf8c668fec Mon Sep 17 00:00:00 2001 From: AntoinePrv Date: Fri, 27 Jan 2023 14:19:16 +0100 Subject: [PATCH 2/4] Unconditionally send echo_update on front_end change --- include/xwidgets/xcommon.hpp | 12 +++++++----- src/xcommon.cpp | 12 ++++++++++-- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/include/xwidgets/xcommon.hpp b/include/xwidgets/xcommon.hpp index e37752b..e9becdb 100644 --- a/include/xwidgets/xcommon.hpp +++ b/include/xwidgets/xcommon.hpp @@ -81,7 +81,7 @@ namespace xw template void notify(const std::string& name, const T& value) const; void send(nl::json&&, xeus::buffer_sequence&&) const; - void send_patch(nl::json&&, xeus::buffer_sequence&&) const; + void send_patch(nl::json&&, xeus::buffer_sequence&&, const char* method = "update") const; private: @@ -117,6 +117,7 @@ namespace xw nl::json state; xeus::buffer_sequence buffers; xwidgets_serialize(value, state[name], buffers); + const char* method = "update"; // A current message is set, which implies that this change is triggered due to // a change from the frontend. @@ -126,19 +127,20 @@ namespace xw const auto& hold_buffers = m_hold->buffers(); // The change that triggered this function is the same as the change coming from - // the frontend so we need not send back an update to the frontend + // the frontend so we need not send back an update but only an "echo_update" since + // protocol 2.1.0 auto const it = hold_state.find(name); if ((it != hold_state.end()) && same_patch(name, *it, hold_buffers, state[name], buffers)) { - return; + method = "echo_update"; } // On the contrary, the update could differ from the change in the frontend. // For instance, if a validator adjusted the value of a property from the one // recieved from the frontend. - // In this case, we need to send an update back to the frontend. + // In this case, we need to send a regular "update" back to the frontend. } - send_patch(std::move(state), std::move(buffers)); + send_patch(std::move(state), std::move(buffers), method); } } diff --git a/src/xcommon.cpp b/src/xcommon.cpp index 3b156cd..9a6554a 100644 --- a/src/xcommon.cpp +++ b/src/xcommon.cpp @@ -1,3 +1,11 @@ +/*************************************************************************** + * Copyright (c) 2022, QuantStack and XWidgets contributors * + * * + * Distributed under the terms of the BSD 3-Clause License. * + * * + * The full license is in the file LICENSE, distributed with this software. * + ****************************************************************************/ + #include "xwidgets/xcommon.hpp" #include @@ -140,7 +148,7 @@ namespace xw return m_buffer_paths; } - void xcommon::send_patch(nl::json&& patch, xeus::buffer_sequence&& buffers) const + void xcommon::send_patch(nl::json&& patch, xeus::buffer_sequence&& buffers, const char* method) const { // extract buffer paths auto paths = nl::json::array(); @@ -152,7 +160,7 @@ namespace xw // data nl::json data; - data["method"] = "update"; + data["method"] = method; data["state"] = std::move(patch); data["buffer_paths"] = std::move(paths); From 07775ab374438b85977f51eb84f25711585bd317 Mon Sep 17 00:00:00 2001 From: AntoinePrv Date: Fri, 27 Jan 2023 14:29:30 +0100 Subject: [PATCH 3/4] Update protocol version --- include/xwidgets/xwidgets_config.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/xwidgets/xwidgets_config.hpp b/include/xwidgets/xwidgets_config.hpp index 6efbdaa..86fbeca 100644 --- a/include/xwidgets/xwidgets_config.hpp +++ b/include/xwidgets/xwidgets_config.hpp @@ -32,7 +32,7 @@ // Protocol version #define XWIDGETS_PROTOCOL_VERSION_MAJOR 2 -#define XWIDGETS_PROTOCOL_VERSION_MINOR 0 +#define XWIDGETS_PROTOCOL_VERSION_MINOR 1 #define XWIDGETS_PROTOCOL_VERSION_PATCH 0 // Semver requirement for @jupyter-widgets/base From cc060689c62ba8b6164595c42bd048f842831454 Mon Sep 17 00:00:00 2001 From: AntoinePrv Date: Fri, 27 Jan 2023 16:04:20 +0100 Subject: [PATCH 4/4] Add JUPYTER_WIDGETS_ECHO env variable for echo_update --- include/xwidgets/xcommon.hpp | 19 ++++++++++- src/xcommon.cpp | 66 ++++++++++++++++++++++++++++++++++++ 2 files changed, 84 insertions(+), 1 deletion(-) diff --git a/include/xwidgets/xcommon.hpp b/include/xwidgets/xcommon.hpp index e9becdb..eaa6fa6 100644 --- a/include/xwidgets/xcommon.hpp +++ b/include/xwidgets/xcommon.hpp @@ -13,8 +13,10 @@ #include #include +#include +#include + #include "xbinary.hpp" -#include "xeus/xcomm.hpp" #include "xwidgets_config.hpp" namespace xw @@ -85,6 +87,13 @@ namespace xw private: + /** + * Indicate whether a global setting activates or deactivates ``echo_update`` messages. + * + * If the optional is empty, then no setting is set explicitly. + */ + static xtl::xoptional global_echo_update(); + bool same_patch(const std::string&, const nl::json&, const xeus::buffer_sequence&, const nl::json&, const xeus::buffer_sequence&) const; @@ -132,6 +141,14 @@ namespace xw auto const it = hold_state.find(name); if ((it != hold_state.end()) && same_patch(name, *it, hold_buffers, state[name], buffers)) { + // If the "echo_update" is explicitly deactivated we do not send the message + auto const echo_update = global_echo_update(); + if (echo_update.has_value() && !echo_update.value()) + { + return; + } + // If the "echo_update" is explicitly activated or unspecified, we continue and + // send message method = "echo_update"; } // On the contrary, the update could differ from the change in the frontend. diff --git a/src/xcommon.cpp b/src/xcommon.cpp index 9a6554a..66ca716 100644 --- a/src/xcommon.cpp +++ b/src/xcommon.cpp @@ -9,10 +9,13 @@ #include "xwidgets/xcommon.hpp" #include +#include #include #include #include +#include + #include "xeus/xinterpreter.hpp" #include "xtarget.hpp" @@ -194,6 +197,69 @@ namespace xw m_comm.close(nl::json::object(), nl::json::object(), xeus::buffer_sequence()); } + namespace + { + std::string tolower(std::string s) + { + auto safe_tolower = [](unsigned char c) + { + return std::tolower(c); + }; + std::transform(s.begin(), s.end(), s.begin(), safe_tolower); + return s; + } + + std::string ltrim(std::string s) + { + auto const safe_isnotspace = [](unsigned char ch) + { + return !std::isspace(ch); + }; + s.erase(s.begin(), std::find_if(s.begin(), s.end(), safe_isnotspace)); + return s; + } + + std::string rtrim(std::string s) + { + auto const safe_isnotspace = [](unsigned char ch) + { + return !std::isspace(ch); + }; + s.erase(std::find_if(s.rbegin(), s.rend(), safe_isnotspace).base(), s.end()); + return s; + } + + std::string trim(std::string s) + { + rtrim(s); + ltrim(s); + return s; + } + + bool is_true_string(const char* str) + { + const std::string s = tolower(trim(str)); + static const auto trues = {"true", "on", "yes", "1"}; + return std::find(trues.begin(), trues.end(), s) < trues.end(); + } + + xtl::xoptional get_tristate_env(const char* name) + { + const char* const val = std::getenv(name); + if (val == nullptr) + { + return {}; + } + return is_true_string(val); + } + } + + xtl::xoptional xcommon::global_echo_update() + { + static const auto out = get_tristate_env("JUPYTER_WIDGETS_ECHO"); + return out; + } + bool xcommon::same_patch(const std::string& name, const nl::json& j1, const xeus::buffer_sequence&, const nl::json& j2, const xeus::buffer_sequence&) const