From 582dcc6c1f12ab56f649678bc8d818a081899c18 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20Fran=C3=A7ois?= Date: Fri, 31 Mar 2023 10:06:44 +0200 Subject: [PATCH 01/21] LogicalVector::Type = bool --- .../amalthea/crates/harp/src/vector/logical_vector.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/extensions/positron-r/amalthea/crates/harp/src/vector/logical_vector.rs b/extensions/positron-r/amalthea/crates/harp/src/vector/logical_vector.rs index c2f045477ea..6a4a0e01c70 100644 --- a/extensions/positron-r/amalthea/crates/harp/src/vector/logical_vector.rs +++ b/extensions/positron-r/amalthea/crates/harp/src/vector/logical_vector.rs @@ -16,11 +16,11 @@ pub struct LogicalVector { } impl Vector for LogicalVector { - type Item = i32; - type Type = i32; + type Item = bool; + type Type = bool; const SEXPTYPE: u32 = LGLSXP; type UnderlyingType = i32; - type CompareType = i32; + type CompareType = bool; unsafe fn new_unchecked(object: impl Into) -> Self { Self { object: RObject::new(object.into()) } @@ -57,11 +57,11 @@ impl Vector for LogicalVector { } fn convert_value(x: &Self::UnderlyingType) -> Self::Type { - *x + *x == 1 } fn format_one(&self, x: Self::Type) -> String { - if x == 1 { + if x { String::from("TRUE") } else { String::from("FALSE") From 736c7e26cbbc61405264a5f890f4774ce7c5911d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20Fran=C3=A7ois?= Date: Fri, 31 Mar 2023 10:29:17 +0200 Subject: [PATCH 02/21] is_vector() --- .../amalthea/crates/harp/src/environment.rs | 58 ++++++++++--------- 1 file changed, 30 insertions(+), 28 deletions(-) diff --git a/extensions/positron-r/amalthea/crates/harp/src/environment.rs b/extensions/positron-r/amalthea/crates/harp/src/environment.rs index ece7d18275f..bdbbac9419e 100644 --- a/extensions/positron-r/amalthea/crates/harp/src/environment.rs +++ b/extensions/positron-r/amalthea/crates/harp/src/environment.rs @@ -138,40 +138,42 @@ impl Binding { } -fn regular_binding_display_value(value: SEXP) -> BindingValue { - // TODO: some form of R based dispatch - +fn is_vector(value: SEXP) -> bool { match r_typeof(value) { - // standard vector - LGLSXP => vec_glimpse(value), - INTSXP => vec_glimpse(value), - REALSXP => vec_glimpse(value), - CPLXSXP => vec_glimpse(value), - STRSXP => vec_glimpse(value), - VECSXP => { - // TODO: data.frame - vec_glimpse(value) - }, + LGLSXP => true, + INTSXP => true, + REALSXP => true, + CPLXSXP => true, + STRSXP => true, + RAWSXP => true, + + _ => false + } +} - _ => BindingValue::new(String::from("???"), false) +fn regular_binding_display_value(value: SEXP) -> BindingValue { + // TODO: some form of R based dispatch + if is_vector(value) { + vec_glimpse(value) + } else { + + // TODO: + // - list (or does that go in vector too) + // - function + // - environment + // - pairlist + // - calls + + BindingValue::new(String::from("???"), false) } } fn regular_binding_display_type(value: SEXP) -> String { - match r_typeof(value) { - // standard vector - LGLSXP => vec_type_info(value), - INTSXP => vec_type_info(value), - REALSXP => vec_type_info(value), - CPLXSXP => vec_type_info(value), - STRSXP => vec_type_info(value), - VECSXP => { - // TODO: data.frame - vec_type_info(value) - }, - - _ => String::from("???") + if is_vector(value) { + vec_type_info(value) + } else { + String::from("???") } } @@ -187,9 +189,9 @@ fn vec_type(value: SEXP) -> String { REALSXP => "dbl", LGLSXP => "lgl", STRSXP => "str", - VECSXP => "list", RAWSXP => "raw", CPLXSXP => "cplx", + VECSXP => "list", // TODO: this should not happen _ => "???" From 19cc7dc9f558faa5c9c160f258da841620a165d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20Fran=C3=A7ois?= Date: Fri, 31 Mar 2023 10:40:10 +0200 Subject: [PATCH 03/21] destructre BindingValue --- .../amalthea/crates/ark/src/environment/variable.rs | 7 ++++--- .../positron-r/amalthea/crates/harp/src/environment.rs | 7 ++++--- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/extensions/positron-r/amalthea/crates/ark/src/environment/variable.rs b/extensions/positron-r/amalthea/crates/ark/src/environment/variable.rs index 478b9ad1b83..a789e59ee33 100644 --- a/extensions/positron-r/amalthea/crates/ark/src/environment/variable.rs +++ b/extensions/positron-r/amalthea/crates/ark/src/environment/variable.rs @@ -6,6 +6,7 @@ // use harp::environment::Binding; +use harp::environment::BindingValue; use serde::Deserialize; use serde::Serialize; @@ -82,21 +83,21 @@ impl EnvironmentVariable { pub fn new(binding: &Binding) -> Self { let display_name = binding.name.to_string(); - let value = binding.display_value(); + let BindingValue{display_value, is_truncated} = binding.display_value(); let display_type = binding.display_type(); let kind = ValueKind::String; Self { display_name, - display_value: value.value, + display_value, display_type, type_info: String::new(), kind, length: 0, size: 0, has_children: false, - is_truncated: value.is_truncated, + is_truncated, } } } diff --git a/extensions/positron-r/amalthea/crates/harp/src/environment.rs b/extensions/positron-r/amalthea/crates/harp/src/environment.rs index bdbbac9419e..795aec6bc58 100644 --- a/extensions/positron-r/amalthea/crates/harp/src/environment.rs +++ b/extensions/positron-r/amalthea/crates/harp/src/environment.rs @@ -77,14 +77,14 @@ impl PartialOrd for Binding { } pub struct BindingValue { - pub value: String, + pub display_value: String, pub is_truncated: bool } impl BindingValue { - pub fn new(value: String, is_truncated: bool) -> Self { + pub fn new(display_value: String, is_truncated: bool) -> Self { BindingValue { - value, + display_value, is_truncated } } @@ -159,6 +159,7 @@ fn regular_binding_display_value(value: SEXP) -> BindingValue { // TODO: // - list (or does that go in vector too) + // - data.frame // - function // - environment // - pairlist From fb597cef884e644ef705f805ebd5914743a61883 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20Fran=C3=A7ois?= Date: Fri, 31 Mar 2023 10:51:57 +0200 Subject: [PATCH 04/21] destructure BindingType --- .../crates/ark/src/environment/variable.rs | 7 ++-- .../amalthea/crates/harp/src/environment.rs | 42 ++++++++++++++----- 2 files changed, 35 insertions(+), 14 deletions(-) diff --git a/extensions/positron-r/amalthea/crates/ark/src/environment/variable.rs b/extensions/positron-r/amalthea/crates/ark/src/environment/variable.rs index a789e59ee33..b595005edc6 100644 --- a/extensions/positron-r/amalthea/crates/ark/src/environment/variable.rs +++ b/extensions/positron-r/amalthea/crates/ark/src/environment/variable.rs @@ -6,6 +6,7 @@ // use harp::environment::Binding; +use harp::environment::BindingType; use harp::environment::BindingValue; use serde::Deserialize; use serde::Serialize; @@ -83,8 +84,8 @@ impl EnvironmentVariable { pub fn new(binding: &Binding) -> Self { let display_name = binding.name.to_string(); - let BindingValue{display_value, is_truncated} = binding.display_value(); - let display_type = binding.display_type(); + let BindingValue{display_value, is_truncated} = binding.get_value(); + let BindingType{display_type, type_info} = binding.get_type(); let kind = ValueKind::String; @@ -92,7 +93,7 @@ impl EnvironmentVariable { display_name, display_value, display_type, - type_info: String::new(), + type_info, kind, length: 0, size: 0, diff --git a/extensions/positron-r/amalthea/crates/harp/src/environment.rs b/extensions/positron-r/amalthea/crates/harp/src/environment.rs index 795aec6bc58..0e9b536ad2f 100644 --- a/extensions/positron-r/amalthea/crates/harp/src/environment.rs +++ b/extensions/positron-r/amalthea/crates/harp/src/environment.rs @@ -94,6 +94,20 @@ impl BindingValue { } } +pub struct BindingType { + pub display_type: String, + pub type_info: String +} + +impl BindingType { + pub fn new(display_type: String, type_info: String) -> Self { + BindingType { + display_type, + type_info + } + } +} + impl Binding { pub fn new(frame: SEXP) -> Self { @@ -116,7 +130,7 @@ impl Binding { } } - pub fn display_value(&self) -> BindingValue { + pub fn get_value(&self) -> BindingValue { match self.kind { BindingKind::Regular => regular_binding_display_value(self.value), BindingKind::Promise(true) => regular_binding_display_value(unsafe{PRVALUE(self.value)}), @@ -126,13 +140,13 @@ impl Binding { } } - pub fn display_type(&self) -> String { + pub fn get_type(&self) -> BindingType { match self.kind { - BindingKind::Active => String::from("active binding"), - BindingKind::Promise(false) => String::from("promise"), + BindingKind::Active => BindingType::new(String::from("active binding"), String::from("")), + BindingKind::Promise(false) => BindingType::new(String::from("promise"), String::from("")), - BindingKind::Regular => regular_binding_display_type(self.value), - BindingKind::Promise(true) => regular_binding_display_type(unsafe{PRVALUE(self.value)}) + BindingKind::Regular => regular_binding_type(self.value), + BindingKind::Promise(true) => regular_binding_type(unsafe{PRVALUE(self.value)}) } } @@ -170,11 +184,11 @@ fn regular_binding_display_value(value: SEXP) -> BindingValue { } -fn regular_binding_display_type(value: SEXP) -> String { +fn regular_binding_type(value: SEXP) -> BindingType { if is_vector(value) { vec_type_info(value) } else { - String::from("???") + BindingType::new(String::from("???"), String::from("")) } } @@ -200,8 +214,15 @@ fn vec_type(value: SEXP) -> String { String::from(rtype) } -fn vec_type_info(value: SEXP) -> String { - format!("{} [{}]", vec_type(value), vec_shape(value)) +fn vec_type_info(value: SEXP) -> BindingType { + let display_type = format!("{} [{}]", vec_type(value), vec_shape(value)); + + let mut type_info = display_type.clone(); + if unsafe{ ALTREP(value) == 1} { + type_info.push_str(altrep_class(value).as_str()) + } + + BindingType::new(display_type, type_info) } fn vec_shape(value: SEXP) -> String { @@ -259,7 +280,6 @@ fn vec_glimpse(value: SEXP) -> BindingValue { } } -#[allow(dead_code)] fn altrep_class(object: SEXP) -> String { let serialized_klass = unsafe{ ATTRIB(ALTREP_CLASS(object)) From cb94f2ff81edde61bab7739e87e575b366a4a4fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20Fran=C3=A7ois?= Date: Fri, 31 Mar 2023 10:59:00 +0200 Subject: [PATCH 05/21] Binding::has_children --- .../amalthea/crates/ark/src/environment/variable.rs | 3 ++- .../amalthea/crates/harp/src/environment.rs | 13 +++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/extensions/positron-r/amalthea/crates/ark/src/environment/variable.rs b/extensions/positron-r/amalthea/crates/ark/src/environment/variable.rs index b595005edc6..aec0f14bed5 100644 --- a/extensions/positron-r/amalthea/crates/ark/src/environment/variable.rs +++ b/extensions/positron-r/amalthea/crates/ark/src/environment/variable.rs @@ -88,6 +88,7 @@ impl EnvironmentVariable { let BindingType{display_type, type_info} = binding.get_type(); let kind = ValueKind::String; + let has_children = binding.has_children(); Self { display_name, @@ -97,7 +98,7 @@ impl EnvironmentVariable { kind, length: 0, size: 0, - has_children: false, + has_children, is_truncated, } } diff --git a/extensions/positron-r/amalthea/crates/harp/src/environment.rs b/extensions/positron-r/amalthea/crates/harp/src/environment.rs index 0e9b536ad2f..93b08000bd2 100644 --- a/extensions/positron-r/amalthea/crates/harp/src/environment.rs +++ b/extensions/positron-r/amalthea/crates/harp/src/environment.rs @@ -150,6 +150,19 @@ impl Binding { } } + pub fn has_children(&self) -> bool { + match self.kind { + // TODO: for now only lists have children + BindingKind::Regular => r_typeof(self.value) == VECSXP, + BindingKind::Promise(true) => r_typeof(unsafe{PRVALUE(self.value)}) == VECSXP, + + // TODO: + // - BindingKind::Promise(false) could have code and env as their children + // - BindingKind::Active could have their function + _ => false + } + } + } fn is_vector(value: SEXP) -> bool { From 867bc258d2165c522d2e2bb06b9211743486374a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20Fran=C3=A7ois?= Date: Fri, 31 Mar 2023 12:05:34 +0200 Subject: [PATCH 06/21] basic wiring for Inspect and DEtails messages --- .../crates/ark/src/environment/message.rs | 24 +++++++++++++ .../ark/src/environment/r_environment.rs | 20 +++++++++++ .../crates/ark/src/environment/variable.rs | 36 +++++++++++++++++++ 3 files changed, 80 insertions(+) diff --git a/extensions/positron-r/amalthea/crates/ark/src/environment/message.rs b/extensions/positron-r/amalthea/crates/ark/src/environment/message.rs index 251b758417b..be4449d52de 100644 --- a/extensions/positron-r/amalthea/crates/ark/src/environment/message.rs +++ b/extensions/positron-r/amalthea/crates/ark/src/environment/message.rs @@ -43,6 +43,12 @@ pub enum EnvironmentMessage { /// A message containing an error message. Error(EnvironmentMessageError), + + /// A message requesting to inspect a variable + Inspect(EnvironmentMessageInspect), + + /// Details about a variable, response to an Inspect message + Details(EnvironmentMessageDetails) } /** @@ -89,3 +95,21 @@ pub struct EnvironmentMessageClear { pub struct EnvironmentMessageDelete { pub variables: Vec, } + +/** + * The data for the Inspect message + */ +#[derive(Debug, Serialize, Deserialize)] +pub struct EnvironmentMessageInspect { + pub path: Vec, +} + +/** + * The data for the Details message + */ +#[derive(Debug, Serialize, Deserialize)] +pub struct EnvironmentMessageDetails { + pub path: Vec, + pub children: Vec, + pub length: usize, +} diff --git a/extensions/positron-r/amalthea/crates/ark/src/environment/r_environment.rs b/extensions/positron-r/amalthea/crates/ark/src/environment/r_environment.rs index eab6a1c8ada..f7ea383912e 100644 --- a/extensions/positron-r/amalthea/crates/ark/src/environment/r_environment.rs +++ b/extensions/positron-r/amalthea/crates/ark/src/environment/r_environment.rs @@ -27,6 +27,8 @@ use log::warn; use crate::environment::message::EnvironmentMessage; use crate::environment::message::EnvironmentMessageClear; use crate::environment::message::EnvironmentMessageDelete; +use crate::environment::message::EnvironmentMessageDetails; +use crate::environment::message::EnvironmentMessageInspect; use crate::environment::message::EnvironmentMessageList; use crate::environment::message::EnvironmentMessageUpdate; use crate::environment::variable::EnvironmentVariable; @@ -159,6 +161,10 @@ impl REnvironment { self.delete(variables, Some(id)); }, + EnvironmentMessage::Inspect(EnvironmentMessageInspect{path}) => { + self.inspect(path, Some(id)); + }, + _ => { error!( "Environment: Don't know how to handle message type '{:?}'", @@ -275,6 +281,20 @@ impl REnvironment { self.update(request_id); } + fn inspect(&mut self, path: Vec, request_id: Option) { + let children = r_lock!{ + EnvironmentVariable::inspect(RObject::new(*self.env), path.clone()) + }; + let length = children.len(); + let msg = EnvironmentMessage::Details(EnvironmentMessageDetails { + path, + children, + length + }); + + self.send_message(msg, request_id); + } + fn send_message(&mut self, message: EnvironmentMessage, request_id: Option) { let data = serde_json::to_value(message); diff --git a/extensions/positron-r/amalthea/crates/ark/src/environment/variable.rs b/extensions/positron-r/amalthea/crates/ark/src/environment/variable.rs index aec0f14bed5..46328ceded1 100644 --- a/extensions/positron-r/amalthea/crates/ark/src/environment/variable.rs +++ b/extensions/positron-r/amalthea/crates/ark/src/environment/variable.rs @@ -8,6 +8,14 @@ use harp::environment::Binding; use harp::environment::BindingType; use harp::environment::BindingValue; +use harp::object::RObject; +use harp::r_symbol; +use harp::vector::CharacterVector; +use harp::vector::Vector; +use libR_sys::R_NamesSymbol; +use libR_sys::Rf_findVarInFrame; +use libR_sys::Rf_getAttrib; +use libR_sys::XLENGTH; use serde::Deserialize; use serde::Serialize; @@ -102,4 +110,32 @@ impl EnvironmentVariable { is_truncated, } } + + pub fn inspect(env: RObject, path: Vec) -> Vec { + // for now path is only one string, and the object is a named list + let name = unsafe{ path.get_unchecked(0) }; + let list = unsafe{ Rf_findVarInFrame(*env, r_symbol!(name))}; + + let mut out : Vec = vec![]; + let n = unsafe { XLENGTH(list) }; + + let names = unsafe { CharacterVector::new_unchecked(Rf_getAttrib(list, R_NamesSymbol)) }; + for i in 0..n { + let display_name = names.get_unchecked(i).unwrap(); + out.push(Self { + display_name, + display_value: String::from("..."), + display_type: String::from("..."), + type_info: String::from("..."), + kind: ValueKind::Other, + length: 0, + size: 0, + has_children: false, + is_truncated: false + }) + } + + out + } + } From b02df516f8eda674485ffbbb00580b36dfa1c7c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20Fran=C3=A7ois?= Date: Fri, 31 Mar 2023 12:40:20 +0200 Subject: [PATCH 07/21] get shape of data frame --- .../amalthea/crates/harp/src/environment.rs | 24 +++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/extensions/positron-r/amalthea/crates/harp/src/environment.rs b/extensions/positron-r/amalthea/crates/harp/src/environment.rs index 93b08000bd2..cce95f368f3 100644 --- a/extensions/positron-r/amalthea/crates/harp/src/environment.rs +++ b/extensions/positron-r/amalthea/crates/harp/src/environment.rs @@ -10,6 +10,8 @@ use std::cmp::Ordering; use c2rust_bitfields::BitfieldStruct; use libR_sys::*; +use crate::exec::RFunction; +use crate::exec::RFunctionExt; use crate::object::RObject; use crate::symbol::RSymbol; use crate::utils::r_inherits; @@ -173,6 +175,7 @@ fn is_vector(value: SEXP) -> bool { CPLXSXP => true, STRSXP => true, RAWSXP => true, + VECSXP => true, _ => false } @@ -219,7 +222,13 @@ fn vec_type(value: SEXP) -> String { STRSXP => "str", RAWSXP => "raw", CPLXSXP => "cplx", - VECSXP => "list", + VECSXP => { + if unsafe { r_inherits(value, "data.frame") } { + "data.frame" + } else { + "list" + } + }, // TODO: this should not happen _ => "???" @@ -240,7 +249,14 @@ fn vec_type_info(value: SEXP) -> BindingType { fn vec_shape(value: SEXP) -> String { unsafe { - let dim = RObject::new(Rf_getAttrib(value, R_DimSymbol)); + let dim = if r_inherits(value, "data.frame") { + RFunction::new("base", "dim.data.frame") + .add(value) + .call() + .unwrap() + } else { + RObject::new(Rf_getAttrib(value, R_DimSymbol)) + }; if *dim == R_NilValue { format!("{}", Rf_xlength(value)) @@ -287,6 +303,10 @@ fn vec_glimpse(value: SEXP) -> BindingValue { BindingValue::new(formatted.1, formatted.0) }, + VECSXP => { + BindingValue::empty() + }, + _ => { BindingValue::empty() } From 3a6d76be6cdde56ba4dcff96f32f494d8589a55a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20Fran=C3=A7ois?= Date: Fri, 31 Mar 2023 12:54:09 +0200 Subject: [PATCH 08/21] initial inspect --- .../amalthea/crates/ark/src/environment/variable.rs | 12 ++++++++---- .../amalthea/crates/harp/src/environment.rs | 8 ++++++++ 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/extensions/positron-r/amalthea/crates/ark/src/environment/variable.rs b/extensions/positron-r/amalthea/crates/ark/src/environment/variable.rs index 46328ceded1..d4e71ad3a20 100644 --- a/extensions/positron-r/amalthea/crates/ark/src/environment/variable.rs +++ b/extensions/positron-r/amalthea/crates/ark/src/environment/variable.rs @@ -15,6 +15,7 @@ use harp::vector::Vector; use libR_sys::R_NamesSymbol; use libR_sys::Rf_findVarInFrame; use libR_sys::Rf_getAttrib; +use libR_sys::VECTOR_ELT; use libR_sys::XLENGTH; use serde::Deserialize; use serde::Serialize; @@ -121,17 +122,20 @@ impl EnvironmentVariable { let names = unsafe { CharacterVector::new_unchecked(Rf_getAttrib(list, R_NamesSymbol)) }; for i in 0..n { + let x = RObject::view(unsafe{ VECTOR_ELT(list, i)}); let display_name = names.get_unchecked(i).unwrap(); + let BindingValue{display_value, is_truncated} = BindingValue::from(*x); + let BindingType{display_type, type_info} = BindingType::from(*x); out.push(Self { display_name, - display_value: String::from("..."), - display_type: String::from("..."), - type_info: String::from("..."), + display_value, + display_type, + type_info, kind: ValueKind::Other, length: 0, size: 0, has_children: false, - is_truncated: false + is_truncated }) } diff --git a/extensions/positron-r/amalthea/crates/harp/src/environment.rs b/extensions/positron-r/amalthea/crates/harp/src/environment.rs index cce95f368f3..e3f55d6e213 100644 --- a/extensions/positron-r/amalthea/crates/harp/src/environment.rs +++ b/extensions/positron-r/amalthea/crates/harp/src/environment.rs @@ -94,6 +94,10 @@ impl BindingValue { pub fn empty() -> Self { Self::new(String::from(""), false) } + + pub fn from(x: SEXP) -> Self { + regular_binding_display_value(x) + } } pub struct BindingType { @@ -108,6 +112,10 @@ impl BindingType { type_info } } + + pub fn from(value: SEXP) -> Self { + regular_binding_type(value) + } } impl Binding { From cb9e69b3fd9edf7ea41f7d7c76a694dc4cb5797e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20Fran=C3=A7ois?= Date: Fri, 31 Mar 2023 14:54:28 +0200 Subject: [PATCH 09/21] allow fully or partially unnamed lists --- .../crates/ark/src/environment/variable.rs | 11 +++++++---- .../ark/src/lsp/modules/public/environment.R | 16 ++++++++++++++++ 2 files changed, 23 insertions(+), 4 deletions(-) create mode 100644 extensions/positron-r/amalthea/crates/ark/src/lsp/modules/public/environment.R diff --git a/extensions/positron-r/amalthea/crates/ark/src/environment/variable.rs b/extensions/positron-r/amalthea/crates/ark/src/environment/variable.rs index d4e71ad3a20..bd5d0dbef90 100644 --- a/extensions/positron-r/amalthea/crates/ark/src/environment/variable.rs +++ b/extensions/positron-r/amalthea/crates/ark/src/environment/variable.rs @@ -8,13 +8,13 @@ use harp::environment::Binding; use harp::environment::BindingType; use harp::environment::BindingValue; +use harp::exec::RFunction; +use harp::exec::RFunctionExt; use harp::object::RObject; use harp::r_symbol; use harp::vector::CharacterVector; use harp::vector::Vector; -use libR_sys::R_NamesSymbol; use libR_sys::Rf_findVarInFrame; -use libR_sys::Rf_getAttrib; use libR_sys::VECTOR_ELT; use libR_sys::XLENGTH; use serde::Deserialize; @@ -113,14 +113,17 @@ impl EnvironmentVariable { } pub fn inspect(env: RObject, path: Vec) -> Vec { - // for now path is only one string, and the object is a named list + // for now path is only one string, and the object is a list let name = unsafe{ path.get_unchecked(0) }; let list = unsafe{ Rf_findVarInFrame(*env, r_symbol!(name))}; let mut out : Vec = vec![]; let n = unsafe { XLENGTH(list) }; - let names = unsafe { CharacterVector::new_unchecked(Rf_getAttrib(list, R_NamesSymbol)) }; + let names = unsafe{ + CharacterVector::new_unchecked(RFunction::from(".ps.list_display_names").add(list).call().unwrap()) + }; + for i in 0..n { let x = RObject::view(unsafe{ VECTOR_ELT(list, i)}); let display_name = names.get_unchecked(i).unwrap(); diff --git a/extensions/positron-r/amalthea/crates/ark/src/lsp/modules/public/environment.R b/extensions/positron-r/amalthea/crates/ark/src/lsp/modules/public/environment.R new file mode 100644 index 00000000000..48ec93ed600 --- /dev/null +++ b/extensions/positron-r/amalthea/crates/ark/src/lsp/modules/public/environment.R @@ -0,0 +1,16 @@ +# +# environment.R +# +# Copyright (C) 2023 Posit Software, PBC. All rights reserved. +# +# +.ps.list_display_names <- function(x) { + names <- names(x) + if (is.null(names)) { + paste0("[[", seq_along(x), "]]") + } else { + empty <- which(names == "") + names[empty] <- paste0("[[", empty, "]]") + } + names +} From f857d17647a5c04362abecaac1bc3a0e62e1c044 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20Fran=C3=A7ois?= Date: Fri, 31 Mar 2023 15:33:05 +0200 Subject: [PATCH 10/21] recurse into lists --- .../crates/ark/src/environment/variable.rs | 25 +++++++++++++------ .../ark/src/lsp/modules/public/environment.R | 17 +++++++++++++ 2 files changed, 34 insertions(+), 8 deletions(-) diff --git a/extensions/positron-r/amalthea/crates/ark/src/environment/variable.rs b/extensions/positron-r/amalthea/crates/ark/src/environment/variable.rs index bd5d0dbef90..ded84e4177a 100644 --- a/extensions/positron-r/amalthea/crates/ark/src/environment/variable.rs +++ b/extensions/positron-r/amalthea/crates/ark/src/environment/variable.rs @@ -12,9 +12,11 @@ use harp::exec::RFunction; use harp::exec::RFunctionExt; use harp::object::RObject; use harp::r_symbol; +use harp::utils::r_typeof; use harp::vector::CharacterVector; use harp::vector::Vector; use libR_sys::Rf_findVarInFrame; +use libR_sys::VECSXP; use libR_sys::VECTOR_ELT; use libR_sys::XLENGTH; use serde::Deserialize; @@ -113,22 +115,29 @@ impl EnvironmentVariable { } pub fn inspect(env: RObject, path: Vec) -> Vec { - // for now path is only one string, and the object is a list - let name = unsafe{ path.get_unchecked(0) }; - let list = unsafe{ Rf_findVarInFrame(*env, r_symbol!(name))}; + // for now only lists can be expanded + let list = unsafe { + RFunction::from(".ps.environment_extract") + .param("env", env) + .param("path", CharacterVector::create(path).cast()) + .call() + .unwrap() + }; let mut out : Vec = vec![]; - let n = unsafe { XLENGTH(list) }; + let n = unsafe { XLENGTH(*list) }; let names = unsafe{ - CharacterVector::new_unchecked(RFunction::from(".ps.list_display_names").add(list).call().unwrap()) + CharacterVector::new_unchecked(RFunction::from(".ps.list_display_names").add(*list).call().unwrap()) }; for i in 0..n { - let x = RObject::view(unsafe{ VECTOR_ELT(list, i)}); + let x = RObject::view(unsafe{ VECTOR_ELT(*list, i)}); let display_name = names.get_unchecked(i).unwrap(); let BindingValue{display_value, is_truncated} = BindingValue::from(*x); let BindingType{display_type, type_info} = BindingType::from(*x); + let has_children = r_typeof(*x) == VECSXP; + out.push(Self { display_name, display_value, @@ -137,9 +146,9 @@ impl EnvironmentVariable { kind: ValueKind::Other, length: 0, size: 0, - has_children: false, + has_children, is_truncated - }) + }); } out diff --git a/extensions/positron-r/amalthea/crates/ark/src/lsp/modules/public/environment.R b/extensions/positron-r/amalthea/crates/ark/src/lsp/modules/public/environment.R index 48ec93ed600..4583fa8f8f0 100644 --- a/extensions/positron-r/amalthea/crates/ark/src/lsp/modules/public/environment.R +++ b/extensions/positron-r/amalthea/crates/ark/src/lsp/modules/public/environment.R @@ -14,3 +14,20 @@ } names } + +.ps.environment_extract <- function(env, path) { + object <- env + + rx_unnamed <- "^[[][[]([[:digit:]])[]][]]" + + for (p in path) { + if (grepl(rx_unnamed, p)) { + index <- as.integer(sub(rx_unnamed, "\\1", p)) + object <- object[[index]] + } else { + object <- object[[p]] + } + } + + object +} From 394760ad4b813d9d824e5186f870ca4d52490053 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20Fran=C3=A7ois?= Date: Fri, 31 Mar 2023 15:45:38 +0200 Subject: [PATCH 11/21] unused imports --- .../positron-r/amalthea/crates/ark/src/environment/variable.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/extensions/positron-r/amalthea/crates/ark/src/environment/variable.rs b/extensions/positron-r/amalthea/crates/ark/src/environment/variable.rs index ded84e4177a..6e4936330db 100644 --- a/extensions/positron-r/amalthea/crates/ark/src/environment/variable.rs +++ b/extensions/positron-r/amalthea/crates/ark/src/environment/variable.rs @@ -11,11 +11,9 @@ use harp::environment::BindingValue; use harp::exec::RFunction; use harp::exec::RFunctionExt; use harp::object::RObject; -use harp::r_symbol; use harp::utils::r_typeof; use harp::vector::CharacterVector; use harp::vector::Vector; -use libR_sys::Rf_findVarInFrame; use libR_sys::VECSXP; use libR_sys::VECTOR_ELT; use libR_sys::XLENGTH; From 8eb862ae4cadbe6b387ddc36cf560f0ca8b39469 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20Fran=C3=A7ois?= Date: Mon, 3 Apr 2023 13:49:34 +0200 Subject: [PATCH 12/21] .ps.environment.listDisplayNames() and .ps.environment.extractFromPath() --- .../amalthea/crates/ark/src/environment/variable.rs | 4 ++-- .../amalthea/crates/ark/src/lsp/modules/public/environment.R | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/extensions/positron-r/amalthea/crates/ark/src/environment/variable.rs b/extensions/positron-r/amalthea/crates/ark/src/environment/variable.rs index 6e4936330db..36080af4878 100644 --- a/extensions/positron-r/amalthea/crates/ark/src/environment/variable.rs +++ b/extensions/positron-r/amalthea/crates/ark/src/environment/variable.rs @@ -115,7 +115,7 @@ impl EnvironmentVariable { pub fn inspect(env: RObject, path: Vec) -> Vec { // for now only lists can be expanded let list = unsafe { - RFunction::from(".ps.environment_extract") + RFunction::from(".ps.environment.resolveObjectFromPath") .param("env", env) .param("path", CharacterVector::create(path).cast()) .call() @@ -126,7 +126,7 @@ impl EnvironmentVariable { let n = unsafe { XLENGTH(*list) }; let names = unsafe{ - CharacterVector::new_unchecked(RFunction::from(".ps.list_display_names").add(*list).call().unwrap()) + CharacterVector::new_unchecked(RFunction::from(".ps.environment.listDisplayNames").add(*list).call().unwrap()) }; for i in 0..n { diff --git a/extensions/positron-r/amalthea/crates/ark/src/lsp/modules/public/environment.R b/extensions/positron-r/amalthea/crates/ark/src/lsp/modules/public/environment.R index 4583fa8f8f0..d03ce96939b 100644 --- a/extensions/positron-r/amalthea/crates/ark/src/lsp/modules/public/environment.R +++ b/extensions/positron-r/amalthea/crates/ark/src/lsp/modules/public/environment.R @@ -4,7 +4,7 @@ # Copyright (C) 2023 Posit Software, PBC. All rights reserved. # # -.ps.list_display_names <- function(x) { +.ps.environment.listDisplayNames <- function(x) { names <- names(x) if (is.null(names)) { paste0("[[", seq_along(x), "]]") @@ -15,7 +15,7 @@ names } -.ps.environment_extract <- function(env, path) { +.ps.environment.resolveObjectFromPath <- function(env, path) { object <- env rx_unnamed <- "^[[][[]([[:digit:]])[]][]]" From 384bf69ef9bbf822868e5aade4648f7314c05018 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20Fran=C3=A7ois?= Date: Mon, 3 Apr 2023 13:58:19 +0200 Subject: [PATCH 13/21] EnvironmentVariable::inspect() returning a Result --- .../ark/src/environment/r_environment.rs | 25 +++++++++++++------ .../crates/ark/src/environment/variable.rs | 11 ++++---- 2 files changed, 23 insertions(+), 13 deletions(-) diff --git a/extensions/positron-r/amalthea/crates/ark/src/environment/r_environment.rs b/extensions/positron-r/amalthea/crates/ark/src/environment/r_environment.rs index f7ea383912e..02f8ce16c91 100644 --- a/extensions/positron-r/amalthea/crates/ark/src/environment/r_environment.rs +++ b/extensions/positron-r/amalthea/crates/ark/src/environment/r_environment.rs @@ -28,6 +28,7 @@ use crate::environment::message::EnvironmentMessage; use crate::environment::message::EnvironmentMessageClear; use crate::environment::message::EnvironmentMessageDelete; use crate::environment::message::EnvironmentMessageDetails; +use crate::environment::message::EnvironmentMessageError; use crate::environment::message::EnvironmentMessageInspect; use crate::environment::message::EnvironmentMessageList; use crate::environment::message::EnvironmentMessageUpdate; @@ -282,17 +283,27 @@ impl REnvironment { } fn inspect(&mut self, path: Vec, request_id: Option) { - let children = r_lock!{ + let inspect = r_lock!{ EnvironmentVariable::inspect(RObject::new(*self.env), path.clone()) }; - let length = children.len(); - let msg = EnvironmentMessage::Details(EnvironmentMessageDetails { - path, - children, - length - }); + let msg = match inspect { + Ok(children) => { + let length = children.len(); + EnvironmentMessage::Details(EnvironmentMessageDetails { + path, + children, + length + }) + }, + Err(_) => { + EnvironmentMessage::Error(EnvironmentMessageError { + message: String::from("Inspection error") + }) + } + }; self.send_message(msg, request_id); + } fn send_message(&mut self, message: EnvironmentMessage, request_id: Option) { diff --git a/extensions/positron-r/amalthea/crates/ark/src/environment/variable.rs b/extensions/positron-r/amalthea/crates/ark/src/environment/variable.rs index 36080af4878..931890c230a 100644 --- a/extensions/positron-r/amalthea/crates/ark/src/environment/variable.rs +++ b/extensions/positron-r/amalthea/crates/ark/src/environment/variable.rs @@ -112,21 +112,20 @@ impl EnvironmentVariable { } } - pub fn inspect(env: RObject, path: Vec) -> Vec { + pub fn inspect(env: RObject, path: Vec) -> Result, harp::error::Error> { // for now only lists can be expanded let list = unsafe { RFunction::from(".ps.environment.resolveObjectFromPath") .param("env", env) .param("path", CharacterVector::create(path).cast()) - .call() - .unwrap() + .call()? }; let mut out : Vec = vec![]; let n = unsafe { XLENGTH(*list) }; - let names = unsafe{ - CharacterVector::new_unchecked(RFunction::from(".ps.environment.listDisplayNames").add(*list).call().unwrap()) + let names = unsafe { + CharacterVector::new_unchecked(RFunction::from(".ps.environment.listDisplayNames").add(*list).call()?) }; for i in 0..n { @@ -149,7 +148,7 @@ impl EnvironmentVariable { }); } - out + Ok(out) } } From 647cc3c3f3b1a7a7b99608565865ea4aa7cda973 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20Fran=C3=A7ois?= Date: Mon, 3 Apr 2023 14:06:28 +0200 Subject: [PATCH 14/21] massing path by reference --- .../amalthea/crates/ark/src/environment/r_environment.rs | 8 ++++---- .../amalthea/crates/ark/src/environment/variable.rs | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/extensions/positron-r/amalthea/crates/ark/src/environment/r_environment.rs b/extensions/positron-r/amalthea/crates/ark/src/environment/r_environment.rs index 02f8ce16c91..089ce52b967 100644 --- a/extensions/positron-r/amalthea/crates/ark/src/environment/r_environment.rs +++ b/extensions/positron-r/amalthea/crates/ark/src/environment/r_environment.rs @@ -163,7 +163,7 @@ impl REnvironment { }, EnvironmentMessage::Inspect(EnvironmentMessageInspect{path}) => { - self.inspect(path, Some(id)); + self.inspect(&path, Some(id)); }, _ => { @@ -282,15 +282,15 @@ impl REnvironment { self.update(request_id); } - fn inspect(&mut self, path: Vec, request_id: Option) { + fn inspect(&mut self, path: &Vec, request_id: Option) { let inspect = r_lock!{ - EnvironmentVariable::inspect(RObject::new(*self.env), path.clone()) + EnvironmentVariable::inspect(RObject::new(*self.env), &path) }; let msg = match inspect { Ok(children) => { let length = children.len(); EnvironmentMessage::Details(EnvironmentMessageDetails { - path, + path: path.clone(), children, length }) diff --git a/extensions/positron-r/amalthea/crates/ark/src/environment/variable.rs b/extensions/positron-r/amalthea/crates/ark/src/environment/variable.rs index 931890c230a..9c87bcd30e6 100644 --- a/extensions/positron-r/amalthea/crates/ark/src/environment/variable.rs +++ b/extensions/positron-r/amalthea/crates/ark/src/environment/variable.rs @@ -112,7 +112,7 @@ impl EnvironmentVariable { } } - pub fn inspect(env: RObject, path: Vec) -> Result, harp::error::Error> { + pub fn inspect(env: RObject, path: &Vec) -> Result, harp::error::Error> { // for now only lists can be expanded let list = unsafe { RFunction::from(".ps.environment.resolveObjectFromPath") From 8ec0f11468159e35ac1379dd5291d957bcb92d30 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20Fran=C3=A7ois?= Date: Mon, 3 Apr 2023 14:07:14 +0200 Subject: [PATCH 15/21] using ::view() --- .../amalthea/crates/ark/src/environment/r_environment.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extensions/positron-r/amalthea/crates/ark/src/environment/r_environment.rs b/extensions/positron-r/amalthea/crates/ark/src/environment/r_environment.rs index 089ce52b967..ea954fb7c02 100644 --- a/extensions/positron-r/amalthea/crates/ark/src/environment/r_environment.rs +++ b/extensions/positron-r/amalthea/crates/ark/src/environment/r_environment.rs @@ -284,7 +284,7 @@ impl REnvironment { fn inspect(&mut self, path: &Vec, request_id: Option) { let inspect = r_lock!{ - EnvironmentVariable::inspect(RObject::new(*self.env), &path) + EnvironmentVariable::inspect(RObject::view(*self.env), &path) }; let msg = match inspect { Ok(children) => { From fc9bb3f13ac13533246433b41c68f72eedc877a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20Fran=C3=A7ois?= Date: Mon, 3 Apr 2023 14:08:31 +0200 Subject: [PATCH 16/21] use libRsys::* --- .../amalthea/crates/ark/src/environment/variable.rs | 4 +--- extensions/positron-r/amalthea/crates/ark/src/shell.rs | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/extensions/positron-r/amalthea/crates/ark/src/environment/variable.rs b/extensions/positron-r/amalthea/crates/ark/src/environment/variable.rs index 9c87bcd30e6..4c3b023d480 100644 --- a/extensions/positron-r/amalthea/crates/ark/src/environment/variable.rs +++ b/extensions/positron-r/amalthea/crates/ark/src/environment/variable.rs @@ -14,9 +14,7 @@ use harp::object::RObject; use harp::utils::r_typeof; use harp::vector::CharacterVector; use harp::vector::Vector; -use libR_sys::VECSXP; -use libR_sys::VECTOR_ELT; -use libR_sys::XLENGTH; +use libR_sys::*; use serde::Deserialize; use serde::Serialize; diff --git a/extensions/positron-r/amalthea/crates/ark/src/shell.rs b/extensions/positron-r/amalthea/crates/ark/src/shell.rs index 5006087dee6..3f8cbf9158c 100644 --- a/extensions/positron-r/amalthea/crates/ark/src/shell.rs +++ b/extensions/positron-r/amalthea/crates/ark/src/shell.rs @@ -37,7 +37,7 @@ use harp::exec::r_parse_vector; use harp::exec::ParseResult; use harp::object::RObject; use harp::r_lock; -use libR_sys::R_GlobalEnv; +use libR_sys::*; use log::*; use serde_json::json; From 299304b03f8e6b649b14bf5e44bdcf2300756c17 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20Fran=C3=A7ois?= Date: Mon, 3 Apr 2023 14:24:07 +0200 Subject: [PATCH 17/21] =?UTF-8?q?returning=20the=20most=20specific=20class?= =?UTF-8?q?,=20i=C3=A7nstead=20of=20data.frame?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../amalthea/crates/harp/src/environment.rs | 30 +++++++++---------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/extensions/positron-r/amalthea/crates/harp/src/environment.rs b/extensions/positron-r/amalthea/crates/harp/src/environment.rs index e3f55d6e213..4fb15555c55 100644 --- a/extensions/positron-r/amalthea/crates/harp/src/environment.rs +++ b/extensions/positron-r/amalthea/crates/harp/src/environment.rs @@ -217,31 +217,31 @@ fn regular_binding_type(value: SEXP) -> BindingType { } fn vec_type(value: SEXP) -> String { - let rtype = match r_typeof(value) { + match r_typeof(value) { INTSXP => { if unsafe {r_inherits(value, "factor")} { - "fct" + String::from("fct") } else { - "int" + String::from("int") } }, - REALSXP => "dbl", - LGLSXP => "lgl", - STRSXP => "str", - RAWSXP => "raw", - CPLXSXP => "cplx", - VECSXP => { - if unsafe { r_inherits(value, "data.frame") } { - "data.frame" + REALSXP => String::from("dbl"), + LGLSXP => String::from("lgl"), + STRSXP => String::from("str"), + RAWSXP => String::from("raw"), + CPLXSXP => String::from("cplx"), + VECSXP => unsafe { + if r_inherits(value, "data.frame") { + let classes = CharacterVector::new_unchecked(Rf_getAttrib(value, R_ClassSymbol)); + classes.get_unchecked(0).unwrap() } else { - "list" + String::from("list") } }, // TODO: this should not happen - _ => "???" - }; - String::from(rtype) + _ => String::from("???") + } } fn vec_type_info(value: SEXP) -> BindingType { From fadfd2d89deee01b51ddc30fa9690de14269a5e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20Fran=C3=A7ois?= Date: Mon, 3 Apr 2023 14:41:53 +0200 Subject: [PATCH 18/21] indicate the number of levels --- .../positron-r/amalthea/crates/harp/src/environment.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/extensions/positron-r/amalthea/crates/harp/src/environment.rs b/extensions/positron-r/amalthea/crates/harp/src/environment.rs index 4fb15555c55..38514750919 100644 --- a/extensions/positron-r/amalthea/crates/harp/src/environment.rs +++ b/extensions/positron-r/amalthea/crates/harp/src/environment.rs @@ -218,9 +218,10 @@ fn regular_binding_type(value: SEXP) -> BindingType { fn vec_type(value: SEXP) -> String { match r_typeof(value) { - INTSXP => { - if unsafe {r_inherits(value, "factor")} { - String::from("fct") + INTSXP => unsafe { + if r_inherits(value, "factor") { + let levels = Rf_getAttrib(value, R_LevelsSymbol); + format!("fct({})", XLENGTH(levels)) } else { String::from("int") } From 8a42e6a473670b3b4c8c4a1490fb3196d72b3e10 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20Fran=C3=A7ois?= Date: Mon, 3 Apr 2023 16:58:36 +0200 Subject: [PATCH 19/21] add CharacterVector::quotes --- .../crates/harp/src/vector/character_vector.rs | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/extensions/positron-r/amalthea/crates/harp/src/vector/character_vector.rs b/extensions/positron-r/amalthea/crates/harp/src/vector/character_vector.rs index fd11bd7ff12..1d7bdc901a7 100644 --- a/extensions/positron-r/amalthea/crates/harp/src/vector/character_vector.rs +++ b/extensions/positron-r/amalthea/crates/harp/src/vector/character_vector.rs @@ -15,6 +15,17 @@ use crate::vector::Vector; #[harp_macros::vector] pub struct CharacterVector { object: RObject, + quotes: bool +} + +impl CharacterVector { + pub unsafe fn unquoted(object: impl Into) -> Self { + let object = object.into(); + Self { + object: RObject::new(object), + quotes: false + } + } } impl Vector for CharacterVector { @@ -32,6 +43,7 @@ impl Vector for CharacterVector { let object = object.into(); Self { object: RObject::new(object), + quotes: true } } @@ -78,7 +90,11 @@ impl Vector for CharacterVector { } fn format_one(&self, x: Self::Type) -> String { - format!("\"{}\"", x) + if self.quotes { + format!("\"{}\"", x) + } else { + x + } } } From 2fd262cf0eadd6d560eea892d259761622ba4bb5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20Fran=C3=A7ois?= Date: Mon, 3 Apr 2023 16:59:09 +0200 Subject: [PATCH 20/21] rework .ps.environment.listDisplayNames --- .../crates/ark/src/lsp/modules/public/environment.R | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/extensions/positron-r/amalthea/crates/ark/src/lsp/modules/public/environment.R b/extensions/positron-r/amalthea/crates/ark/src/lsp/modules/public/environment.R index d03ce96939b..11c1c16b350 100644 --- a/extensions/positron-r/amalthea/crates/ark/src/lsp/modules/public/environment.R +++ b/extensions/positron-r/amalthea/crates/ark/src/lsp/modules/public/environment.R @@ -7,7 +7,7 @@ .ps.environment.listDisplayNames <- function(x) { names <- names(x) if (is.null(names)) { - paste0("[[", seq_along(x), "]]") + names <- paste0("[[", seq_along(x), "]]") } else { empty <- which(names == "") names[empty] <- paste0("[[", empty, "]]") @@ -16,10 +16,12 @@ } .ps.environment.resolveObjectFromPath <- function(env, path) { - object <- env - rx_unnamed <- "^[[][[]([[:digit:]])[]][]]" + # start with environment + object <- env + + # and then move down the path for (p in path) { if (grepl(rx_unnamed, p)) { index <- as.integer(sub(rx_unnamed, "\\1", p)) From d8b0e932dd12f2da59c60ae1c8b13d4febe4fd70 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20Fran=C3=A7ois?= Date: Mon, 3 Apr 2023 16:59:30 +0200 Subject: [PATCH 21/21] try calling format() for non trivial vectors --- .../amalthea/crates/harp/src/environment.rs | 144 +++++++++++++----- 1 file changed, 107 insertions(+), 37 deletions(-) diff --git a/extensions/positron-r/amalthea/crates/harp/src/environment.rs b/extensions/positron-r/amalthea/crates/harp/src/environment.rs index 38514750919..5afacbeb993 100644 --- a/extensions/positron-r/amalthea/crates/harp/src/environment.rs +++ b/extensions/positron-r/amalthea/crates/harp/src/environment.rs @@ -163,8 +163,8 @@ impl Binding { pub fn has_children(&self) -> bool { match self.kind { // TODO: for now only lists have children - BindingKind::Regular => r_typeof(self.value) == VECSXP, - BindingKind::Promise(true) => r_typeof(unsafe{PRVALUE(self.value)}) == VECSXP, + BindingKind::Regular => has_children(self.value), + BindingKind::Promise(true) => has_children(unsafe{PRVALUE(self.value)}), // TODO: // - BindingKind::Promise(false) could have code and env as their children @@ -175,44 +175,129 @@ impl Binding { } -fn is_vector(value: SEXP) -> bool { - match r_typeof(value) { - LGLSXP => true, - INTSXP => true, - REALSXP => true, - CPLXSXP => true, - STRSXP => true, - RAWSXP => true, - VECSXP => true, - - _ => false +fn has_children(value: SEXP) -> bool { + r_typeof(value) == VECSXP && !unsafe{ r_inherits(value, "POSIXlt") } +} + +fn is_simple_vector(value: SEXP) -> bool { + unsafe { + let class = Rf_getAttrib(value, R_ClassSymbol); + + match r_typeof(value) { + LGLSXP => class == R_NilValue, + INTSXP => class == R_NilValue || r_inherits(value, "factor"), + REALSXP => class == R_NilValue, + CPLXSXP => class == R_NilValue, + STRSXP => class == R_NilValue, + RAWSXP => class == R_NilValue, + + _ => false + } + } + +} + +fn first_class(value: SEXP) -> Option { + unsafe { + let classes = Rf_getAttrib(value, R_ClassSymbol); + if classes == R_NilValue { + None + } else { + let classes = CharacterVector::new_unchecked(classes); + Some(classes.get_unchecked(0).unwrap()) + } + } +} + +fn all_classes(value: SEXP) -> String { + unsafe { + let classes = Rf_getAttrib(value, R_ClassSymbol); + let classes = CharacterVector::new_unchecked(classes); + classes.format("/", 0).1 } } fn regular_binding_display_value(value: SEXP) -> BindingValue { - // TODO: some form of R based dispatch - if is_vector(value) { + if is_simple_vector(value) { vec_glimpse(value) - } else { + } else if r_typeof(value) == VECSXP && ! unsafe{r_inherits(value, "POSIXlt")}{ + // TODO : handle records such as POSIXlt etc ... + // This includes data frames + BindingValue::empty() + } else { // TODO: - // - list (or does that go in vector too) - // - data.frame // - function // - environment // - pairlist // - calls - BindingValue::new(String::from("???"), false) + unsafe { + // try to call format() on the object + let formatted = RFunction::new("base", "format") + .add(value) + .call(); + + match formatted { + Ok(fmt) => { + if r_typeof(*fmt) == STRSXP { + let fmt = CharacterVector::unquoted(*fmt); + let fmt = fmt.format(", ", 100); + + BindingValue::new(fmt.1, fmt.0) + } else { + BindingValue::new(String::from("???"), false) + } + }, + Err(_) => { + BindingValue::new(String::from("???"), false) + } + } + } } } fn regular_binding_type(value: SEXP) -> BindingType { - if is_vector(value) { + if is_simple_vector(value) { vec_type_info(value) + } else if r_typeof(value) == VECSXP { + unsafe { + if r_inherits(value, "data.frame") { + let dfclass = first_class(value).unwrap(); + + let dim = RFunction::new("base", "dim.data.frame") + .add(value) + .call() + .unwrap(); + + let dim = IntegerVector::new(dim).unwrap(); + let shape = dim.format(",", 0).1; + + BindingType::new( + format!("{} [{}]", dfclass, shape), + + // TODO: add info about column types ? + format!("{} [{}]", dfclass, shape) + ) + } else { + match first_class(value) { + None => BindingType::new(String::from("list"), String::from("list")), + Some(class) => { + BindingType::new(class, all_classes(value)) + } + } + // TODO: should type_info be different ? + // TODO: deal with record types, e.g. POSIXlt + + } + } } else { - BindingType::new(String::from("???"), String::from("")) + let class = first_class(value); + match class { + Some(class) => BindingType::new(class, all_classes(value)), + None => BindingType::new(String::from("???"), String::from("")) + } } } @@ -231,14 +316,6 @@ fn vec_type(value: SEXP) -> String { STRSXP => String::from("str"), RAWSXP => String::from("raw"), CPLXSXP => String::from("cplx"), - VECSXP => unsafe { - if r_inherits(value, "data.frame") { - let classes = CharacterVector::new_unchecked(Rf_getAttrib(value, R_ClassSymbol)); - classes.get_unchecked(0).unwrap() - } else { - String::from("list") - } - }, // TODO: this should not happen _ => String::from("???") @@ -258,14 +335,7 @@ fn vec_type_info(value: SEXP) -> BindingType { fn vec_shape(value: SEXP) -> String { unsafe { - let dim = if r_inherits(value, "data.frame") { - RFunction::new("base", "dim.data.frame") - .add(value) - .call() - .unwrap() - } else { - RObject::new(Rf_getAttrib(value, R_DimSymbol)) - }; + let dim = RObject::new(Rf_getAttrib(value, R_DimSymbol)); if *dim == R_NilValue { format!("{}", Rf_xlength(value))