add channel to indicate when help is/isn't shown

Author: jmcphers

crates/ark/src/browser.rs

@@ -11,6 +11,7 @@ use anyhow::Result;
 use harp::object::RObject;
 use libR_sys::*;
 
+use crate::help::message::HelpReply;
 use crate::help::message::HelpRequest;
 use crate::interface::R_MAIN;
 
@@ -46,7 +47,16 @@ unsafe fn handle_help_url(url: &str) -> Result<bool> {
         return Ok(false);
     }
 
-    Ok(true)
+    // Wait up to 1 second for a reply from the help thread
+    let reply = main
+        .help_rx
+        .as_ref()
+        .unwrap()
+        .recv_timeout(std::time::Duration::from_secs(1))?;
+
+    match reply {
+        HelpReply::ShowHelpUrlReply(found) => Ok(found),
+    }
 }
 
 unsafe fn ps_browse_url_impl(url: SEXP) -> Result<()> {

crates/ark/src/help/message.rs

@@ -32,6 +32,14 @@ pub enum HelpMessage {
 pub enum HelpRequest {
     /// Request to show the given URL to the user in the Help pane.
     ShowHelpUrlRequest(String),
+}
+
+/**
+ * Enum representing replies from the Help thread.
+ */
+pub enum HelpReply {
+    /// Reply to ShowHelpUrlRequest; indicates whether the URL was successfully
+    /// shown.
     ShowHelpUrlReply(bool),
 }
 

crates/ark/src/help/r_help.rs

@@ -22,6 +22,7 @@ use stdext::spawn;
 
 use crate::browser;
 use crate::help::message::HelpMessage;
+use crate::help::message::HelpReply;
 use crate::help::message::HelpRequest;
 use crate::help::message::ShowHelpContent;
 use crate::help::message::ShowTopicReply;
@@ -36,6 +37,7 @@ pub struct RHelp {
     comm: CommSocket,
     r_help_port: u16,
     help_request_rx: Receiver<HelpRequest>,
+    help_reply_tx: Sender<HelpReply>,
 }
 
 impl RHelp {
@@ -45,7 +47,7 @@ impl RHelp {
      *
      * - `comm`: The socket for communicating with the front end.
      */
-    pub fn start(comm: CommSocket) -> Result<Sender<HelpRequest>> {
+    pub fn start(comm: CommSocket) -> Result<(Sender<HelpRequest>, Receiver<HelpReply>)> {
         // Check to see whether the help server has started. We set the port
         // number when it starts, so if it's still at the default value (0), it
         // hasn't started.
@@ -79,6 +81,7 @@ impl RHelp {
         // Create the channels that will be used to communicate with the help
         // thread from other threads.
         let (help_request_tx, help_request_rx) = crossbeam::channel::unbounded();
+        let (help_reply_tx, help_reply_rx) = crossbeam::channel::unbounded();
 
         // Start the help request thread and wait for requests from the front
         // end.
@@ -87,12 +90,13 @@ impl RHelp {
                 comm,
                 r_help_port,
                 help_request_rx,
+                help_reply_tx,
             };
             help.execution_thread();
         });
 
         // Return the channel for sending help requests to the help thread.
-        Ok(help_request_tx)
+        Ok((help_request_tx, help_reply_rx))
     }
 
     /**
@@ -199,20 +203,33 @@ impl RHelp {
 
     fn handle_request(&self, message: HelpRequest) -> Result<()> {
         match message {
-            HelpRequest::ShowHelpUrlRequest(url) => self.show_help_url(url.as_str()),
-            _ => Err(anyhow!("Help: Received unexpected request {:?}", message)),
+            HelpRequest::ShowHelpUrlRequest(url) => {
+                let found = match self.show_help_url(&url) {
+                    Ok(found) => found,
+                    Err(err) => {
+                        error!("Error showing help URL {}: {:?}", url, err);
+                        false
+                    },
+                };
+                self.help_reply_tx
+                    .send(HelpReply::ShowHelpUrlReply(found))?;
+            },
         }
+        Ok(())
     }
 
-    fn show_help_url(&self, url: &str) -> Result<()> {
-        // Check for help URLs
+    /// Shows a help URL by sending a message to the front end. Returns
+    /// `Ok(true)` if the URL was handled, `Ok(false)` if it wasn't.
+    fn show_help_url(&self, url: &str) -> Result<bool> {
+        // Check for help URLs. If this is an R help URL, we'll re-direct it to
+        // our help proxy server.
         let prefix = format!("http://127.0.0.1:{}/", self.r_help_port);
         if !url.starts_with(&prefix) {
-            return Err(anyhow!(
-                "Help URL '{}' doesn't have expected prefix '{}'",
-                url,
-                prefix
-            ));
+            info!(
+                "Help URL '{}' doesn't have expected prefix '{}'; not handling",
+                url, prefix
+            );
+            return Ok(false);
         }
 
         // Re-direct the help request to our help proxy server.
@@ -227,7 +244,9 @@ impl RHelp {
         });
         let json = serde_json::to_value(msg)?;
         self.comm.outgoing_tx.send(CommChannelMsg::Data(json))?;
-        Ok(())
+
+        // The URL was handled.
+        Ok(true)
     }
 
     fn show_help_topic(&self, topic: String) -> Result<bool> {

crates/ark/src/interface.rs

@@ -67,6 +67,7 @@ use stdext::*;
 use crate::dap::dap::DapBackendEvent;
 use crate::dap::Dap;
 use crate::errors;
+use crate::help::message::HelpReply;
 use crate::help::message::HelpRequest;
 use crate::kernel::Kernel;
 use crate::lsp::events::EVENTS;
@@ -280,8 +281,9 @@ pub struct RMain {
     pub error_message: String, // `evalue` in the Jupyter protocol
     pub error_traceback: Vec<String>,
 
-    // Channel to communicate with the Help thread
+    // Channels to communicate with the Help thread
     pub help_tx: Option<Sender<HelpRequest>>,
+    pub help_rx: Option<Receiver<HelpReply>>,
 
     dap: Arc<Mutex<Dap>>,
     is_debugging: bool,
@@ -380,6 +382,7 @@ impl RMain {
             error_message: String::new(),
             error_traceback: Vec::new(),
             help_tx: None,
+            help_rx: None,
             dap,
             is_debugging: false,
             old_show_error_messages: None,

crates/ark/src/shell.rs

@@ -328,7 +328,7 @@ impl ShellHandler for Shell {
             },
             Comm::Help => {
                 // Start the R Help handler
-                let help_request_tx = match RHelp::start(comm.clone()) {
+                let (help_request_tx, help_reply_rx) = match RHelp::start(comm.clone()) {
                     Ok(tx) => tx,
                     Err(err) => {
                         warn!("Could not start R Help handler: {}", err);
@@ -341,6 +341,7 @@ impl ShellHandler for Shell {
                 r_task(|| unsafe {
                     let main = R_MAIN.as_mut().unwrap();
                     main.help_tx = Some(help_request_tx.clone());
+                    main.help_rx = Some(help_reply_rx.clone());
                 });
 
                 Ok(true)

crates/ark/tests/help.rs

@@ -43,7 +43,7 @@ fn test_help_comm() {
     // Start the help comm. It's important to save the help request sender so
     // that the help comm doesn't exit before we're done with it; allowing the
     // sender to be dropped signals the help comm to exit.
-    let help_request_tx = RHelp::start(comm).unwrap();
+    let (help_request_tx, help_reply_rx) = RHelp::start(comm).unwrap();
 
     // Send a request for the help topic 'library'
     let request = HelpMessage::ShowHelpTopicRequest(ShowTopicRequest {
@@ -76,6 +76,7 @@ fn test_help_comm() {
         },
     }
 
-    // No-op request to satisfy usage of help_request_tx
+    // No-op request to keep variables in scope
     help_request_tx.len();
+    help_reply_rx.len();
 }