Update to protocol 2.1.0 (echo_update) #251
Conversation
AntoinePrv
commented
Jan 27, 2023
AntoinePrv
commented
- Add comments around the update protocol
- Unconditionally send echo_update on front_end change
- Update protocol version
|
@SylvainCorlay @martinRenou @maartenbreddels @jasongrout this is a PR to add the |
AntoinePrv
changed the title
AntoinePrv
force-pushed
the
echo_update
branch
from
af91126 to
cc06068
Compare
|
|
||
| xtl::xoptional<bool> xcommon::global_echo_update() | ||
| { | ||
| static const auto out = get_tristate_env("JUPYTER_WIDGETS_ECHO"); |
There was a problem hiding this comment.
Why do we need to check "on", "true", "yes", "1"?
There was a problem hiding this comment.
I think that was only for ipywidgets 7, where we had to opt in for backwards compatibility . I think in ipywidgets 8 there is no op in or out anymore. But not 100% sure.
There was a problem hiding this comment.
I was wrong, it's still in, and the bar for accepting values was set quite high: https://github.com/jupyter-widgets/ipywidgets/blob/main/python/ipywidgets/ipywidgets/widgets/widget.py#L29 :)
| bool is_true_string(const char* str) | ||
| { | ||
| const std::string s = tolower(trim(str)); | ||
| static const auto trues = {"true", "on", "yes", "1"}; |
There was a problem hiding this comment.
I'm curious, why aren't we mirroring the logic in Jupyter Widgets and Jupyter Core of testing for the absence of negative values instead of the presence of positive values?
There was a problem hiding this comment.
Indeed there is no reason. I did not have all that context at the time of writing, we can change that.
