From 24562038de10dea75b0d2a5590458492b654b260 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Romain=20Fran=C3=A7ois?= <romain@posit.co>
Date: Mon, 23 Jan 2023 14:22:17 +0100
Subject: [PATCH 01/30] r_top_level_exec() and r_parse_vector()

---
 .../amalthea/crates/harp/src/exec.rs          | 163 ++++++++++++++++++
 1 file changed, 163 insertions(+)

diff --git a/extensions/positron-r/amalthea/crates/harp/src/exec.rs b/extensions/positron-r/amalthea/crates/harp/src/exec.rs
index 59653dee5b78..d1ce2ba23ec9 100644
--- a/extensions/positron-r/amalthea/crates/harp/src/exec.rs
+++ b/extensions/positron-r/amalthea/crates/harp/src/exec.rs
@@ -6,6 +6,10 @@
 //
 
 use std::ffi::CStr;
+use std::mem;
+use std::os::raw::c_int;
+use std::os::raw::c_void;
+use std::os::raw::c_char;
 
 use libR_sys::*;
 
@@ -18,6 +22,18 @@ use crate::utils::r_inherits;
 use crate::utils::r_stringify;
 use crate::utils::r_typeof;
 
+extern "C" {
+    pub static R_ParseError: c_int;
+}
+
+extern "C" {
+    pub static R_ParseErrorMsg: [c_char; 256usize];
+}
+
+extern "C" {
+    pub static R_ParseContext: [c_char; 256usize];
+}
+
 pub struct RArgument {
     pub name: String,
     pub value: RObject,
@@ -178,6 +194,67 @@ pub unsafe fn geterrmessage() -> String {
 
 }
 
+pub unsafe fn r_top_level_exec<F>(mut f: F) -> bool where F: FnMut() {
+    let mut cb: &mut dyn FnMut() = &mut f;
+    let cb = &mut cb;
+
+    extern fn top_level_exec_fn(arg: *mut c_void) {
+        let closure: &mut &mut dyn FnMut() = unsafe { mem::transmute(arg) };
+        closure()
+    }
+
+    let success = R_ToplevelExec(
+        Some(top_level_exec_fn),
+        cb as *mut _ as *mut c_void
+    );
+
+    success == Rboolean_TRUE
+}
+
+pub enum ParseResult {
+    Ok(SEXP),
+    Incomplete(),
+    Error {
+        message: String
+    },
+    SyntaxError {
+        message: String,
+        line: i32
+    }
+}
+
+#[allow(non_upper_case_globals)]
+pub unsafe fn r_parse_vector(code: String) -> ParseResult {
+
+    let mut ps : ParseStatus = 0;
+    let mut protect = RProtect::new();
+    let r_code = protect.add(crate::r_string!(code));
+
+    let mut out: SEXP = R_NilValue;
+
+    let success = r_top_level_exec(|| {
+        out = R_ParseVector(r_code, -1, &mut ps, R_NilValue);
+    });
+
+    match success {
+        false => ParseResult::Error{
+            message: geterrmessage()
+        },
+        true => {
+            match ps {
+                ParseStatus_PARSE_OK => ParseResult::Ok(out),
+                ParseStatus_PARSE_INCOMPLETE => ParseResult::Incomplete(),
+                _ => {
+                    ParseResult::SyntaxError{
+                        message: CStr::from_ptr(R_ParseErrorMsg.as_ptr()).to_string_lossy().to_string(),
+                        line: R_ParseError as i32
+                    }
+                }
+            }
+        }
+    }
+}
+
 #[cfg(test)]
 mod tests {
 
@@ -275,5 +352,91 @@ mod tests {
 
     }}
 
+    #[test]
+    fn test_top_level_exec() { r_test! {
+        let mut ps : ParseStatus = 0;
+        let mut protect = RProtect::new();
+        let code = protect.add(crate::r_string!("force(42)"));
+
+        let mut out: SEXP = R_NilValue;
+
+        // successfull
+        let success = r_top_level_exec(|| {
+            out = R_ParseVector(code, -1, &mut ps, R_NilValue);
+        });
+        assert_eq!(success, true);
+        assert_eq!(r_typeof(out), EXPRSXP as u32);
+
+        let call = VECTOR_ELT(out, 0);
+        assert_eq!(r_typeof(call), LANGSXP as u32);
+        assert_eq!(Rf_length(call), 2);
+        assert_eq!(CAR(call), r_symbol!("force"));
+
+        let arg = CADR(call);
+        assert_eq!(r_typeof(arg), REALSXP as u32);
+        assert_eq!(*REAL(arg), 42.0);
+
+        // failed
+        let code = protect.add(crate::r_string!("42 + _"));
+        let success = r_top_level_exec(|| {
+            out = R_ParseVector(code, -1, &mut ps, R_NilValue);
+        });
+        assert_eq!(success, false);
+    }}
+
+    #[test]
+    fn test_parse_vector() { r_test! {
+        // complete
+        assert!(
+            match r_parse_vector(String::from("force(42)")) {
+                ParseResult::Ok(out) => {
+                    assert_eq!(r_typeof(out), EXPRSXP as u32);
+
+                    let call = VECTOR_ELT(out, 0);
+                    assert_eq!(r_typeof(call), LANGSXP as u32);
+                    assert_eq!(Rf_length(call), 2);
+                    assert_eq!(CAR(call), r_symbol!("force"));
+
+                    let arg = CADR(call);
+                    assert_eq!(r_typeof(arg), REALSXP as u32);
+                    assert_eq!(*REAL(arg), 42.0);
+
+                    true
+                },
+                _ => false
+            }
+        );
+
+        // incomplete
+        assert!(match r_parse_vector(String::from("force(42")) {
+            ParseResult::Incomplete() => true,
+            _ => false
+        });
+
+        // error
+        assert!(match r_parse_vector(String::from("42 + _")) {
+            ParseResult::Error {message} => {
+                assert!(message.contains("invalid use of pipe placeholder"));
+
+                true
+            },
+            _ => false
+        });
+
+        // "normal" syntax error
+        assert!(match r_parse_vector(String::from("1+1\n*42")) {
+            ParseResult::SyntaxError {message, line} => {
+                assert!(message.contains("unexpected"));
+                assert_eq!(line, 2);
+
+                true
+            },
+            _ => false
+        });
+
+    }}
+
+
+
 }
 

From 1a741770804c7c22632e83780cd778a04647f152 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Romain=20Fran=C3=A7ois?= <romain@posit.co>
Date: Mon, 23 Jan 2023 17:49:55 +0100
Subject: [PATCH 02/30] simplify r_top_level_exec() use with returned value

---
 .../amalthea/crates/harp/src/exec.rs          | 50 +++++++++++--------
 1 file changed, 29 insertions(+), 21 deletions(-)

diff --git a/extensions/positron-r/amalthea/crates/harp/src/exec.rs b/extensions/positron-r/amalthea/crates/harp/src/exec.rs
index d1ce2ba23ec9..34ee46716fdc 100644
--- a/extensions/positron-r/amalthea/crates/harp/src/exec.rs
+++ b/extensions/positron-r/amalthea/crates/harp/src/exec.rs
@@ -10,6 +10,7 @@ use std::mem;
 use std::os::raw::c_int;
 use std::os::raw::c_void;
 use std::os::raw::c_char;
+use std::mem::MaybeUninit;
 
 use libR_sys::*;
 
@@ -194,13 +195,18 @@ pub unsafe fn geterrmessage() -> String {
 
 }
 
