Open
Conversation
DavisVaughan
commented
Feb 26, 2026
| pub enum LastValue { | ||
| /// Always show the .Last.value variable in the Variables pane. This is used | ||
| /// by tests to show the value without changing the global option. | ||
| Always, |
Contributor
Author
There was a problem hiding this comment.
Only used by tests before, but we don't use this now. We literally set the option like an R user would.
Comment on lines
220
to
222
| let mut variables: Vec<Variable> = vec![]; | ||
| r_task(|| { | ||
| if let Some(last_value) = self.last_value() { | ||
| variables.push(last_value.var()); | ||
| } | ||
|
|
||
| for binding in self.current_bindings.get() { |
Contributor
Author
There was a problem hiding this comment.
I thought it was very strange that we had all this special handling around .Last.value.
I realized it could be greatly simplified by shoving .Last.value handling into bindings().
Either:
- The option is on, and
.Last.valueis returned frombindings() - The option is off, and
.Last.valueis not returned frombindings()
Everything else (including assigned and removed handling below) just correctly falls out from this.
Comment on lines
+635
to
+639
| if self.show_last_value() { | ||
| if let Some(last_value) = Self::last_value().log_err() { | ||
| bindings.push(last_value); | ||
| } | ||
| } |
Contributor
Author
There was a problem hiding this comment.
This is the one special place we handle it now
DavisVaughan
commented
Feb 26, 2026
This magically works because the ark_test side has the `self.recv_iopub_busy()` and `self.recv_iopub_idle()` overrides that buffer Variables messages
So they all get proper buffering
It runs R code, so is very R specific
DavisVaughan
force-pushed
the
feature/simpler-last-value
branch
from
a6f392c to
41538eb
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
While reviewing some of @lionel-'s tweaks to
variables.rs, it occurred to me that the.Last.valuehandling is quite complicated and could be heavily simplifiedI think the existing tests were also actually wrong. If the
.Last.valueis being shown and was set toNULLand after executing another R statement it is stillNULL, then we should not get an update for that. The pointer value didn't change, so nothing should update.The only way to actually test ^ is to really send R code through
r_read_console()so I've switched to using frontend powered tests for.Last.valuetesting.