Support formulas #630
Support formulas #630
Conversation
dfalbel
changed the base branch from
main
to
bugfix/honor-max-display-entries
dfalbel
force-pushed
the
feature/support-formula
branch
from
b096d69 to
ea1a3cd
Compare
| RAWSXP | LGLSXP | INTSXP | REALSXP | STRSXP | CPLXSXP => Self::from_default(value), | ||
| _ => Self::from_error(Error::Anyhow(anyhow!( | ||
| "Unexpected type {}", | ||
| r_type2char(r_typeof(value)) | ||
| ))), |
There was a problem hiding this comment.
why this new restriction? For explicitness?
There was a problem hiding this comment.
yes, I taught that capturing an unexpected type here would be more explicit.
Before that, for example, the formula support error was being captured at:
ark/crates/harp/src/vector/formatted_vector.rs
Lines 112 to 115 in 92ce2f7
Which is fine, but, AFAICT we should only use FormattedVector for vector like objects.
crates/ark/src/variables/variable.rs
Outdated
| } | ||
|
|
||
| fn from_formula(value: SEXP) -> Self { | ||
| let display_value: Result<RObject, Error> = RFunction::new("base", "format") |
There was a problem hiding this comment.
| let display_value: Result<RObject, Error> = RFunction::new("base", "format") | |
| let display_value: harp::Result<RObject> = RFunction::new("base", "format") |
There was a problem hiding this comment.
Thanks! I simplified the error handling by returning an anyhow::Result in the form_formula method and handling it in from_language.
| #[test] | ||
| fn test_support_formula() { | ||
| r_task(|| { | ||
| let vars = inspect_from_expr("list(x = x ~ y + z + a)"); |
There was a problem hiding this comment.
Needs a test with multiline deparsing, e.g. with {.
There was a problem hiding this comment.
Fixed and added a test for this!
crates/ark/src/variables/variable.rs
Outdated
| .call_in(ARK_ENVS.positron_ns)? | ||
| .try_into()?; | ||
|
|
||
| let mut display_value = String::with_capacity(MAX_DISPLAY_VALUE_LENGTH); |
There was a problem hiding this comment.
We might want to join lines with ; (with trailing space) and add a space after { and before }. But the deparser might add lines in other cases too, so I guess we can't easily do that.
I would have just ellipsised after the first line to avoid this issue, but your approach allows seeing more of the contents. I'll leave it up to you to decide what is best.
There was a problem hiding this comment.
Ohh I missed that. I decided to just add an ellipsis after the first {: c812b8f
Build on top of #629
Addresses posit-dev/positron#4119