-pub unsafe fn r_top_level_exec<F>(mut f: F) -> bool where F: FnMut() {
-    let mut cb: &mut dyn FnMut() = &mut f;
+pub unsafe fn r_top_level_exec<F, R>(mut f: F) -> Result<R> where F: FnMut() -> R {
+    let mut result: MaybeUninit<R> = MaybeUninit::uninit();
+
+    let mut enclos = || {
+        result.write(f());
+    };
+    let mut cb: &mut dyn FnMut() = &mut enclos;
     let cb = &mut cb;
 
     extern fn top_level_exec_fn(arg: *mut c_void) {
         let closure: &mut &mut dyn FnMut() = unsafe { mem::transmute(arg) };
-        closure()
+        closure();
     }
 
     let success = R_ToplevelExec(
@@ -208,7 +214,13 @@ pub unsafe fn r_top_level_exec<F>(mut f: F) -> bool where F: FnMut() {
         cb as *mut _ as *mut c_void
     );
 
-    success == Rboolean_TRUE
+    match success != 0 {
+        false => Err(Error::EvaluationError { code: String::from(""), message: String::from("") }),
+        true => {
+            Ok(result.assume_init())
+        }
+    }
+
 }
 
 pub enum ParseResult {
@@ -230,17 +242,15 @@ pub unsafe fn r_parse_vector(code: String) -> ParseResult {
     let mut protect = RProtect::new();
     let r_code = protect.add(crate::r_string!(code));
 
-    let mut out: SEXP = R_NilValue;
+    let lambda = || {
+        R_ParseVector(r_code, -1, &mut ps, R_NilValue)
+    };
 
-    let success = r_top_level_exec(|| {
-        out = R_ParseVector(r_code, -1, &mut ps, R_NilValue);
-    });
-
-    match success {
-        false => ParseResult::Error{
+    match r_top_level_exec(lambda) {
+        Err(_) => ParseResult::Error{
             message: geterrmessage()
         },
-        true => {
+        Ok(out) => {
             match ps {
                 ParseStatus_PARSE_OK => ParseResult::Ok(out),
                 ParseStatus_PARSE_INCOMPLETE => ParseResult::Incomplete(),
@@ -358,13 +368,10 @@ mod tests {
         let mut protect = RProtect::new();
         let code = protect.add(crate::r_string!("force(42)"));
 
-        let mut out: SEXP = R_NilValue;
-
         // successfull
-        let success = r_top_level_exec(|| {
-            out = R_ParseVector(code, -1, &mut ps, R_NilValue);
-        });
-        assert_eq!(success, true);
+        let out = r_top_level_exec(|| {
+            R_ParseVector(code, -1, &mut ps, R_NilValue)
+        }).unwrap();
         assert_eq!(r_typeof(out), EXPRSXP as u32);
 
         let call = VECTOR_ELT(out, 0);
@@ -378,10 +385,11 @@ mod tests {
 
         // failed
         let code = protect.add(crate::r_string!("42 + _"));
-        let success = r_top_level_exec(|| {
-            out = R_ParseVector(code, -1, &mut ps, R_NilValue);
+        let failed = r_top_level_exec(|| {
+            R_ParseVector(code, -1, &mut ps, R_NilValue)
         });
-        assert_eq!(success, false);
+        assert!(failed.is_err());
+
     }}
 
     #[test]

From eac6bf18956aebb064ff7ab5a4055ce56dc7c3a7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Romain=20Fran=C3=A7ois?= <romain@posit.co>
Date: Mon, 23 Jan 2023 18:31:44 +0100
Subject: [PATCH 03/30] minor code polish

---
 .../amalthea/crates/harp/src/error.rs         |  7 +++++-
 .../amalthea/crates/harp/src/exec.rs          | 22 ++++++++++---------
 2 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/extensions/positron-r/amalthea/crates/harp/src/error.rs b/extensions/positron-r/amalthea/crates/harp/src/error.rs
index dcc346576245..6fa9a7a5c122 100644
--- a/extensions/positron-r/amalthea/crates/harp/src/error.rs
+++ b/extensions/positron-r/amalthea/crates/harp/src/error.rs
@@ -20,7 +20,8 @@ pub enum Error {
     UnsafeEvaluationError(String),
     UnexpectedLength(u32, u32),
     UnexpectedType(u32, Vec<u32>),
-    InvalidUtf8(Utf8Error)
+    InvalidUtf8(Utf8Error),
+    TopLevelExecError()
 }
 
 // empty implementation required for 'anyhow'
@@ -74,6 +75,10 @@ impl fmt::Display for Error {
                 write!(f, "Invalid UTF-8 in string: {}", error)
             }
 
+            Error::TopLevelExecError() => {
+                write!(f, "Top Level exec error")
+            }
+
         }
     }
 }
diff --git a/extensions/positron-r/amalthea/crates/harp/src/exec.rs b/extensions/positron-r/amalthea/crates/harp/src/exec.rs
index 34ee46716fdc..8c932a9a007e 100644
--- a/extensions/positron-r/amalthea/crates/harp/src/exec.rs
+++ b/extensions/positron-r/amalthea/crates/harp/src/exec.rs
@@ -195,14 +195,15 @@ pub unsafe fn geterrmessage() -> String {
 
 }
 
-pub unsafe fn r_top_level_exec<F, R>(mut f: F) -> Result<R> where F: FnMut() -> R {
+pub unsafe fn r_top_level_exec<F, R>(mut fun: F) -> Result<R> where F: FnMut() -> R {
+    // this will hold the result of calling fun() on success
     let mut result: MaybeUninit<R> = MaybeUninit::uninit();
 
-    let mut enclos = || {
-        result.write(f());
+    // wrap fun into a void closure
+    let mut void_closure: &mut dyn FnMut() = &mut || {
+        result.write(fun());
     };
-    let mut cb: &mut dyn FnMut() = &mut enclos;
-    let cb = &mut cb;
+    let void_closure = &mut void_closure;
 
     extern fn top_level_exec_fn(arg: *mut c_void) {
         let closure: &mut &mut dyn FnMut() = unsafe { mem::transmute(arg) };
@@ -211,14 +212,15 @@ pub unsafe fn r_top_level_exec<F, R>(mut f: F) -> Result<R> where F: FnMut() ->
 
     let success = R_ToplevelExec(
         Some(top_level_exec_fn),
-        cb as *mut _ as *mut c_void
+        void_closure as *mut _ as *mut c_void
     );
 
     match success != 0 {
-        false => Err(Error::EvaluationError { code: String::from(""), message: String::from("") }),
-        true => {
-            Ok(result.assume_init())
-        }
+        false => Err(Error::TopLevelExecError()),
+
+        // there was no jump, so we can assume
+        // result has been initialized
+        true => Ok(result.assume_init())
     }
 
 }

From 42bbcea1913b9b7116b8f7a66c9abc91b10e9729 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Romain=20Fran=C3=A7ois?= <romain@posit.co>
Date: Tue, 24 Jan 2023 13:29:32 +0100
Subject: [PATCH 04/30] + r_try_catch_error() around R_tryCatchError()

---
 .../amalthea/crates/harp/src/error.rs         | 33 ++++++++
 .../amalthea/crates/harp/src/exec.rs          | 77 ++++++++++++++++++-
 .../amalthea/crates/harp/src/lib.rs           |  4 +-
 3 files changed, 110 insertions(+), 4 deletions(-)

diff --git a/extensions/positron-r/amalthea/crates/harp/src/error.rs b/extensions/positron-r/amalthea/crates/harp/src/error.rs
index 6fa9a7a5c122..8a9ce936c7b3 100644
--- a/extensions/positron-r/amalthea/crates/harp/src/error.rs
+++ b/extensions/positron-r/amalthea/crates/harp/src/error.rs
@@ -8,6 +8,11 @@
 use std::fmt;
 use std::str::Utf8Error;
 
+use libR_sys::{Rf_getAttrib, R_ClassSymbol, SEXP, Rf_lang2, Rf_eval, R_GlobalEnv};
+
+use crate::object::RObject;
+use crate::protect::RProtect;
+use crate::r_symbol;
 use crate::utils::r_type2char;
 
 pub type Result<T> = std::result::Result<T, Error>;
@@ -24,6 +29,33 @@ pub enum Error {
     TopLevelExecError()
 }
 
+pub struct TryCatchError {
+    condition: RObject
+}
+
+impl TryCatchError {
+    pub fn new(condition: SEXP) -> TryCatchError {
+        TryCatchError {
+            condition: RObject::from(condition)
+        }
+    }
+
+    pub fn classes(&self) -> Vec<String>  {
+        unsafe {
+            RObject::from(Rf_getAttrib(*self.condition, R_ClassSymbol)).try_into().unwrap()
+        }
+    }
+
+    pub fn message(&self) -> Vec<String> {
+        unsafe {
+            let mut protect = RProtect::new();
+            let call = protect.add(Rf_lang2(r_symbol!("conditionMessage"), *self.condition));
+
+            RObject::from(Rf_eval(call, R_GlobalEnv)).try_into().unwrap()
+        }
+    }
+}
+
 // empty implementation required for 'anyhow'
 impl std::error::Error for Error {
 
@@ -79,6 +111,7 @@ impl fmt::Display for Error {
                 write!(f, "Top Level exec error")
             }
 
+
         }
     }
 }
diff --git a/extensions/positron-r/amalthea/crates/harp/src/exec.rs b/extensions/positron-r/amalthea/crates/harp/src/exec.rs
index 8c932a9a007e..ce18edab70f3 100644
--- a/extensions/positron-r/amalthea/crates/harp/src/exec.rs
+++ b/extensions/positron-r/amalthea/crates/harp/src/exec.rs
@@ -16,6 +16,7 @@ use libR_sys::*;
 
 use crate::error::Error;
 use crate::error::Result;
+use crate::error::TryCatchError;
 use crate::object::RObject;
 use crate::protect::RProtect;
 use crate::r_symbol;
@@ -225,6 +226,41 @@ pub unsafe fn r_top_level_exec<F, R>(mut fun: F) -> Result<R> where F: FnMut() -
 
 }
 
+pub unsafe fn r_try_catch_error<F>(mut fun: F) -> std::result::Result<RObject, TryCatchError> where F: FnMut() -> SEXP {
+    extern fn body_fn(arg: *mut c_void) -> SEXP {
+        let closure: &mut &mut dyn FnMut() -> SEXP = unsafe { mem::transmute(arg) };
+        closure()
+    }
+
+    // handler just returns the condition and sets success to false
+    let mut success: bool = true;
+    extern fn handler_fn(condition: SEXP, arg: *mut c_void) -> SEXP {
+        let success_ptr = arg as *mut bool;
+        unsafe {
+            *success_ptr = false;
+        }
+
+        condition
+    }
+
+    let mut body_data: &mut dyn FnMut() -> SEXP = &mut fun;
+    let body_data = &mut body_data;
+
+    let success_ptr: *mut bool = &mut success;
+
+    let result = R_tryCatchError(
+        Some(body_fn),
+        body_data as *mut _ as *mut c_void,
+        Some(handler_fn),
+        success_ptr as *mut c_void
+    );
+    match success {
+        true => Ok(RObject::from(result)),
+        false => Err(TryCatchError::new(result))
+    }
+
+}
+
 pub enum ParseResult {
     Ok(SEXP),
     Incomplete(),
@@ -248,13 +284,14 @@ pub unsafe fn r_parse_vector(code: String) -> ParseResult {
         R_ParseVector(r_code, -1, &mut ps, R_NilValue)
     };
 
-    match r_top_level_exec(lambda) {
+    match r_try_catch_error(lambda) {
         Err(_) => ParseResult::Error{
             message: geterrmessage()
         },
+
         Ok(out) => {
             match ps {
-                ParseStatus_PARSE_OK => ParseResult::Ok(out),
+                ParseStatus_PARSE_OK => ParseResult::Ok(*out),
                 ParseStatus_PARSE_INCOMPLETE => ParseResult::Incomplete(),
                 _ => {
                     ParseResult::SyntaxError{
@@ -270,6 +307,7 @@ pub unsafe fn r_parse_vector(code: String) -> ParseResult {
 #[cfg(test)]
 mod tests {
 
+    use std::ffi::CString;
     use std::io::Write;
 
     use crate::r_lock;
@@ -394,6 +432,41 @@ mod tests {
 
     }}
 
+    #[test]
+    fn test_try_catch_error(){ r_test! {
+
+        let ok = r_try_catch_error(|| {
+            Rf_ScalarInteger(42)
+        });
+        assert!(match ok {
+            Ok(value) => {
+                assert_eq!(INTEGER_ELT(*value, 0), 42);
+
+                true
+            },
+
+            Err(_) => false
+        });
+
+
+        let out = r_try_catch_error(|| {
+            let msg = CString::new("ouch").unwrap();
+            Rf_error(unsafe {msg.as_ptr()});
+
+            R_NilValue
+        });
+
+        assert!(match out {
+            Err(err) => {
+                assert_eq!(err.message(), ["ouch"]);
+                assert_eq!(err.classes(), ["simpleError", "error", "condition"]);
+                true
+            },
+            _ => false
+        });
+
+    }}
+
     #[test]
     fn test_parse_vector() { r_test! {
         // complete
diff --git a/extensions/positron-r/amalthea/crates/harp/src/lib.rs b/extensions/positron-r/amalthea/crates/harp/src/lib.rs
index ab9cc94000f8..f911f6af02b7 100644
--- a/extensions/positron-r/amalthea/crates/harp/src/lib.rs
+++ b/extensions/positron-r/amalthea/crates/harp/src/lib.rs
@@ -28,13 +28,13 @@ macro_rules! r_symbol {
     ($id:literal) => {{
         use std::os::raw::c_char;
         let value = concat!($id, "\0");
-        Rf_install(value.as_ptr() as *const c_char)
+        libR_sys::Rf_install(value.as_ptr() as *const c_char)
     }};
 
     ($id:expr) => {{
         use std::os::raw::c_char;
         let cstr = [&*$id, "\0"].concat();
-        Rf_install(cstr.as_ptr() as *const c_char)
+        libR_sys::Rf_install(cstr.as_ptr() as *const c_char)
     }};
 
 }

From a9009cea54dd60d905272ac56e26eb778f8cf870 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Romain=20Fran=C3=A7ois?= <romain@posit.co>
Date: Tue, 24 Jan 2023 13:47:08 +0100
Subject: [PATCH 05/30] use baseenv()

---
 extensions/positron-r/amalthea/crates/harp/src/error.rs | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/extensions/positron-r/amalthea/crates/harp/src/error.rs b/extensions/positron-r/amalthea/crates/harp/src/error.rs
index 8a9ce936c7b3..dfae289342c3 100644
--- a/extensions/positron-r/amalthea/crates/harp/src/error.rs
+++ b/extensions/positron-r/amalthea/crates/harp/src/error.rs
@@ -8,7 +8,7 @@
 use std::fmt;
 use std::str::Utf8Error;
 
-use libR_sys::{Rf_getAttrib, R_ClassSymbol, SEXP, Rf_lang2, Rf_eval, R_GlobalEnv};
+use libR_sys::{Rf_getAttrib, R_ClassSymbol, SEXP, Rf_lang2, Rf_eval, R_BaseEnv};
 
 use crate::object::RObject;
 use crate::protect::RProtect;
@@ -51,7 +51,7 @@ impl TryCatchError {
             let mut protect = RProtect::new();
             let call = protect.add(Rf_lang2(r_symbol!("conditionMessage"), *self.condition));
 
-            RObject::from(Rf_eval(call, R_GlobalEnv)).try_into().unwrap()
+            RObject::from(Rf_eval(call, R_BaseEnv)).try_into().unwrap()
         }
     }
 }

From e9633c121a6fcf34ae1d215e932cb4e55a6bd9a2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Romain=20Fran=C3=A7ois?= <romain@posit.co>
Date: Wed, 25 Jan 2023 18:33:13 +0100
Subject: [PATCH 06/30] RError

---
 .../amalthea/crates/harp/src/error.rs         | 29 +++++++-----
 .../amalthea/crates/harp/src/exec.rs          | 46 ++++++++-----------
 2 files changed, 36 insertions(+), 39 deletions(-)

diff --git a/extensions/positron-r/amalthea/crates/harp/src/error.rs b/extensions/positron-r/amalthea/crates/harp/src/error.rs
index dfae289342c3..4cf731d2c813 100644
--- a/extensions/positron-r/amalthea/crates/harp/src/error.rs
+++ b/extensions/positron-r/amalthea/crates/harp/src/error.rs
@@ -13,7 +13,7 @@ use libR_sys::{Rf_getAttrib, R_ClassSymbol, SEXP, Rf_lang2, Rf_eval, R_BaseEnv};
 use crate::object::RObject;
 use crate::protect::RProtect;
 use crate::r_symbol;
-use crate::utils::r_type2char;
+use crate::utils::{r_type2char, r_inherits};
 
 pub type Result<T> = std::result::Result<T, Error>;
 
@@ -29,31 +29,38 @@ pub enum Error {
     TopLevelExecError()
 }
 
-pub struct TryCatchError {
-    condition: RObject
-}
+pub struct RError(pub RObject);
 
-impl TryCatchError {
-    pub fn new(condition: SEXP) -> TryCatchError {
-        TryCatchError {
-            condition: RObject::from(condition)
-        }
+impl RError {
+    pub fn new(condition: SEXP) -> Self {
+        RError(RObject::from(condition))
+    }
+
+    pub fn condition(&self) -> SEXP {
+        *self.0
     }
 
     pub fn classes(&self) -> Vec<String>  {
         unsafe {
-            RObject::from(Rf_getAttrib(*self.condition, R_ClassSymbol)).try_into().unwrap()
+            RObject::from(Rf_getAttrib(*self.0, R_ClassSymbol)).try_into().unwrap()
         }
     }
 
     pub fn message(&self) -> Vec<String> {
         unsafe {
             let mut protect = RProtect::new();
-            let call = protect.add(Rf_lang2(r_symbol!("conditionMessage"), *self.condition));
+            let call = protect.add(Rf_lang2(r_symbol!("conditionMessage"), *self.0));
 
             RObject::from(Rf_eval(call, R_BaseEnv)).try_into().unwrap()
         }
     }
+
+    pub fn inherits(&self, class: &str) -> bool {
+        unsafe {
+            r_inherits(*self.0, &class)
+        }
+    }
+
 }
 
 // empty implementation required for 'anyhow'
diff --git a/extensions/positron-r/amalthea/crates/harp/src/exec.rs b/extensions/positron-r/amalthea/crates/harp/src/exec.rs
index ce18edab70f3..a7bbed5eef63 100644
--- a/extensions/positron-r/amalthea/crates/harp/src/exec.rs
+++ b/extensions/positron-r/amalthea/crates/harp/src/exec.rs
@@ -15,8 +15,8 @@ use std::mem::MaybeUninit;
 use libR_sys::*;
 
 use crate::error::Error;
+use crate::error::RError;
 use crate::error::Result;
-use crate::error::TryCatchError;
 use crate::object::RObject;
 use crate::protect::RProtect;
 use crate::r_symbol;
@@ -32,10 +32,6 @@ extern "C" {
     pub static R_ParseErrorMsg: [c_char; 256usize];
 }
 
-extern "C" {
-    pub static R_ParseContext: [c_char; 256usize];
-}
-
 pub struct RArgument {
     pub name: String,
     pub value: RObject,
@@ -226,7 +222,7 @@ pub unsafe fn r_top_level_exec<F, R>(mut fun: F) -> Result<R> where F: FnMut() -
 
 }
 
-pub unsafe fn r_try_catch_error<F>(mut fun: F) -> std::result::Result<RObject, TryCatchError> where F: FnMut() -> SEXP {
+pub unsafe fn r_try_catch_error<F>(mut fun: F) -> std::result::Result<RObject, RError> where F: FnMut() -> SEXP {
     extern fn body_fn(arg: *mut c_void) -> SEXP {
         let closure: &mut &mut dyn FnMut() -> SEXP = unsafe { mem::transmute(arg) };
         closure()
@@ -256,21 +252,20 @@ pub unsafe fn r_try_catch_error<F>(mut fun: F) -> std::result::Result<RObject, T
     );
     match success {
         true => Ok(RObject::from(result)),
-        false => Err(TryCatchError::new(result))
+        false => Err(RError::new(result))
     }
 
 }
 
+
 pub enum ParseResult {
     Ok(SEXP),
     Incomplete(),
-    Error {
-        message: String
-    },
     SyntaxError {
         message: String,
         line: i32
-    }
+    },
+    ParseError(RError)
 }
 
 #[allow(non_upper_case_globals)]
@@ -285,9 +280,7 @@ pub unsafe fn r_parse_vector(code: String) -> ParseResult {
     };
 
     match r_try_catch_error(lambda) {
-        Err(_) => ParseResult::Error{
-            message: geterrmessage()
-        },
+        Err(error) => ParseResult::ParseError(RError::new(*error.0)),
 
         Ok(out) => {
             match ps {
@@ -424,9 +417,10 @@ mod tests {
         assert_eq!(*REAL(arg), 42.0);
 
         // failed
-        let code = protect.add(crate::r_string!("42 + _"));
+        let msg = CString::new("ouch").unwrap();
+        let err_msg = unsafe {msg.as_ptr()};
         let failed = r_top_level_exec(|| {
-            R_ParseVector(code, -1, &mut ps, R_NilValue)
+            Rf_error(err_msg);
         });
         assert!(failed.is_err());
 
@@ -491,20 +485,16 @@ mod tests {
         );
 
         // incomplete
-        assert!(match r_parse_vector(String::from("force(42")) {
-            ParseResult::Incomplete() => true,
-            _ => false
-        });
+        assert!(matches!(
+            r_parse_vector(String::from("force(42")),
+            ParseResult::Incomplete()
+        ));
 
         // error
-        assert!(match r_parse_vector(String::from("42 + _")) {
-            ParseResult::Error {message} => {
-                assert!(message.contains("invalid use of pipe placeholder"));
-
-                true
-            },
-            _ => false
-        });
+        assert!(matches!(
+            r_parse_vector(String::from("42 + _")),
+            ParseResult::ParseError(_)
+        ));
 
         // "normal" syntax error
         assert!(match r_parse_vector(String::from("1+1\n*42")) {

From d63544540adbf87562e9f1e157a354ff755557ba Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Romain=20Fran=C3=A7ois?= <romain@posit.co>
Date: Thu, 26 Jan 2023 09:52:14 +0100
Subject: [PATCH 07/30] + `assert_match!()`

---
 .../amalthea/crates/harp/src/exec.rs          | 72 ++++++++-----------
 .../amalthea/crates/harp/src/lib.rs           | 32 +++++++++
 2 files changed, 60 insertions(+), 44 deletions(-)

diff --git a/extensions/positron-r/amalthea/crates/harp/src/exec.rs b/extensions/positron-r/amalthea/crates/harp/src/exec.rs
index a7bbed5eef63..d14d6e4346da 100644
--- a/extensions/positron-r/amalthea/crates/harp/src/exec.rs
+++ b/extensions/positron-r/amalthea/crates/harp/src/exec.rs
@@ -303,6 +303,7 @@ mod tests {
     use std::ffi::CString;
     use std::io::Write;
 
+    use crate::assert_match;
     use crate::r_lock;
     use crate::r_test;
     use crate::r_test_unlocked;
@@ -432,14 +433,9 @@ mod tests {
         let ok = r_try_catch_error(|| {
             Rf_ScalarInteger(42)
         });
-        assert!(match ok {
-            Ok(value) => {
-                assert_eq!(INTEGER_ELT(*value, 0), 42);
-
-                true
-            },
-
-            Err(_) => false
+        assert_match!(ok, Ok(value) => {
+            assert_eq!(r_typeof(*value), INTSXP as u32);
+            assert_eq!(INTEGER_ELT(*value, 0), 42);
         });
 
 
@@ -450,13 +446,9 @@ mod tests {
             R_NilValue
         });
 
-        assert!(match out {
-            Err(err) => {
-                assert_eq!(err.message(), ["ouch"]);
-                assert_eq!(err.classes(), ["simpleError", "error", "condition"]);
-                true
-            },
-            _ => false
+        assert_match!(out, Err(err) => {
+            assert_eq!(err.message(), ["ouch"]);
+            assert_eq!(err.classes(), ["simpleError", "error", "condition"]);
         });
 
     }}
@@ -464,52 +456,44 @@ mod tests {
     #[test]
     fn test_parse_vector() { r_test! {
         // complete
-        assert!(
-            match r_parse_vector(String::from("force(42)")) {
-                ParseResult::Ok(out) => {
-                    assert_eq!(r_typeof(out), EXPRSXP as u32);
-
-                    let call = VECTOR_ELT(out, 0);
-                    assert_eq!(r_typeof(call), LANGSXP as u32);
-                    assert_eq!(Rf_length(call), 2);
-                    assert_eq!(CAR(call), r_symbol!("force"));
-
-                    let arg = CADR(call);
-                    assert_eq!(r_typeof(arg), REALSXP as u32);
-                    assert_eq!(*REAL(arg), 42.0);
-
-                    true
-                },
-                _ => false
+        assert_match!(
+            r_parse_vector(String::from("force(42)")),
+            ParseResult::Ok(out) => {
+                assert_eq!(r_typeof(out), EXPRSXP as u32);
+
+                let call = VECTOR_ELT(out, 0);
+                assert_eq!(r_typeof(call), LANGSXP as u32);
+                assert_eq!(Rf_length(call), 2);
+                assert_eq!(CAR(call), r_symbol!("force"));
+
+                let arg = CADR(call);
+                assert_eq!(r_typeof(arg), REALSXP as u32);
+                assert_eq!(*REAL(arg), 42.0);
             }
         );
 
         // incomplete
-        assert!(matches!(
+        assert_match!(
             r_parse_vector(String::from("force(42")),
             ParseResult::Incomplete()
-        ));
+        );
 
         // error
-        assert!(matches!(
+        assert_match!(
             r_parse_vector(String::from("42 + _")),
             ParseResult::ParseError(_)
-        ));
+        );
 
         // "normal" syntax error
-        assert!(match r_parse_vector(String::from("1+1\n*42")) {
+        assert_match!(
+            r_parse_vector(String::from("1+1\n*42")),
             ParseResult::SyntaxError {message, line} => {
                 assert!(message.contains("unexpected"));
                 assert_eq!(line, 2);
-
-                true
-            },
-            _ => false
-        });
+            }
+        );
 
     }}
 
-
-
 }
 
diff --git a/extensions/positron-r/amalthea/crates/harp/src/lib.rs b/extensions/positron-r/amalthea/crates/harp/src/lib.rs
index f911f6af02b7..77f155559a6d 100644
--- a/extensions/positron-r/amalthea/crates/harp/src/lib.rs
+++ b/extensions/positron-r/amalthea/crates/harp/src/lib.rs
@@ -67,3 +67,35 @@ macro_rules! r_lock {
     }}
 
 }
+
+/// Asserts that the given expression matches the given pattern
+/// and optionally some further assertions
+///
+/// # Examples
+///
+/// ```
+/// #[macro_use] extern crate harp;
+/// # fn main() {
+/// assert_match!(1 + 1, 2);
+/// assert_match!(1 + 1, 2 => {
+///    assert_eq!(40 + 2, 42)
+/// });
+/// # }
+/// ```
+#[macro_export]
+macro_rules! assert_match {
+
+    ($expression:expr, $pattern:pat_param => $code:block) => {
+        assert!(match $expression {
+            $pattern => {
+                $code
+                true
+            },
+            _ => false
+        })
+    };
+
+    ($expression:expr, $pattern:pat_param) => {
+        assert!(matches!($expression, $pattern))
+    };
+}

From f4555db7e3ca7accf54a8aa4d41b600d8b2d58cd Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Romain=20Fran=C3=A7ois?= <romain@posit.co>
Date: Thu, 26 Jan 2023 10:13:45 +0100
Subject: [PATCH 08/30] r_try_catch_error() handles closures that return `()`
 aka void

---
 .../amalthea/crates/harp/src/exec.rs          | 43 ++++++++++++++++---
 .../amalthea/crates/harp/src/utils.rs         |  6 +++
 2 files changed, 44 insertions(+), 5 deletions(-)

diff --git a/extensions/positron-r/amalthea/crates/harp/src/exec.rs b/extensions/positron-r/amalthea/crates/harp/src/exec.rs
index d14d6e4346da..a5eae4faf3f2 100644
--- a/extensions/positron-r/amalthea/crates/harp/src/exec.rs
+++ b/extensions/positron-r/amalthea/crates/harp/src/exec.rs
@@ -222,10 +222,37 @@ pub unsafe fn r_top_level_exec<F, R>(mut fun: F) -> Result<R> where F: FnMut() -
 
 }
 
-pub unsafe fn r_try_catch_error<F>(mut fun: F) -> std::result::Result<RObject, RError> where F: FnMut() -> SEXP {
+enum MaybeSEXP {
+    Yes(SEXP),
+    No
+}
+
+impl From<SEXP> for MaybeSEXP {
+    fn from(x: SEXP) -> Self {
+        MaybeSEXP::Yes(x)
+    }
+}
+
+impl From<()> for MaybeSEXP {
+    fn from(_x: ()) -> Self {
+        MaybeSEXP::No
+    }
+}
+
+impl MaybeSEXP {
+    fn get(&self) -> SEXP {
+        match self {
+            MaybeSEXP::Yes(x) => *x,
+            MaybeSEXP::No => unsafe {R_NilValue}
+        }
+    }
+}
+
+pub unsafe fn r_try_catch_error<F, R>(mut fun: F) -> std::result::Result<RObject, RError> where F: FnMut() -> R {
     extern fn body_fn(arg: *mut c_void) -> SEXP {
         let closure: &mut &mut dyn FnMut() -> SEXP = unsafe { mem::transmute(arg) };
-        closure()
+        let out : MaybeSEXP = closure().into();
+        out.get()
     }
 
     // handler just returns the condition and sets success to false
@@ -239,7 +266,7 @@ pub unsafe fn r_try_catch_error<F>(mut fun: F) -> std::result::Result<RObject, R
         condition
     }
 
-    let mut body_data: &mut dyn FnMut() -> SEXP = &mut fun;
+    let mut body_data: &mut dyn FnMut() -> R = &mut fun;
     let body_data = &mut body_data;
 
     let success_ptr: *mut bool = &mut success;
@@ -307,6 +334,7 @@ mod tests {
     use crate::r_lock;
     use crate::r_test;
     use crate::r_test_unlocked;
+    use crate::utils::r_is_null;
 
     use super::*;
 
@@ -430,6 +458,7 @@ mod tests {
     #[test]
     fn test_try_catch_error(){ r_test! {
 
+        // ok SEXP
         let ok = r_try_catch_error(|| {
             Rf_ScalarInteger(42)
         });
@@ -438,12 +467,16 @@ mod tests {
             assert_eq!(INTEGER_ELT(*value, 0), 42);
         });
 
+        // ok void
+        let void_ok = r_try_catch_error(|| {});
+        assert_match!(void_ok, Ok(value) => {
+            assert!(r_is_null(*value));
+        });
 
+        // error
         let out = r_try_catch_error(|| {
             let msg = CString::new("ouch").unwrap();
             Rf_error(unsafe {msg.as_ptr()});
-
-            R_NilValue
         });
 
         assert_match!(out, Err(err) => {
diff --git a/extensions/positron-r/amalthea/crates/harp/src/utils.rs b/extensions/positron-r/amalthea/crates/harp/src/utils.rs
index 3eec9dc501ad..1513f4030f27 100644
--- a/extensions/positron-r/amalthea/crates/harp/src/utils.rs
+++ b/extensions/positron-r/amalthea/crates/harp/src/utils.rs
@@ -48,6 +48,12 @@ pub unsafe fn r_assert_length(object: SEXP, expected: u32) -> Result<u32> {
     Ok(actual)
 }
 
+pub fn r_is_null(object: SEXP) -> bool {
+    unsafe {
+        Rf_isNull(object) == 1
+    }
+}
+
 pub unsafe fn r_typeof(object: SEXP) -> u32 {
     TYPEOF(object) as u32
 }

From da76478eb4b7de5d961fe91bf3a65d34634ba231 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Romain=20Fran=C3=A7ois?= <romain@posit.co>
Date: Thu, 26 Jan 2023 10:35:52 +0100
Subject: [PATCH 09/30] doc for r_try_catch_error

---
 .../amalthea/crates/harp/src/exec.rs           | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/extensions/positron-r/amalthea/crates/harp/src/exec.rs b/extensions/positron-r/amalthea/crates/harp/src/exec.rs
index a5eae4faf3f2..e516fae4c053 100644
--- a/extensions/positron-r/amalthea/crates/harp/src/exec.rs
+++ b/extensions/positron-r/amalthea/crates/harp/src/exec.rs
@@ -248,6 +248,24 @@ impl MaybeSEXP {
     }
 }
 
+/// Wrapper around R_tryCatchError
+///
+/// Takes a single closure that returns either a SEXP or `()`. If an R error is
+/// thrown this returns a an RError in the Err variant, otherwise it returns the
+/// result of the closure wrapped in an RObject.
+///
+/// Safety: the body of the closure should be as simple as possible because in the event
+///         of an R error, R will jump and there is no rust unwinding, i.e. rust values
+///         are not dropped. A good rule of thumb is to consider the body of the closure
+///         as C code.
+///
+/// ```ignore
+/// SEXP R_tryCatchError(
+///     SEXP (*body)(void *), void *bdata,
+///     SEXP (*handler)(SEXP, void *), void *hdata)
+/// ```
+///
+///
 pub unsafe fn r_try_catch_error<F, R>(mut fun: F) -> std::result::Result<RObject, RError> where F: FnMut() -> R {
     extern fn body_fn(arg: *mut c_void) -> SEXP {
         let closure: &mut &mut dyn FnMut() -> SEXP = unsafe { mem::transmute(arg) };

From bfc70f3999f7d460316cf550b6b64ddb760e0304 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Romain=20Fran=C3=A7ois?= <romain@posit.co>
Date: Thu, 26 Jan 2023 11:00:13 +0100
Subject: [PATCH 10/30] some more documentation in r_try_catch_error, and
 better handle (I think) the template parameter

---
 .../amalthea/crates/harp/src/exec.rs          | 34 +++++++++++++------
 1 file changed, 24 insertions(+), 10 deletions(-)

diff --git a/extensions/positron-r/amalthea/crates/harp/src/exec.rs b/extensions/positron-r/amalthea/crates/harp/src/exec.rs
index e516fae4c053..8e328a750eab 100644
--- a/extensions/positron-r/amalthea/crates/harp/src/exec.rs
+++ b/extensions/positron-r/amalthea/crates/harp/src/exec.rs
@@ -222,7 +222,7 @@ pub unsafe fn r_top_level_exec<F, R>(mut fun: F) -> Result<R> where F: FnMut() -
 
 }
 
-enum MaybeSEXP {
+pub enum MaybeSEXP {
     Yes(SEXP),
     No
 }
@@ -266,35 +266,49 @@ impl MaybeSEXP {
 /// ```
 ///
 ///
-pub unsafe fn r_try_catch_error<F, R>(mut fun: F) -> std::result::Result<RObject, RError> where F: FnMut() -> R {
-    extern fn body_fn(arg: *mut c_void) -> SEXP {
-        let closure: &mut &mut dyn FnMut() -> SEXP = unsafe { mem::transmute(arg) };
+pub unsafe fn r_try_catch_error<F, R>(mut fun: F) -> std::result::Result<RObject, RError> where F: FnMut() -> R, MaybeSEXP: From<R> {
+    // C function that is passed as `body`
+    // the actual closure is passed as a void* through arg
+    extern fn body_fn<S>(arg: *mut c_void) -> SEXP where MaybeSEXP: From<S>{
+        // extract the "closure" from the void*
+        let closure: &mut &mut dyn FnMut() -> S = unsafe { mem::transmute(arg) };
+
+        // call the closure and return it result as a SEXP
         let out : MaybeSEXP = closure().into();
         out.get()
     }
 
+    // The actual closure is passed as a void*
+    let mut body_data: &mut dyn FnMut() -> R = &mut fun;
+    let body_data = &mut body_data;
+
     // handler just returns the condition and sets success to false
+    // to signal that an error was caught
+    //
+    // This is similar to doing tryCatch(<C code>, error = force) in R
+    // except that we can handle the weird case where the code
+    // succeeds but returns a an error object
     let mut success: bool = true;
+    let success_ptr: *mut bool = &mut success;
+
     extern fn handler_fn(condition: SEXP, arg: *mut c_void) -> SEXP {
+        // signal that there was an error
         let success_ptr = arg as *mut bool;
         unsafe {
             *success_ptr = false;
         }
 
+        // and return the R condition as is
         condition
     }
 
-    let mut body_data: &mut dyn FnMut() -> R = &mut fun;
-    let body_data = &mut body_data;
-
-    let success_ptr: *mut bool = &mut success;
-
     let result = R_tryCatchError(
-        Some(body_fn),
+        Some(body_fn::<R>),
         body_data as *mut _ as *mut c_void,
         Some(handler_fn),
         success_ptr as *mut c_void
     );
+
     match success {
         true => Ok(RObject::from(result)),
         false => Err(RError::new(result))

From 5ff86defc8b430ebd5f7276205f38a8fc9ce6cdd Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Romain=20Fran=C3=A7ois?= <romain@posit.co>
Date: Thu, 26 Jan 2023 14:37:19 +0100
Subject: [PATCH 11/30] added the variants r_try_catch() and
 r_try_catch_finally()

---
 .../amalthea/crates/harp/src/exec.rs          | 126 ++++++++++++++++++
 1 file changed, 126 insertions(+)

diff --git a/extensions/positron-r/amalthea/crates/harp/src/exec.rs b/extensions/positron-r/amalthea/crates/harp/src/exec.rs
index 8e328a750eab..15cbda3d4c44 100644
--- a/extensions/positron-r/amalthea/crates/harp/src/exec.rs
+++ b/extensions/positron-r/amalthea/crates/harp/src/exec.rs
@@ -316,6 +316,132 @@ pub unsafe fn r_try_catch_error<F, R>(mut fun: F) -> std::result::Result<RObject
 
 }
 
+pub unsafe fn r_try_catch_finally<F, R, Finally>(mut fun: F, classes: Vec<String>, mut finally: Finally) -> std::result::Result<RObject, RError> where F: FnMut() -> R, MaybeSEXP: From<R>, Finally: FnMut() {
+    // C function that is passed as `body`
+    // the actual closure is passed as a void* through arg
+    extern fn body_fn<S>(arg: *mut c_void) -> SEXP where MaybeSEXP: From<S>{
+        // extract the "closure" from the void*
+        let closure: &mut &mut dyn FnMut() -> S = unsafe { mem::transmute(arg) };
+
+        // call the closure and return it result as a SEXP
+        let out : MaybeSEXP = closure().into();
+        out.get()
+    }
+
+    // The actual closure is passed as a void*
+    let mut body_data: &mut dyn FnMut() -> R = &mut fun;
+    let body_data = &mut body_data;
+
+    // handler just returns the condition and sets success to false
+    // to signal that an error was caught
+    //
+    // This is similar to doing tryCatch(<C code>, error = force) in R
+    // except that we can handle the weird case where the code
+    // succeeds but returns a an error object
+    let mut success: bool = true;
+    let success_ptr: *mut bool = &mut success;
+
+    extern fn handler_fn(condition: SEXP, arg: *mut c_void) -> SEXP {
+        // signal that there was an error
+        let success_ptr = arg as *mut bool;
+        unsafe {
+            *success_ptr = false;
+        }
+
+        // and return the R condition as is
+        condition
+    }
+
+    let classes : RObject = classes.into();
+
+    // C function that is passed as `finally`
+    // the actual closure is passed as a void* through arg
+    extern fn finally_fn(arg: *mut c_void) {
+        // extract the "closure" from the void*
+        let closure: &mut &mut dyn FnMut() = unsafe { mem::transmute(arg) };
+
+        closure();
+    }
+
+    // The actual finally closure is passed as a void*
+    let mut finally_data: &mut dyn FnMut() = &mut finally;
+    let finally_data = &mut finally_data;
+
+    let result = R_tryCatch(
+        Some(body_fn::<R>),
+        body_data as *mut _ as *mut c_void,
+
+        *classes,
+
+        Some(handler_fn),
+        success_ptr as *mut c_void,
+
+        Some(finally_fn),
+        finally_data as *mut _ as *mut c_void,
+    );
+
+    match success {
+        true => Ok(RObject::from(result)),
+        false => Err(RError::new(result))
+    }
+}
+
+pub unsafe fn r_try_catch<F, R>(mut fun: F, classes: Vec<String>) -> std::result::Result<RObject, RError> where F: FnMut() -> R, MaybeSEXP: From<R> {
+    // C function that is passed as `body`
+    // the actual closure is passed as a void* through arg
+    extern fn body_fn<S>(arg: *mut c_void) -> SEXP where MaybeSEXP: From<S>{
+        // extract the "closure" from the void*
+        let closure: &mut &mut dyn FnMut() -> S = unsafe { mem::transmute(arg) };
+
+        // call the closure and return it result as a SEXP
+        let out : MaybeSEXP = closure().into();
+        out.get()
+    }
+
+    // The actual closure is passed as a void*
+    let mut body_data: &mut dyn FnMut() -> R = &mut fun;
+    let body_data = &mut body_data;
+
+    // handler just returns the condition and sets success to false
+    // to signal that an error was caught
+    //
+    // This is similar to doing tryCatch(<C code>, error = force) in R
+    // except that we can handle the weird case where the code
+    // succeeds but returns a an error object
+    let mut success: bool = true;
+    let success_ptr: *mut bool = &mut success;
+
+    extern fn handler_fn(condition: SEXP, arg: *mut c_void) -> SEXP {
+        // signal that there was an error
+        let success_ptr = arg as *mut bool;
+        unsafe {
+            *success_ptr = false;
+        }
+
+        // and return the R condition as is
+        condition
+    }
+
+    let classes : RObject = classes.into();
+
+    let result = R_tryCatch(
+        Some(body_fn::<R>),
+        body_data as *mut _ as *mut c_void,
+
+        *classes,
+
+        Some(handler_fn),
+        success_ptr as *mut c_void,
+
+        None,
+        std::ptr::null_mut() as *mut c_void
+    );
+
+    match success {
+        true => Ok(RObject::from(result)),
+        false => Err(RError::new(result))
+    }
+}
 
 pub enum ParseResult {
     Ok(SEXP),

From 52af9b6243a21d7068f749c0f2715480823d58f7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Romain=20Fran=C3=A7ois?= <romain@posit.co>
Date: Thu, 9 Feb 2023 11:07:47 +0100
Subject: [PATCH 12/30] let og r_top_level_exec() for now, we can always bring
 it back

---
 .../amalthea/crates/harp/src/exec.rs          | 62 -------------------
 1 file changed, 62 deletions(-)

diff --git a/extensions/positron-r/amalthea/crates/harp/src/exec.rs b/extensions/positron-r/amalthea/crates/harp/src/exec.rs
index 15cbda3d4c44..88a80c9ad1eb 100644
--- a/extensions/positron-r/amalthea/crates/harp/src/exec.rs
+++ b/extensions/positron-r/amalthea/crates/harp/src/exec.rs
@@ -10,7 +10,6 @@ use std::mem;
 use std::os::raw::c_int;
 use std::os::raw::c_void;
 use std::os::raw::c_char;
-use std::mem::MaybeUninit;
 
 use libR_sys::*;
 
@@ -192,36 +191,6 @@ pub unsafe fn geterrmessage() -> String {
 
 }
 
-pub unsafe fn r_top_level_exec<F, R>(mut fun: F) -> Result<R> where F: FnMut() -> R {
-    // this will hold the result of calling fun() on success
-    let mut result: MaybeUninit<R> = MaybeUninit::uninit();
-
-    // wrap fun into a void closure
-    let mut void_closure: &mut dyn FnMut() = &mut || {
-        result.write(fun());
-    };
-    let void_closure = &mut void_closure;
-
-    extern fn top_level_exec_fn(arg: *mut c_void) {
-        let closure: &mut &mut dyn FnMut() = unsafe { mem::transmute(arg) };
-        closure();
-    }
-
-    let success = R_ToplevelExec(
-        Some(top_level_exec_fn),
-        void_closure as *mut _ as *mut c_void
-    );
-
-    match success != 0 {
-        false => Err(Error::TopLevelExecError()),
-
-        // there was no jump, so we can assume
-        // result has been initialized
-        true => Ok(result.assume_init())
-    }
-
-}
-
 pub enum MaybeSEXP {
     Yes(SEXP),
     No
@@ -582,37 +551,6 @@ mod tests {
 
     }}
 
-    #[test]
-    fn test_top_level_exec() { r_test! {
-        let mut ps : ParseStatus = 0;
-        let mut protect = RProtect::new();
-        let code = protect.add(crate::r_string!("force(42)"));
-
-        // successfull
-        let out = r_top_level_exec(|| {
-            R_ParseVector(code, -1, &mut ps, R_NilValue)
-        }).unwrap();
-        assert_eq!(r_typeof(out), EXPRSXP as u32);
-
-        let call = VECTOR_ELT(out, 0);
-        assert_eq!(r_typeof(call), LANGSXP as u32);
-        assert_eq!(Rf_length(call), 2);
-        assert_eq!(CAR(call), r_symbol!("force"));
-
-        let arg = CADR(call);
-        assert_eq!(r_typeof(arg), REALSXP as u32);
-        assert_eq!(*REAL(arg), 42.0);
-
-        // failed
-        let msg = CString::new("ouch").unwrap();
-        let err_msg = unsafe {msg.as_ptr()};
-        let failed = r_top_level_exec(|| {
-            Rf_error(err_msg);
-        });
-        assert!(failed.is_err());
-
-    }}
-
     #[test]
     fn test_try_catch_error(){ r_test! {
 

From d478cc860e658c02c8b4d6483c242059a666843d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Romain=20Fran=C3=A7ois?= <romain@posit.co>
Date: Thu, 9 Feb 2023 14:27:36 +0100
Subject: [PATCH 13/30] rewrite try_catch() on top of try_catch_finally

---
 .../amalthea/crates/harp/src/exec.rs          | 57 +------------------
 1 file changed, 2 insertions(+), 55 deletions(-)

diff --git a/extensions/positron-r/amalthea/crates/harp/src/exec.rs b/extensions/positron-r/amalthea/crates/harp/src/exec.rs
index 88a80c9ad1eb..41211fa65f0e 100644
--- a/extensions/positron-r/amalthea/crates/harp/src/exec.rs
+++ b/extensions/positron-r/amalthea/crates/harp/src/exec.rs
@@ -355,61 +355,8 @@ pub unsafe fn r_try_catch_finally<F, R, Finally>(mut fun: F, classes: Vec<String
     }
 }
 
-pub unsafe fn r_try_catch<F, R>(mut fun: F, classes: Vec<String>) -> std::result::Result<RObject, RError> where F: FnMut() -> R, MaybeSEXP: From<R> {
-    // C function that is passed as `body`
-    // the actual closure is passed as a void* through arg
-    extern fn body_fn<S>(arg: *mut c_void) -> SEXP where MaybeSEXP: From<S>{
-        // extract the "closure" from the void*
-        let closure: &mut &mut dyn FnMut() -> S = unsafe { mem::transmute(arg) };
-
-        // call the closure and return it result as a SEXP
-        let out : MaybeSEXP = closure().into();
-        out.get()
-    }
-
-    // The actual closure is passed as a void*
-    let mut body_data: &mut dyn FnMut() -> R = &mut fun;
-    let body_data = &mut body_data;
-
-    // handler just returns the condition and sets success to false
-    // to signal that an error was caught
-    //
-    // This is similar to doing tryCatch(<C code>, error = force) in R
-    // except that we can handle the weird case where the code
-    // succeeds but returns a an error object
-    let mut success: bool = true;
-    let success_ptr: *mut bool = &mut success;
-
-    extern fn handler_fn(condition: SEXP, arg: *mut c_void) -> SEXP {
-        // signal that there was an error
-        let success_ptr = arg as *mut bool;
-        unsafe {
-            *success_ptr = false;
-        }
-
-        // and return the R condition as is
-        condition
-    }
-
-    let classes : RObject = classes.into();
-
-    let result = R_tryCatch(
-        Some(body_fn::<R>),
-        body_data as *mut _ as *mut c_void,
-
-        *classes,
-
-        Some(handler_fn),
-        success_ptr as *mut c_void,
-
-        None,
-        std::ptr::null_mut() as *mut c_void
-    );
-
-    match success {
-        true => Ok(RObject::from(result)),
-        false => Err(RError::new(result))
-    }
+pub unsafe fn r_try_catch<F, R>(fun: F, classes: Vec<String>) -> std::result::Result<RObject, RError> where F: FnMut() -> R, MaybeSEXP: From<R> {
+    r_try_catch_finally(fun, classes, ||{} )
 }
 
 pub enum ParseResult {

From 7c162ff20c528efaeae6d156f1f53e8e8057d902 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Romain=20Fran=C3=A7ois?= <romain@posit.co>
Date: Thu, 9 Feb 2023 15:38:10 +0100
Subject: [PATCH 14/30] + trait ToRStrings

---
 .../amalthea/crates/harp/src/exec.rs          | 77 +++++--------------
 .../amalthea/crates/harp/src/object.rs        | 70 ++++++++++++++++-
 2 files changed, 85 insertions(+), 62 deletions(-)

diff --git a/extensions/positron-r/amalthea/crates/harp/src/exec.rs b/extensions/positron-r/amalthea/crates/harp/src/exec.rs
index 41211fa65f0e..4d8843aa42ec 100644
--- a/extensions/positron-r/amalthea/crates/harp/src/exec.rs
+++ b/extensions/positron-r/amalthea/crates/harp/src/exec.rs
@@ -17,6 +17,8 @@ use crate::error::Error;
 use crate::error::RError;
 use crate::error::Result;
 use crate::object::RObject;
+use crate::object::ToRStrings;
+use crate::object::r_strings;
 use crate::protect::RProtect;
 use crate::r_symbol;
 use crate::utils::r_inherits;
@@ -217,28 +219,31 @@ impl MaybeSEXP {
     }
 }
 
-/// Wrapper around R_tryCatchError
+/// Wrappers around R_tryCatch()
 ///
 /// Takes a single closure that returns either a SEXP or `()`. If an R error is
 /// thrown this returns a an RError in the Err variant, otherwise it returns the
 /// result of the closure wrapped in an RObject.
 ///
+/// The handler closure is not used per se, we just get the condition verbatim in the Err variant
+///
 /// Safety: the body of the closure should be as simple as possible because in the event
 ///         of an R error, R will jump and there is no rust unwinding, i.e. rust values
 ///         are not dropped. A good rule of thumb is to consider the body of the closure
 ///         as C code.
 ///
 /// ```ignore
-/// SEXP R_tryCatchError(
+/// SEXP R_tryCatch(
 ///     SEXP (*body)(void *), void *bdata,
-///     SEXP (*handler)(SEXP, void *), void *hdata)
+///     SEXP conds,
+///     SEXP (*handler)(SEXP, void *), void *hdata),
+///     void (*finally)(void*), void* fdata
+/// )
 /// ```
-///
-///
-pub unsafe fn r_try_catch_error<F, R>(mut fun: F) -> std::result::Result<RObject, RError> where F: FnMut() -> R, MaybeSEXP: From<R> {
+pub unsafe fn r_try_catch_finally<F, R, S, Finally>(mut fun: F, classes: S, mut finally: Finally) -> std::result::Result<RObject, RError> where F: FnMut() -> R, MaybeSEXP: From<R>, Finally: FnMut(), S: ToRStrings {
     // C function that is passed as `body`
     // the actual closure is passed as a void* through arg
-    extern fn body_fn<S>(arg: *mut c_void) -> SEXP where MaybeSEXP: From<S>{
+    extern fn body_fn<S>(arg: *mut c_void) -> SEXP where MaybeSEXP: From<S> {
         // extract the "closure" from the void*
         let closure: &mut &mut dyn FnMut() -> S = unsafe { mem::transmute(arg) };
 
@@ -271,57 +276,7 @@ pub unsafe fn r_try_catch_error<F, R>(mut fun: F) -> std::result::Result<RObject
         condition
     }
 
-    let result = R_tryCatchError(
-        Some(body_fn::<R>),
-        body_data as *mut _ as *mut c_void,
-        Some(handler_fn),
-        success_ptr as *mut c_void
-    );
-
-    match success {
-        true => Ok(RObject::from(result)),
-        false => Err(RError::new(result))
-    }
-
-}
-
-pub unsafe fn r_try_catch_finally<F, R, Finally>(mut fun: F, classes: Vec<String>, mut finally: Finally) -> std::result::Result<RObject, RError> where F: FnMut() -> R, MaybeSEXP: From<R>, Finally: FnMut() {
-    // C function that is passed as `body`
-    // the actual closure is passed as a void* through arg
-    extern fn body_fn<S>(arg: *mut c_void) -> SEXP where MaybeSEXP: From<S>{
-        // extract the "closure" from the void*
-        let closure: &mut &mut dyn FnMut() -> S = unsafe { mem::transmute(arg) };
-
-        // call the closure and return it result as a SEXP
-        let out : MaybeSEXP = closure().into();
-        out.get()
-    }
-
-    // The actual closure is passed as a void*
-    let mut body_data: &mut dyn FnMut() -> R = &mut fun;
-    let body_data = &mut body_data;
-
-    // handler just returns the condition and sets success to false
-    // to signal that an error was caught
-    //
-    // This is similar to doing tryCatch(<C code>, error = force) in R
-    // except that we can handle the weird case where the code
-    // succeeds but returns a an error object
-    let mut success: bool = true;
-    let success_ptr: *mut bool = &mut success;
-
-    extern fn handler_fn(condition: SEXP, arg: *mut c_void) -> SEXP {
-        // signal that there was an error
-        let success_ptr = arg as *mut bool;
-        unsafe {
-            *success_ptr = false;
-        }
-
-        // and return the R condition as is
-        condition
-    }
-
-    let classes : RObject = classes.into();
+    let classes = r_strings(classes);
 
     // C function that is passed as `finally`
     // the actual closure is passed as a void* through arg
@@ -355,10 +310,14 @@ pub unsafe fn r_try_catch_finally<F, R, Finally>(mut fun: F, classes: Vec<String
     }
 }
 
-pub unsafe fn r_try_catch<F, R>(fun: F, classes: Vec<String>) -> std::result::Result<RObject, RError> where F: FnMut() -> R, MaybeSEXP: From<R> {
+pub unsafe fn r_try_catch<F, R, S>(fun: F, classes: S) -> std::result::Result<RObject, RError> where F: FnMut() -> R, MaybeSEXP: From<R>, S : ToRStrings {
     r_try_catch_finally(fun, classes, ||{} )
 }
 
+pub unsafe fn r_try_catch_error<F, R>(fun: F) -> std::result::Result<RObject, RError> where F: FnMut() -> R, MaybeSEXP: From<R> {
+    r_try_catch_finally(fun, &["error"], ||{} )
+}
+
 pub enum ParseResult {
     Ok(SEXP),
     Incomplete(),
diff --git a/extensions/positron-r/amalthea/crates/harp/src/object.rs b/extensions/positron-r/amalthea/crates/harp/src/object.rs
index 60b7d12cb079..588f9897d013 100644
--- a/extensions/positron-r/amalthea/crates/harp/src/object.rs
+++ b/extensions/positron-r/amalthea/crates/harp/src/object.rs
@@ -272,6 +272,32 @@ impl<S> From<Vec<S>> for RObject where S : ToCharSxp {
     }
 }
 
+pub trait ToRStrings {
+    fn to_r_strings(self) -> RObject;
+}
+
+impl<S: ToCharSxp> ToRStrings for &[S] {
+    fn to_r_strings(self) -> RObject {
+        self.into()
+    }
+}
+
+impl<S: ToCharSxp, const N: usize> ToRStrings for &[S; N] {
+    fn to_r_strings(self) -> RObject {
+        self.into()
+    }
+}
+
+impl<S: ToCharSxp> ToRStrings for Vec<S> {
+    fn to_r_strings(self) -> RObject {
+        self.into()
+    }
+}
+
+pub fn r_strings<S: ToRStrings>(strings: S) -> RObject {
+    strings.to_r_strings()
+}
+
 /// Convert RObject into other types.
 
 impl From<RObject> for SEXP {
@@ -391,11 +417,11 @@ impl TryFrom<RObject> for HashMap<String, String> {
 
 #[cfg(test)]
 mod tests {
-    use libR_sys::{STRING_ELT, R_NaString};
+    use libR_sys::{STRING_ELT, R_NaString, STRSXP};
 
-    use crate::{r_test, r_string, protect, utils::CharSxpEq};
+    use crate::{r_test, r_string, protect, utils::{CharSxpEq, r_typeof}};
 
-    use super::RObject;
+    use super::{RObject, r_strings};
 
     #[test]
     #[allow(non_snake_case)]
@@ -458,4 +484,42 @@ mod tests {
         assert_eq!(r_strings, expected);            // [String; const N]
         assert_eq!(r_strings, expected.to_vec());   // Vec<String>
     }}
+
+    #[test]
+    fn test_r_strings() { r_test! {
+        let alphabet = ["a", "b", "c"];
+
+        // &[&str]
+        let s = r_strings(&alphabet);
+        assert_eq!(r_typeof(s.sexp), STRSXP);
+        assert_eq!(s, alphabet);
+
+        // &[&str; N]
+        let s = r_strings(&alphabet[..]);
+        assert_eq!(r_typeof(s.sexp), STRSXP);
+        assert_eq!(s, alphabet);
+
+        // Vec<&str>
+        let s = r_strings(alphabet.to_vec());
+        assert_eq!(r_typeof(s.sexp), STRSXP);
+        assert_eq!(s, alphabet);
+
+        // &[String]
+        let alphabet = alphabet.map(|s| { String::from(s) });
+        let s = r_strings(&alphabet);
+        assert_eq!(r_typeof(s.sexp), STRSXP);
+        assert_eq!(s, alphabet);
+
+        // &[String; N]
+        let s = r_strings(&alphabet[..]);
+        assert_eq!(r_typeof(s.sexp), STRSXP);
+        assert_eq!(s, alphabet);
+
+        // Vec<String>
+        let s = r_strings(alphabet.to_vec());
+        assert_eq!(r_typeof(s.sexp), STRSXP);
+        assert_eq!(s, alphabet);
+
+    }}
+
 }

From e7ce82aed40eb73bd83b3933e9f747e29f26f01c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Romain=20Fran=C3=A7ois?= <romain@posit.co>
Date: Thu, 9 Feb 2023 15:48:12 +0100
Subject: [PATCH 15/30] r_string() for &str and String

---
 .../amalthea/crates/harp/src/exec.rs           |  2 +-
 .../amalthea/crates/harp/src/object.rs         | 18 ++++++++++++++++++
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/extensions/positron-r/amalthea/crates/harp/src/exec.rs b/extensions/positron-r/amalthea/crates/harp/src/exec.rs
index 4d8843aa42ec..92839042da71 100644
--- a/extensions/positron-r/amalthea/crates/harp/src/exec.rs
+++ b/extensions/positron-r/amalthea/crates/harp/src/exec.rs
@@ -315,7 +315,7 @@ pub unsafe fn r_try_catch<F, R, S>(fun: F, classes: S) -> std::result::Result<RO
 }
 
 pub unsafe fn r_try_catch_error<F, R>(fun: F) -> std::result::Result<RObject, RError> where F: FnMut() -> R, MaybeSEXP: From<R> {
-    r_try_catch_finally(fun, &["error"], ||{} )
+    r_try_catch_finally(fun, "error", ||{} )
 }
 
 pub enum ParseResult {
diff --git a/extensions/positron-r/amalthea/crates/harp/src/object.rs b/extensions/positron-r/amalthea/crates/harp/src/object.rs
index 588f9897d013..58ea453986a2 100644
--- a/extensions/positron-r/amalthea/crates/harp/src/object.rs
+++ b/extensions/positron-r/amalthea/crates/harp/src/object.rs
@@ -294,6 +294,12 @@ impl<S: ToCharSxp> ToRStrings for Vec<S> {
     }
 }
 
+impl<S: ToCharSxp> ToRStrings for S {
+    fn to_r_strings(self) -> RObject {
+        [self].to_r_strings()
+    }
+}
+
 pub fn r_strings<S: ToRStrings>(strings: S) -> RObject {
     strings.to_r_strings()
 }
@@ -520,6 +526,18 @@ mod tests {
         assert_eq!(r_typeof(s.sexp), STRSXP);
         assert_eq!(s, alphabet);
 
+        // &str
+        let string = "Banana";
+        let s = r_strings(string);
+        assert_eq!(r_typeof(s.sexp), STRSXP);
+        assert_eq!(s, [string]);
+
+        // String
+        let string = String::from("Pineapple");
+        let s = r_strings(string);
+        assert_eq!(r_typeof(s.sexp), STRSXP);
+        assert_eq!(s, ["Pineapple"]); // string was moved
+
     }}
 
 }

From ccd9d944f4aeed7abba37f159bc90e11170ac3b2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Romain=20Fran=C3=A7ois?= <romain@posit.co>
Date: Thu, 9 Feb 2023 16:09:38 +0100
Subject: [PATCH 16/30] implement PartialEq(RObject == single string)

---
 .../positron-r/amalthea/crates/harp/src/object.rs  |  4 ++--
 .../positron-r/amalthea/crates/harp/src/utils.rs   | 14 ++++++++++++++
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/extensions/positron-r/amalthea/crates/harp/src/object.rs b/extensions/positron-r/amalthea/crates/harp/src/object.rs
index 58ea453986a2..99af00ef07d8 100644
--- a/extensions/positron-r/amalthea/crates/harp/src/object.rs
+++ b/extensions/positron-r/amalthea/crates/harp/src/object.rs
@@ -530,13 +530,13 @@ mod tests {
         let string = "Banana";
         let s = r_strings(string);
         assert_eq!(r_typeof(s.sexp), STRSXP);
-        assert_eq!(s, [string]);
+        assert_eq!(s, string);
 
         // String
         let string = String::from("Pineapple");
         let s = r_strings(string);
         assert_eq!(r_typeof(s.sexp), STRSXP);
-        assert_eq!(s, ["Pineapple"]); // string was moved
+        assert_eq!(s, "Pineapple"); // string was moved
 
     }}
 
diff --git a/extensions/positron-r/amalthea/crates/harp/src/utils.rs b/extensions/positron-r/amalthea/crates/harp/src/utils.rs
index 3d4882d9e7dc..7088a18ba14e 100644
--- a/extensions/positron-r/amalthea/crates/harp/src/utils.rs
+++ b/extensions/positron-r/amalthea/crates/harp/src/utils.rs
@@ -100,6 +100,20 @@ impl<S> PartialEq<Vec<S>> for RObject where S : CharSxpEq {
     }
 }
 
+impl <S> PartialEq<S> for RObject where S : CharSxpEq {
+    fn eq(&self, other: &S) -> bool {
+        unsafe {
+            let sexp = self.sexp;
+            if Rf_length(sexp) != 1 {
+                return false;
+            }
+
+            other.eq_charsxp(STRING_ELT(sexp, 0))
+        }
+
+    }
+}
+
 pub fn r_is_null(object: SEXP) -> bool {
     unsafe {
         Rf_isNull(object) == 1

From e56c117855e889045e68d72806f2048621edd311 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Romain=20Fran=C3=A7ois?= <romain@posit.co>
Date: Fri, 10 Feb 2023 10:12:22 +0100
Subject: [PATCH 17/30] Update
 extensions/positron-r/amalthea/crates/harp/src/exec.rs

Co-authored-by: Kevin Ushey <kevin@posit.co>
---
 extensions/positron-r/amalthea/crates/harp/src/exec.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/extensions/positron-r/amalthea/crates/harp/src/exec.rs b/extensions/positron-r/amalthea/crates/harp/src/exec.rs
index 92839042da71..163ec450233b 100644
--- a/extensions/positron-r/amalthea/crates/harp/src/exec.rs
+++ b/extensions/positron-r/amalthea/crates/harp/src/exec.rs
@@ -311,7 +311,7 @@ pub unsafe fn r_try_catch_finally<F, R, S, Finally>(mut fun: F, classes: S, mut
 }
 
 pub unsafe fn r_try_catch<F, R, S>(fun: F, classes: S) -> std::result::Result<RObject, RError> where F: FnMut() -> R, MaybeSEXP: From<R>, S : ToRStrings {
-    r_try_catch_finally(fun, classes, ||{} )
+    r_try_catch_finally(fun, classes, || {})
 }
 
 pub unsafe fn r_try_catch_error<F, R>(fun: F) -> std::result::Result<RObject, RError> where F: FnMut() -> R, MaybeSEXP: From<R> {

From 0f7cadcdd8838944f751ecaebb3db34b9bdcee7f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Romain=20Fran=C3=A7ois?= <romain@posit.co>
Date: Fri, 10 Feb 2023 10:12:33 +0100
Subject: [PATCH 18/30] Update
 extensions/positron-r/amalthea/crates/harp/src/exec.rs

Co-authored-by: Kevin Ushey <kevin@posit.co>
---
 extensions/positron-r/amalthea/crates/harp/src/exec.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/extensions/positron-r/amalthea/crates/harp/src/exec.rs b/extensions/positron-r/amalthea/crates/harp/src/exec.rs
index 163ec450233b..e26067ea35e6 100644
--- a/extensions/positron-r/amalthea/crates/harp/src/exec.rs
+++ b/extensions/positron-r/amalthea/crates/harp/src/exec.rs
@@ -315,7 +315,7 @@ pub unsafe fn r_try_catch<F, R, S>(fun: F, classes: S) -> std::result::Result<RO
 }
 
 pub unsafe fn r_try_catch_error<F, R>(fun: F) -> std::result::Result<RObject, RError> where F: FnMut() -> R, MaybeSEXP: From<R> {
-    r_try_catch_finally(fun, "error", ||{} )
+    r_try_catch_finally(fun, "error", || {})
 }
 
 pub enum ParseResult {

From c27b2daddf0fd71817987deb43a64d0f45564560 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Romain=20Fran=C3=A7ois?= <romain@posit.co>
Date: Fri, 10 Feb 2023 10:14:03 +0100
Subject: [PATCH 19/30] use libR_sys::*

---
 extensions/positron-r/amalthea/crates/harp/src/error.rs  | 2 +-
 extensions/positron-r/amalthea/crates/harp/src/object.rs | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/extensions/positron-r/amalthea/crates/harp/src/error.rs b/extensions/positron-r/amalthea/crates/harp/src/error.rs
index 4cf731d2c813..9a44ffc817ea 100644
--- a/extensions/positron-r/amalthea/crates/harp/src/error.rs
+++ b/extensions/positron-r/amalthea/crates/harp/src/error.rs
@@ -8,7 +8,7 @@
 use std::fmt;
 use std::str::Utf8Error;
 
-use libR_sys::{Rf_getAttrib, R_ClassSymbol, SEXP, Rf_lang2, Rf_eval, R_BaseEnv};
+use libR_sys::*;
 
 use crate::object::RObject;
 use crate::protect::RProtect;
diff --git a/extensions/positron-r/amalthea/crates/harp/src/object.rs b/extensions/positron-r/amalthea/crates/harp/src/object.rs
index 99af00ef07d8..50dcf61d9890 100644
--- a/extensions/positron-r/amalthea/crates/harp/src/object.rs
+++ b/extensions/positron-r/amalthea/crates/harp/src/object.rs
@@ -423,7 +423,7 @@ impl TryFrom<RObject> for HashMap<String, String> {
 
 #[cfg(test)]
 mod tests {
-    use libR_sys::{STRING_ELT, R_NaString, STRSXP};
+    use libR_sys::*;
 
     use crate::{r_test, r_string, protect, utils::{CharSxpEq, r_typeof}};
 

From ca8811d4ffc44a6d69b50b30ca096b87fc21097b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Romain=20Fran=C3=A7ois?= <romain@posit.co>
Date: Fri, 10 Feb 2023 10:30:20 +0100
Subject: [PATCH 20/30] where

---
 .../amalthea/crates/harp/src/exec.rs          | 26 ++++++++++++++++---
 .../amalthea/crates/harp/src/object.rs        |  6 ++---
 .../amalthea/crates/harp/src/utils.rs         |  8 +++---
 3 files changed, 29 insertions(+), 11 deletions(-)

diff --git a/extensions/positron-r/amalthea/crates/harp/src/exec.rs b/extensions/positron-r/amalthea/crates/harp/src/exec.rs
index e26067ea35e6..7aa74f22e4f0 100644
--- a/extensions/positron-r/amalthea/crates/harp/src/exec.rs
+++ b/extensions/positron-r/amalthea/crates/harp/src/exec.rs
@@ -240,10 +240,19 @@ impl MaybeSEXP {
 ///     void (*finally)(void*), void* fdata
 /// )
 /// ```
-pub unsafe fn r_try_catch_finally<F, R, S, Finally>(mut fun: F, classes: S, mut finally: Finally) -> std::result::Result<RObject, RError> where F: FnMut() -> R, MaybeSEXP: From<R>, Finally: FnMut(), S: ToRStrings {
+pub unsafe fn r_try_catch_finally<F, R, S, Finally>(mut fun: F, classes: S, mut finally: Finally) -> std::result::Result<RObject, RError>
+where
+    F: FnMut() -> R,
+    MaybeSEXP: From<R>,
+    Finally: FnMut(),
+    S: ToRStrings
+{
     // C function that is passed as `body`
     // the actual closure is passed as a void* through arg
-    extern fn body_fn<S>(arg: *mut c_void) -> SEXP where MaybeSEXP: From<S> {
+    extern fn body_fn<S>(arg: *mut c_void) -> SEXP
+    where
+        MaybeSEXP: From<S>
+    {
         // extract the "closure" from the void*
         let closure: &mut &mut dyn FnMut() -> S = unsafe { mem::transmute(arg) };
 
@@ -310,11 +319,20 @@ pub unsafe fn r_try_catch_finally<F, R, S, Finally>(mut fun: F, classes: S, mut
     }
 }
 
-pub unsafe fn r_try_catch<F, R, S>(fun: F, classes: S) -> std::result::Result<RObject, RError> where F: FnMut() -> R, MaybeSEXP: From<R>, S : ToRStrings {
+pub unsafe fn r_try_catch<F, R, S>(fun: F, classes: S) -> std::result::Result<RObject, RError>
+where
+    F: FnMut() -> R,
+    MaybeSEXP: From<R>,
+    S : ToRStrings
+{
     r_try_catch_finally(fun, classes, || {})
 }
 
-pub unsafe fn r_try_catch_error<F, R>(fun: F) -> std::result::Result<RObject, RError> where F: FnMut() -> R, MaybeSEXP: From<R> {
+pub unsafe fn r_try_catch_error<F, R>(fun: F) -> std::result::Result<RObject, RError>
+where
+    F: FnMut() -> R,
+    MaybeSEXP: From<R>
+{
     r_try_catch_finally(fun, "error", || {})
 }
 
diff --git a/extensions/positron-r/amalthea/crates/harp/src/object.rs b/extensions/positron-r/amalthea/crates/harp/src/object.rs
index 50dcf61d9890..0fb3e87b4df6 100644
--- a/extensions/positron-r/amalthea/crates/harp/src/object.rs
+++ b/extensions/positron-r/amalthea/crates/harp/src/object.rs
@@ -245,7 +245,7 @@ impl ToCharSxp for String {
     }
 }
 
-impl<S> From<&[S]> for RObject where S : ToCharSxp {
+impl<S: ToCharSxp> From<&[S]> for RObject {
     fn from(value: &[S]) -> Self {
         unsafe {
             let n = value.len() as isize;
@@ -260,13 +260,13 @@ impl<S> From<&[S]> for RObject where S : ToCharSxp {
     }
 }
 
-impl<S, const N: usize> From<&[S; N]> for RObject where S : ToCharSxp {
+impl<S: ToCharSxp, const N: usize> From<&[S; N]> for RObject {
     fn from(value: &[S; N]) -> Self {
         RObject::from(&value[..])
     }
 }
 
-impl<S> From<Vec<S>> for RObject where S : ToCharSxp {
+impl<S: ToCharSxp> From<Vec<S>> for RObject {
     fn from(value: Vec<S>) -> Self {
         RObject::from(&value[..])
     }
diff --git a/extensions/positron-r/amalthea/crates/harp/src/utils.rs b/extensions/positron-r/amalthea/crates/harp/src/utils.rs
index 7088a18ba14e..f0cf6fa9368e 100644
--- a/extensions/positron-r/amalthea/crates/harp/src/utils.rs
+++ b/extensions/positron-r/amalthea/crates/harp/src/utils.rs
@@ -66,7 +66,7 @@ impl CharSxpEq for String {
     }
 }
 
-impl<S> PartialEq<[S]> for RObject where S : CharSxpEq {
+impl<S: CharSxpEq> PartialEq<[S]> for RObject {
     fn eq(&self, other: &[S]) -> bool {
         unsafe {
             let object = self.sexp;
@@ -88,19 +88,19 @@ impl<S> PartialEq<[S]> for RObject where S : CharSxpEq {
     }
 }
 
-impl<S, const N: usize> PartialEq<[S; N]> for RObject where S : CharSxpEq {
+impl<S: CharSxpEq, const N: usize> PartialEq<[S; N]> for RObject {
     fn eq(&self, other: &[S; N]) -> bool {
         self.eq(&other[..])
     }
 }
 
-impl<S> PartialEq<Vec<S>> for RObject where S : CharSxpEq {
+impl<S: CharSxpEq> PartialEq<Vec<S>> for RObject {
     fn eq(&self, other: &Vec<S>) -> bool {
         self.eq(&other[..])
     }
 }
 
-impl <S> PartialEq<S> for RObject where S : CharSxpEq {
+impl <S: CharSxpEq> PartialEq<S> for RObject {
     fn eq(&self, other: &S) -> bool {
         unsafe {
             let sexp = self.sexp;

From 1aa4547d17b81938122d665cbd3e3781dc5088ae Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Romain=20Fran=C3=A7ois?= <romain@posit.co>
Date: Fri, 10 Feb 2023 10:33:39 +0100
Subject: [PATCH 21/30] extern "C"

---
 extensions/positron-r/amalthea/crates/harp/src/exec.rs | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/extensions/positron-r/amalthea/crates/harp/src/exec.rs b/extensions/positron-r/amalthea/crates/harp/src/exec.rs
index 7aa74f22e4f0..233338c345cf 100644
--- a/extensions/positron-r/amalthea/crates/harp/src/exec.rs
+++ b/extensions/positron-r/amalthea/crates/harp/src/exec.rs
@@ -249,7 +249,7 @@ where
 {
     // C function that is passed as `body`
     // the actual closure is passed as a void* through arg
-    extern fn body_fn<S>(arg: *mut c_void) -> SEXP
+    extern "C" fn body_fn<S>(arg: *mut c_void) -> SEXP
     where
         MaybeSEXP: From<S>
     {
@@ -274,7 +274,7 @@ where
     let mut success: bool = true;
     let success_ptr: *mut bool = &mut success;
 
-    extern fn handler_fn(condition: SEXP, arg: *mut c_void) -> SEXP {
+    extern "C" fn handler_fn(condition: SEXP, arg: *mut c_void) -> SEXP {
         // signal that there was an error
         let success_ptr = arg as *mut bool;
         unsafe {
@@ -289,7 +289,7 @@ where
 
     // C function that is passed as `finally`
     // the actual closure is passed as a void* through arg
-    extern fn finally_fn(arg: *mut c_void) {
+    extern "C" fn finally_fn(arg: *mut c_void) {
         // extract the "closure" from the void*
         let closure: &mut &mut dyn FnMut() = unsafe { mem::transmute(arg) };
 

From 4ce42dcd5b4cf43c7e09d2e2c129d28eb89407cf Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Romain=20Fran=C3=A7ois?= <romain@posit.co>
Date: Fri, 10 Feb 2023 10:36:25 +0100
Subject: [PATCH 22/30] + article

---
 extensions/positron-r/amalthea/crates/harp/src/exec.rs | 1 +
 1 file changed, 1 insertion(+)

diff --git a/extensions/positron-r/amalthea/crates/harp/src/exec.rs b/extensions/positron-r/amalthea/crates/harp/src/exec.rs
index 233338c345cf..23e61f06d2bf 100644
--- a/extensions/positron-r/amalthea/crates/harp/src/exec.rs
+++ b/extensions/positron-r/amalthea/crates/harp/src/exec.rs
@@ -254,6 +254,7 @@ where
         MaybeSEXP: From<S>
     {
         // extract the "closure" from the void*
+        // idea from https://adventures.michaelfbryan.com/posts/rust-closures-in-ffi/
         let closure: &mut &mut dyn FnMut() -> S = unsafe { mem::transmute(arg) };
 
         // call the closure and return it result as a SEXP

From 0ee77f31c4d71303ffbec3306257876bf032e617 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Romain=20Fran=C3=A7ois?= <romain@posit.co>
Date: Fri, 10 Feb 2023 10:51:19 +0100
Subject: [PATCH 23/30] remove MaybeSEXP

---
 .../amalthea/crates/harp/src/exec.rs          | 38 +++----------------
 .../amalthea/crates/harp/src/object.rs        |  8 ++++
 2 files changed, 14 insertions(+), 32 deletions(-)

diff --git a/extensions/positron-r/amalthea/crates/harp/src/exec.rs b/extensions/positron-r/amalthea/crates/harp/src/exec.rs
index 23e61f06d2bf..d385066d2173 100644
--- a/extensions/positron-r/amalthea/crates/harp/src/exec.rs
+++ b/extensions/positron-r/amalthea/crates/harp/src/exec.rs
@@ -193,32 +193,6 @@ pub unsafe fn geterrmessage() -> String {
 
 }
 
-pub enum MaybeSEXP {
-    Yes(SEXP),
-    No
-}
-
-impl From<SEXP> for MaybeSEXP {
-    fn from(x: SEXP) -> Self {
-        MaybeSEXP::Yes(x)
-    }
-}
-
-impl From<()> for MaybeSEXP {
-    fn from(_x: ()) -> Self {
-        MaybeSEXP::No
-    }
-}
-
-impl MaybeSEXP {
-    fn get(&self) -> SEXP {
-        match self {
-            MaybeSEXP::Yes(x) => *x,
-            MaybeSEXP::No => unsafe {R_NilValue}
-        }
-    }
-}
-
 /// Wrappers around R_tryCatch()
 ///
 /// Takes a single closure that returns either a SEXP or `()`. If an R error is
@@ -243,7 +217,7 @@ impl MaybeSEXP {
 pub unsafe fn r_try_catch_finally<F, R, S, Finally>(mut fun: F, classes: S, mut finally: Finally) -> std::result::Result<RObject, RError>
 where
     F: FnMut() -> R,
-    MaybeSEXP: From<R>,
+    R: Into<RObject>,
     Finally: FnMut(),
     S: ToRStrings
 {
@@ -251,15 +225,15 @@ where
     // the actual closure is passed as a void* through arg
     extern "C" fn body_fn<S>(arg: *mut c_void) -> SEXP
     where
-        MaybeSEXP: From<S>
+        S: Into<RObject>
     {
         // extract the "closure" from the void*
         // idea from https://adventures.michaelfbryan.com/posts/rust-closures-in-ffi/
         let closure: &mut &mut dyn FnMut() -> S = unsafe { mem::transmute(arg) };
 
         // call the closure and return it result as a SEXP
-        let out : MaybeSEXP = closure().into();
-        out.get()
+        let out : RObject = closure().into();
+        out.sexp
     }
 
     // The actual closure is passed as a void*
@@ -323,7 +297,7 @@ where
 pub unsafe fn r_try_catch<F, R, S>(fun: F, classes: S) -> std::result::Result<RObject, RError>
 where
     F: FnMut() -> R,
-    MaybeSEXP: From<R>,
+    RObject: From<R>,
     S : ToRStrings
 {
     r_try_catch_finally(fun, classes, || {})
@@ -332,7 +306,7 @@ where
 pub unsafe fn r_try_catch_error<F, R>(fun: F) -> std::result::Result<RObject, RError>
 where
     F: FnMut() -> R,
-    MaybeSEXP: From<R>
+    RObject: From<R>
 {
     r_try_catch_finally(fun, "error", || {})
 }
diff --git a/extensions/positron-r/amalthea/crates/harp/src/object.rs b/extensions/positron-r/amalthea/crates/harp/src/object.rs
index 0fb3e87b4df6..7603b5a21551 100644
--- a/extensions/positron-r/amalthea/crates/harp/src/object.rs
+++ b/extensions/positron-r/amalthea/crates/harp/src/object.rs
@@ -170,6 +170,14 @@ impl From<SEXP> for RObject {
     }
 }
 
+impl From<()> for RObject {
+    fn from(_value: ()) -> Self {
+        unsafe {
+            RObject::from(R_NilValue)
+        }
+    }
+}
+
 impl From<bool> for RObject {
     fn from(value: bool) -> Self {
         unsafe {

From b78f4b6a57cb9bae22b89cdddd3f6a6d2efa7f09 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Romain=20Fran=C3=A7ois?= <romain@posit.co>
Date: Fri, 10 Feb 2023 10:59:51 +0100
Subject: [PATCH 24/30] + test r_try_catch_error() returning Vec<&str>

---
 extensions/positron-r/amalthea/crates/harp/src/exec.rs | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/extensions/positron-r/amalthea/crates/harp/src/exec.rs b/extensions/positron-r/amalthea/crates/harp/src/exec.rs
index d385066d2173..02ac0018042f 100644
--- a/extensions/positron-r/amalthea/crates/harp/src/exec.rs
+++ b/extensions/positron-r/amalthea/crates/harp/src/exec.rs
@@ -468,6 +468,15 @@ mod tests {
             assert!(r_is_null(*value));
         });
 
+        // ok something else, Vec<&str>
+        let string_ok = r_try_catch_error(|| {
+            vec!["hello", "world"]
+        });
+        assert_match!(string_ok, Ok(value) => {
+            assert_eq!(r_typeof(value.sexp), STRSXP);
+            assert_eq!(value, ["hello", "world"]);
+        });
+
         // error
         let out = r_try_catch_error(|| {
             let msg = CString::new("ouch").unwrap();

From 3d4e7af8edb37a555619d79ccdaa5eda60b35775 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Romain=20Fran=C3=A7ois?= <romain@posit.co>
Date: Fri, 10 Feb 2023 11:01:04 +0100
Subject: [PATCH 25/30] use super::*;

---
 extensions/positron-r/amalthea/crates/harp/src/object.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/extensions/positron-r/amalthea/crates/harp/src/object.rs b/extensions/positron-r/amalthea/crates/harp/src/object.rs
index 7603b5a21551..0b58836a5c1b 100644
--- a/extensions/positron-r/amalthea/crates/harp/src/object.rs
+++ b/extensions/positron-r/amalthea/crates/harp/src/object.rs
@@ -435,7 +435,7 @@ mod tests {
 
     use crate::{r_test, r_string, protect, utils::{CharSxpEq, r_typeof}};
 
-    use super::{RObject, r_strings};
+    use super::*;
 
     #[test]
     #[allow(non_snake_case)]

From bd66ff5b1b0d35dd2212dba726284cd06ff65e3d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Romain=20Fran=C3=A7ois?= <romain@posit.co>
Date: Fri, 10 Feb 2023 11:07:05 +0100
Subject: [PATCH 26/30] pub fn classes(&self) -> Result<Vec<String>>

---
 extensions/positron-r/amalthea/crates/harp/src/error.rs | 8 ++++----
 extensions/positron-r/amalthea/crates/harp/src/exec.rs  | 8 ++++++--
 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/extensions/positron-r/amalthea/crates/harp/src/error.rs b/extensions/positron-r/amalthea/crates/harp/src/error.rs
index 9a44ffc817ea..e507b58c7c5e 100644
--- a/extensions/positron-r/amalthea/crates/harp/src/error.rs
+++ b/extensions/positron-r/amalthea/crates/harp/src/error.rs
@@ -40,18 +40,18 @@ impl RError {
         *self.0
     }
 
-    pub fn classes(&self) -> Vec<String>  {
+    pub fn classes(&self) -> Result<Vec<String>>  {
         unsafe {
-            RObject::from(Rf_getAttrib(*self.0, R_ClassSymbol)).try_into().unwrap()
+            RObject::from(Rf_getAttrib(*self.0, R_ClassSymbol)).try_into()
         }
     }
 
-    pub fn message(&self) -> Vec<String> {
+    pub fn message(&self) -> Result<Vec<String>> {
         unsafe {
             let mut protect = RProtect::new();
             let call = protect.add(Rf_lang2(r_symbol!("conditionMessage"), *self.0));
 
-            RObject::from(Rf_eval(call, R_BaseEnv)).try_into().unwrap()
+            RObject::from(Rf_eval(call, R_BaseEnv)).try_into()
         }
     }
 
diff --git a/extensions/positron-r/amalthea/crates/harp/src/exec.rs b/extensions/positron-r/amalthea/crates/harp/src/exec.rs
index 02ac0018042f..02def3ecd41b 100644
--- a/extensions/positron-r/amalthea/crates/harp/src/exec.rs
+++ b/extensions/positron-r/amalthea/crates/harp/src/exec.rs
@@ -484,8 +484,12 @@ mod tests {
         });
 
         assert_match!(out, Err(err) => {
-            assert_eq!(err.message(), ["ouch"]);
-            assert_eq!(err.classes(), ["simpleError", "error", "condition"]);
+            assert_match!(err.message(), Ok(message) => {
+                assert_eq!(message, ["ouch"])
+            });
+            assert_match!(err.classes(), Ok(classes) => {
+                assert_eq!(classes, ["simpleError", "error", "condition"]);
+            });
         });
 
     }}

From 847375bab5dfa007872e02db711beba614eeeb5f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Romain=20Fran=C3=A7ois?= <romain@posit.co>
Date: Fri, 10 Feb 2023 11:23:14 +0100
Subject: [PATCH 27/30] wrapping the conditionMessage() call in a tryCatch

---
 .../amalthea/crates/harp/src/error.rs         | 25 ++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/extensions/positron-r/amalthea/crates/harp/src/error.rs b/extensions/positron-r/amalthea/crates/harp/src/error.rs
index e507b58c7c5e..aa5e0a166fac 100644
--- a/extensions/positron-r/amalthea/crates/harp/src/error.rs
+++ b/extensions/positron-r/amalthea/crates/harp/src/error.rs
@@ -10,6 +10,7 @@ use std::str::Utf8Error;
 
 use libR_sys::*;
 
+use crate::exec::r_try_catch_error;
 use crate::object::RObject;
 use crate::protect::RProtect;
 use crate::r_symbol;
@@ -51,7 +52,29 @@ impl RError {
             let mut protect = RProtect::new();
             let call = protect.add(Rf_lang2(r_symbol!("conditionMessage"), *self.0));
 
-            RObject::from(Rf_eval(call, R_BaseEnv)).try_into()
+            let result = r_try_catch_error(|| {
+                Rf_eval(call, R_BaseEnv)
+            });
+            match result {
+                Ok(message) => {
+                    RObject::from(message).try_into()
+                },
+                Err(error) => {
+                    let msg = match error.message() {
+                        Ok(message) => {
+                            message.join("\n")
+                        },
+                        Err(_error) => {
+                            String::from("Error evaluating conditionMessage()")
+                        }
+                    };
+
+                    Err(Error::EvaluationError {
+                        code: String::from("conditionMessage()"),
+                        message: msg
+                    })
+                }
+            }
         }
     }
 

From feb6d4efe3978380becbcd15cf6b51a07909f95f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Romain=20Fran=C3=A7ois?= <romain@posit.co>
Date: Fri, 10 Feb 2023 12:17:30 +0100
Subject: [PATCH 28/30] RError -> TryCatchError

---
 .../amalthea/crates/harp/src/error.rs         | 64 +----------------
 .../amalthea/crates/harp/src/exec.rs          | 69 +++++++++++++++++--
 2 files changed, 63 insertions(+), 70 deletions(-)

diff --git a/extensions/positron-r/amalthea/crates/harp/src/error.rs b/extensions/positron-r/amalthea/crates/harp/src/error.rs
index aa5e0a166fac..5e2b00762fed 100644
--- a/extensions/positron-r/amalthea/crates/harp/src/error.rs
+++ b/extensions/positron-r/amalthea/crates/harp/src/error.rs
@@ -8,13 +8,7 @@
 use std::fmt;
 use std::str::Utf8Error;
 
-use libR_sys::*;
-
-use crate::exec::r_try_catch_error;
-use crate::object::RObject;
-use crate::protect::RProtect;
-use crate::r_symbol;
-use crate::utils::{r_type2char, r_inherits};
+use crate::utils::r_type2char;
 
 pub type Result<T> = std::result::Result<T, Error>;
 
@@ -30,62 +24,6 @@ pub enum Error {
     TopLevelExecError()
 }
 
-pub struct RError(pub RObject);
-
-impl RError {
-    pub fn new(condition: SEXP) -> Self {
-        RError(RObject::from(condition))
-    }
-
-    pub fn condition(&self) -> SEXP {
-        *self.0
-    }
-
-    pub fn classes(&self) -> Result<Vec<String>>  {
-        unsafe {
-            RObject::from(Rf_getAttrib(*self.0, R_ClassSymbol)).try_into()
-        }
-    }
-
-    pub fn message(&self) -> Result<Vec<String>> {
-        unsafe {
-            let mut protect = RProtect::new();
-            let call = protect.add(Rf_lang2(r_symbol!("conditionMessage"), *self.0));
-
-            let result = r_try_catch_error(|| {
-                Rf_eval(call, R_BaseEnv)
-            });
-            match result {
-                Ok(message) => {
-                    RObject::from(message).try_into()
-                },
-                Err(error) => {
-                    let msg = match error.message() {
-                        Ok(message) => {
-                            message.join("\n")
-                        },
-                        Err(_error) => {
-                            String::from("Error evaluating conditionMessage()")
-                        }
-                    };
-
-                    Err(Error::EvaluationError {
-                        code: String::from("conditionMessage()"),
-                        message: msg
-                    })
-                }
-            }
-        }
-    }
-
-    pub fn inherits(&self, class: &str) -> bool {
-        unsafe {
-            r_inherits(*self.0, &class)
-        }
-    }
-
-}
-
 // empty implementation required for 'anyhow'
 impl std::error::Error for Error {
 
diff --git a/extensions/positron-r/amalthea/crates/harp/src/exec.rs b/extensions/positron-r/amalthea/crates/harp/src/exec.rs
index 02def3ecd41b..aadfe5f037e7 100644
--- a/extensions/positron-r/amalthea/crates/harp/src/exec.rs
+++ b/extensions/positron-r/amalthea/crates/harp/src/exec.rs
@@ -14,7 +14,6 @@ use std::os::raw::c_char;
 use libR_sys::*;
 
 use crate::error::Error;
-use crate::error::RError;
 use crate::error::Result;
 use crate::object::RObject;
 use crate::object::ToRStrings;
@@ -193,6 +192,62 @@ pub unsafe fn geterrmessage() -> String {
 
 }
 
+pub struct TryCatchError(pub RObject);
+
+impl TryCatchError {
+    pub fn new(condition: SEXP) -> Self {
+        TryCatchError(RObject::from(condition))
+    }
+
+    pub fn condition(&self) -> SEXP {
+        *self.0
+    }
+
+    pub fn classes(&self) -> Result<Vec<String>>  {
+        unsafe {
+            RObject::from(Rf_getAttrib(*self.0, R_ClassSymbol)).try_into()
+        }
+    }
+
+    pub fn message(&self) -> Result<Vec<String>> {
+        unsafe {
+            let mut protect = RProtect::new();
+            let call = protect.add(Rf_lang2(r_symbol!("conditionMessage"), *self.0));
+
+            let result = r_try_catch_error(|| {
+                Rf_eval(call, R_BaseEnv)
+            });
+            match result {
+                Ok(message) => {
+                    RObject::from(message).try_into()
+                },
+                Err(error) => {
+                    let msg = match error.message() {
+                        Ok(message) => {
+                            message.join("\n")
+                        },
+                        Err(_error) => {
+                            String::from("Error evaluating conditionMessage()")
+                        }
+                    };
+
+                    Err(Error::EvaluationError {
+                        code: String::from("conditionMessage()"),
+                        message: msg
+                    })
+                }
+            }
+        }
+    }
+
+    pub fn inherits(&self, class: &str) -> bool {
+        unsafe {
+            r_inherits(*self.0, &class)
+        }
+    }
+
+}
+
 /// Wrappers around R_tryCatch()
 ///
 /// Takes a single closure that returns either a SEXP or `()`. If an R error is
@@ -214,7 +269,7 @@ pub unsafe fn geterrmessage() -> String {
 ///     void (*finally)(void*), void* fdata
 /// )
 /// ```
-pub unsafe fn r_try_catch_finally<F, R, S, Finally>(mut fun: F, classes: S, mut finally: Finally) -> std::result::Result<RObject, RError>
+pub unsafe fn r_try_catch_finally<F, R, S, Finally>(mut fun: F, classes: S, mut finally: Finally) -> std::result::Result<RObject, TryCatchError>
 where
     F: FnMut() -> R,
     R: Into<RObject>,
@@ -290,11 +345,11 @@ where
 
     match success {
         true => Ok(RObject::from(result)),
-        false => Err(RError::new(result))
+        false => Err(TryCatchError::new(result))
     }
 }
 
-pub unsafe fn r_try_catch<F, R, S>(fun: F, classes: S) -> std::result::Result<RObject, RError>
+pub unsafe fn r_try_catch<F, R, S>(fun: F, classes: S) -> std::result::Result<RObject, TryCatchError>
 where
     F: FnMut() -> R,
     RObject: From<R>,
@@ -303,7 +358,7 @@ where
     r_try_catch_finally(fun, classes, || {})
 }
 
-pub unsafe fn r_try_catch_error<F, R>(fun: F) -> std::result::Result<RObject, RError>
+pub unsafe fn r_try_catch_error<F, R>(fun: F) -> std::result::Result<RObject, TryCatchError>
 where
     F: FnMut() -> R,
     RObject: From<R>
@@ -318,7 +373,7 @@ pub enum ParseResult {
         message: String,
         line: i32
     },
-    ParseError(RError)
+    ParseError(TryCatchError)
 }
 
 #[allow(non_upper_case_globals)]
@@ -333,7 +388,7 @@ pub unsafe fn r_parse_vector(code: String) -> ParseResult {
     };
 
     match r_try_catch_error(lambda) {
-        Err(error) => ParseResult::ParseError(RError::new(*error.0)),
+        Err(error) => ParseResult::ParseError(TryCatchError::new(*error.0)),
 
         Ok(out) => {
             match ps {

From cb9314efbf06cdb5d832c3845a53637e896564aa Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Romain=20Fran=C3=A7ois?= <romain@posit.co>
Date: Fri, 10 Feb 2023 13:56:33 +0100
Subject: [PATCH 29/30] - TopLevelExecError

---
 extensions/positron-r/amalthea/crates/harp/src/error.rs | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/extensions/positron-r/amalthea/crates/harp/src/error.rs b/extensions/positron-r/amalthea/crates/harp/src/error.rs
index 5e2b00762fed..dcc346576245 100644
--- a/extensions/positron-r/amalthea/crates/harp/src/error.rs
+++ b/extensions/positron-r/amalthea/crates/harp/src/error.rs
@@ -20,8 +20,7 @@ pub enum Error {
     UnsafeEvaluationError(String),
     UnexpectedLength(u32, u32),
     UnexpectedType(u32, Vec<u32>),
-    InvalidUtf8(Utf8Error),
-    TopLevelExecError()
+    InvalidUtf8(Utf8Error)
 }
 
 // empty implementation required for 'anyhow'
@@ -75,11 +74,6 @@ impl fmt::Display for Error {
                 write!(f, "Invalid UTF-8 in string: {}", error)
             }
 
-            Error::TopLevelExecError() => {
-                write!(f, "Top Level exec error")
-            }
-
-
         }
     }
 }

From 9d21f83bc47c1c309d2e5d93077cbebe0551f05d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Romain=20Fran=C3=A7ois?= <romain@posit.co>
Date: Fri, 10 Feb 2023 14:50:06 +0100
Subject: [PATCH 30/30] rework the Error enum so that try_catch return a
 Result<RObject>

---
 .../amalthea/crates/harp/src/error.rs         |  12 +-
 .../amalthea/crates/harp/src/exec.rs          | 147 +++++++-----------
 2 files changed, 64 insertions(+), 95 deletions(-)

diff --git a/extensions/positron-r/amalthea/crates/harp/src/error.rs b/extensions/positron-r/amalthea/crates/harp/src/error.rs
index dcc346576245..5d574c2ae6f2 100644
--- a/extensions/positron-r/amalthea/crates/harp/src/error.rs
+++ b/extensions/positron-r/amalthea/crates/harp/src/error.rs
@@ -20,7 +20,9 @@ pub enum Error {
     UnsafeEvaluationError(String),
     UnexpectedLength(u32, u32),
     UnexpectedType(u32, Vec<u32>),
-    InvalidUtf8(Utf8Error)
+    InvalidUtf8(Utf8Error),
+    TryCatchError { message: Vec<String>, classes : Vec<String> },
+    ParseSyntaxError { message: String, line: i32 }
 }
 
 // empty implementation required for 'anyhow'
@@ -74,6 +76,14 @@ impl fmt::Display for Error {
                 write!(f, "Invalid UTF-8 in string: {}", error)
             }
 
+            Error::TryCatchError { message: _, classes: _ } => {
+                write!(f, "tryCatch error")
+            }
+
+            Error::ParseSyntaxError { message, line } => {
+                write!(f, "Syntax error on line {} when parsing: {}", line, message)
+            }
+
         }
     }
 }
diff --git a/extensions/positron-r/amalthea/crates/harp/src/exec.rs b/extensions/positron-r/amalthea/crates/harp/src/exec.rs
index aadfe5f037e7..224181b85799 100644
--- a/extensions/positron-r/amalthea/crates/harp/src/exec.rs
+++ b/extensions/positron-r/amalthea/crates/harp/src/exec.rs
@@ -192,62 +192,6 @@ pub unsafe fn geterrmessage() -> String {
 
 }
 
-pub struct TryCatchError(pub RObject);
-
-impl TryCatchError {
-    pub fn new(condition: SEXP) -> Self {
-        TryCatchError(RObject::from(condition))
-    }
-
-    pub fn condition(&self) -> SEXP {
-        *self.0
-    }
-
-    pub fn classes(&self) -> Result<Vec<String>>  {
-        unsafe {
-            RObject::from(Rf_getAttrib(*self.0, R_ClassSymbol)).try_into()
-        }
-    }
-
-    pub fn message(&self) -> Result<Vec<String>> {
-        unsafe {
-            let mut protect = RProtect::new();
-            let call = protect.add(Rf_lang2(r_symbol!("conditionMessage"), *self.0));
-
-            let result = r_try_catch_error(|| {
-                Rf_eval(call, R_BaseEnv)
-            });
-            match result {
-                Ok(message) => {
-                    RObject::from(message).try_into()
-                },
-                Err(error) => {
-                    let msg = match error.message() {
-                        Ok(message) => {
-                            message.join("\n")
-                        },
-                        Err(_error) => {
-                            String::from("Error evaluating conditionMessage()")
-                        }
-                    };
-
-                    Err(Error::EvaluationError {
-                        code: String::from("conditionMessage()"),
-                        message: msg
-                    })
-                }
-            }
-        }
-    }
-
-    pub fn inherits(&self, class: &str) -> bool {
-        unsafe {
-            r_inherits(*self.0, &class)
-        }
-    }
-
-}
-
 /// Wrappers around R_tryCatch()
 ///
 /// Takes a single closure that returns either a SEXP or `()`. If an R error is
@@ -269,7 +213,7 @@ impl TryCatchError {
 ///     void (*finally)(void*), void* fdata
 /// )
 /// ```
-pub unsafe fn r_try_catch_finally<F, R, S, Finally>(mut fun: F, classes: S, mut finally: Finally) -> std::result::Result<RObject, TryCatchError>
+pub unsafe fn r_try_catch_finally<F, R, S, Finally>(mut fun: F, classes: S, mut finally: Finally) -> Result<RObject>
 where
     F: FnMut() -> R,
     R: Into<RObject>,
@@ -344,12 +288,32 @@ where
     );
 
     match success {
-        true => Ok(RObject::from(result)),
-        false => Err(TryCatchError::new(result))
+        true => {
+            // the call to tryCatch() was successful, so we return the result
+            // as an RObject
+            Ok(RObject::from(result))
+        },
+        false => {
+            // the call to tryCatch failed, so result is a condition
+            // from which we can extract classes and message via a call to conditionMessage()
+            let classes : Vec<String> = RObject::from(Rf_getAttrib(result, R_ClassSymbol)).try_into()?;
+
+            let mut protect = RProtect::new();
+            let call = protect.add(Rf_lang2(r_symbol!("conditionMessage"), result));
+
+            // TODO: wrap the call to conditionMessage() in a tryCatch
+            //       but this cannot be another call to r_try_catch_error()
+            //       because it creates a recursion problem
+            let message: Vec<String> = RObject::from(Rf_eval(call, R_BaseEnv)).try_into()?;
+
+            Err(Error::TryCatchError {
+                message, classes
+            })
+        }
     }
 }
 
-pub unsafe fn r_try_catch<F, R, S>(fun: F, classes: S) -> std::result::Result<RObject, TryCatchError>
+pub unsafe fn r_try_catch<F, R, S>(fun: F, classes: S) -> Result<RObject>
 where
     F: FnMut() -> R,
     RObject: From<R>,
@@ -358,7 +322,7 @@ where
     r_try_catch_finally(fun, classes, || {})
 }
 
-pub unsafe fn r_try_catch_error<F, R>(fun: F) -> std::result::Result<RObject, TryCatchError>
+pub unsafe fn r_try_catch_error<F, R>(fun: F) -> Result<RObject>
 where
     F: FnMut() -> R,
     RObject: From<R>
@@ -367,17 +331,12 @@ where
 }
 
 pub enum ParseResult {
-    Ok(SEXP),
-    Incomplete(),
-    SyntaxError {
-        message: String,
-        line: i32
-    },
-    ParseError(TryCatchError)
+    Complete(SEXP),
+    Incomplete()
 }
 
 #[allow(non_upper_case_globals)]
-pub unsafe fn r_parse_vector(code: String) -> ParseResult {
+pub unsafe fn r_parse_vector(code: String) -> Result<ParseResult> {
 
     let mut ps : ParseStatus = 0;
     let mut protect = RProtect::new();
@@ -387,20 +346,24 @@ pub unsafe fn r_parse_vector(code: String) -> ParseResult {
         R_ParseVector(r_code, -1, &mut ps, R_NilValue)
     };
 
-    match r_try_catch_error(lambda) {
-        Err(error) => ParseResult::ParseError(TryCatchError::new(*error.0)),
-
-        Ok(out) => {
-            match ps {
-                ParseStatus_PARSE_OK => ParseResult::Ok(*out),
-                ParseStatus_PARSE_INCOMPLETE => ParseResult::Incomplete(),
-                _ => {
-                    ParseResult::SyntaxError{
-                        message: CStr::from_ptr(R_ParseErrorMsg.as_ptr()).to_string_lossy().to_string(),
-                        line: R_ParseError as i32
-                    }
-                }
-            }
+    let result = r_try_catch_error(lambda)?;
+
+    match ps {
+        ParseStatus_PARSE_OK => {
+            Ok(ParseResult::Complete(*result))
+        },
+        ParseStatus_PARSE_INCOMPLETE => Ok(ParseResult::Incomplete()),
+        ParseStatus_PARSE_ERROR => {
+            Err(Error::ParseSyntaxError {
+                message: CStr::from_ptr(R_ParseErrorMsg.as_ptr()).to_string_lossy().to_string(),
+                line: R_ParseError as i32
+            })
+        },
+        _ => {
+            // should not get here
+            Err(Error::ParseError {
+                code, message: String::from("Unknown parse error")
+            })
         }
     }
 }
@@ -538,13 +501,9 @@ mod tests {
             Rf_error(unsafe {msg.as_ptr()});
         });
 
-        assert_match!(out, Err(err) => {
-            assert_match!(err.message(), Ok(message) => {
-                assert_eq!(message, ["ouch"])
-            });
-            assert_match!(err.classes(), Ok(classes) => {
-                assert_eq!(classes, ["simpleError", "error", "condition"]);
-            });
+        assert_match!(out, Err(Error::TryCatchError { message, classes }) => {
+            assert_eq!(message, ["ouch"]);
+            assert_eq!(classes, ["simpleError", "error", "condition"]);
         });
 
     }}
@@ -554,7 +513,7 @@ mod tests {
         // complete
         assert_match!(
             r_parse_vector(String::from("force(42)")),
-            ParseResult::Ok(out) => {
+            Ok(ParseResult::Complete(out)) => {
                 assert_eq!(r_typeof(out), EXPRSXP as u32);
 
                 let call = VECTOR_ELT(out, 0);
@@ -571,19 +530,19 @@ mod tests {
         // incomplete
         assert_match!(
             r_parse_vector(String::from("force(42")),
-            ParseResult::Incomplete()
+            Ok(ParseResult::Incomplete())
         );
 
         // error
         assert_match!(
             r_parse_vector(String::from("42 + _")),
-            ParseResult::ParseError(_)
+            Err(_) => {}
         );
 
         // "normal" syntax error
         assert_match!(
             r_parse_vector(String::from("1+1\n*42")),
-            ParseResult::SyntaxError {message, line} => {
+            Err(Error::ParseSyntaxError {message, line}) => {
                 assert!(message.contains("unexpected"));
                 assert_eq!(line, 2);
             }