Skip to content

Commit 81e9c1d

Browse files
authored
Allow extensions to contribute to F1 show help topic (#785)
* Include the extract operator within help topic node * Allow extensions to register custom help handlers * Add entrypoint to allow serving web-pages in the help pane * move implementation to rust and use debug_env() * add documentation for ark methods * make env immutable * Can simplify slightly * check RMain initialized * Rename topic * don't fallback when the help topic is clearly an expression * rename making it clear this is not the default approach * Add one test case and simplify * refactor tests + add test for custom handlers
1 parent 674475d commit 81e9c1d

File tree

8 files changed

+311
-77
lines changed

8 files changed

+311
-77
lines changed

crates/ark/src/browser.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ use libr::Rf_ScalarLogical;
1111
use libr::SEXP;
1212

1313
use crate::help::message::HelpEvent;
14+
use crate::help::message::ShowHelpUrlKind;
1415
use crate::help::message::ShowHelpUrlParams;
1516
use crate::interface::RMain;
1617
use crate::ui::events::send_open_with_system_event;
@@ -30,7 +31,10 @@ fn is_help_url(url: &str) -> bool {
3031

3132
fn handle_help_url(url: String) -> anyhow::Result<()> {
3233
RMain::with(|main| {
33-
let event = HelpEvent::ShowHelpUrl(ShowHelpUrlParams { url });
34+
let event = HelpEvent::ShowHelpUrl(ShowHelpUrlParams {
35+
url,
36+
kind: ShowHelpUrlKind::HelpProxy,
37+
});
3438
main.send_help_event(event)
3539
})
3640
}

crates/ark/src/help/message.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,17 @@ pub enum HelpEvent {
1515
ShowHelpUrl(ShowHelpUrlParams),
1616
}
1717

18+
#[derive(Debug)]
19+
pub enum ShowHelpUrlKind {
20+
HelpProxy,
21+
External,
22+
}
23+
1824
#[derive(Debug)]
1925
pub struct ShowHelpUrlParams {
2026
/// Url to attempt to show.
2127
pub url: String,
28+
pub kind: ShowHelpUrlKind,
2229
}
2330

2431
impl std::fmt::Display for HelpEvent {

crates/ark/src/help/r_help.rs

Lines changed: 126 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,20 @@ use crossbeam::channel::Sender;
1818
use crossbeam::select;
1919
use harp::exec::RFunction;
2020
use harp::exec::RFunctionExt;
21+
use harp::RObject;
22+
use libr::R_GlobalEnv;
23+
use libr::R_NilValue;
24+
use libr::SEXP;
2125
use log::info;
2226
use log::trace;
2327
use log::warn;
2428
use stdext::spawn;
2529

2630
use crate::help::message::HelpEvent;
31+
use crate::help::message::ShowHelpUrlKind;
2732
use crate::help::message::ShowHelpUrlParams;
33+
use crate::interface::RMain;
34+
use crate::methods::ArkGenerics;
2835
use crate::r_task;
2936

3037
/**
@@ -182,27 +189,37 @@ impl RHelp {
182189
/// coming through here has already been verified to look like a help URL with
183190
/// `is_help_url()`, so if we get an unexpected prefix, that's an error.
184191
fn handle_show_help_url(&self, params: ShowHelpUrlParams) -> anyhow::Result<()> {
185-
let url = params.url;
192+
let url = params.url.clone();
186193

187-
if !Self::is_help_url(url.as_str(), self.r_port) {
188-
let prefix = Self::help_url_prefix(self.r_port);
189-
return Err(anyhow!(
190-
"Help URL '{url}' doesn't have expected prefix '{prefix}'."
191-
));
192-
}
194+
let url = match params.kind {
195+
ShowHelpUrlKind::HelpProxy => {
196+
if !Self::is_help_url(url.as_str(), self.r_port) {
197+
let prefix = Self::help_url_prefix(self.r_port);
198+
return Err(anyhow!(
199+
"Help URL '{url}' doesn't have expected prefix '{prefix}'."
200+
));
201+
}
193202

194-
// Re-direct the help event to our help proxy server.
195-
let r_prefix = Self::help_url_prefix(self.r_port);
196-
let proxy_prefix = Self::help_url_prefix(self.proxy_port);
203+
// Re-direct the help event to our help proxy server.
204+
let r_prefix = Self::help_url_prefix(self.r_port);
205+
let proxy_prefix = Self::help_url_prefix(self.proxy_port);
197206

198-
let proxy_url = url.replace(r_prefix.as_str(), proxy_prefix.as_str());
207+
url.replace(r_prefix.as_str(), proxy_prefix.as_str())
208+
},
209+
ShowHelpUrlKind::External => {
210+
// The URL is not a help URL; just use it as-is.
211+
url
212+
},
213+
};
199214

200215
log::trace!(
201-
"Sending frontend event `ShowHelp` with R url '{url}' and proxy url '{proxy_url}'"
216+
"Sending frontend event `ShowHelp` with R url '{}' and proxy url '{}'",
217+
params.url,
218+
url
202219
);
203220

204221
let msg = HelpFrontendEvent::ShowHelp(ShowHelpParams {
205-
content: proxy_url,
222+
content: url,
206223
kind: ShowHelpKind::Url,
207224
focus: true,
208225
});
@@ -215,15 +232,75 @@ impl RHelp {
215232

216233
#[tracing::instrument(level = "trace", skip(self))]
217234
fn show_help_topic(&self, topic: String) -> anyhow::Result<bool> {
218-
let found = r_task(|| unsafe {
219-
RFunction::from(".ps.help.showHelpTopic")
220-
.add(topic)
221-
.call()?
222-
.to::<bool>()
223-
})?;
235+
let topic = HelpTopic::parse(topic);
236+
237+
let found = match topic {
238+
HelpTopic::Simple(symbol) => r_task(|| unsafe {
239+
// Try evaluating the help handler first and then fall back to
240+
// the default help topic display function.
241+
242+
if let Ok(Some(result)) = Self::r_custom_help_handler(symbol.clone()) {
243+
return Ok(result);
244+
}
245+
246+
RFunction::from(".ps.help.showHelpTopic")
247+
.add(symbol)
248+
.call()?
249+
.to::<bool>()
250+
}),
251+
HelpTopic::Expression(expression) => {
252+
// For expressions, we have to use the help handler
253+
// If that fails there's no fallback.
254+
r_task(|| match Self::r_custom_help_handler(expression) {
255+
Ok(Some(result)) => Ok(result),
256+
// No method found
257+
Ok(None) => Ok(false),
258+
// Error during evaluation
259+
Err(err) => Err(harp::Error::Anyhow(err)),
260+
})
261+
},
262+
}?;
263+
224264
Ok(found)
225265
}
226266

267+
// Must be called in a `r_task` context.
268+
// Tries calling a custom help handler defined as an ark method.
269+
fn r_custom_help_handler(topic: String) -> anyhow::Result<Option<bool>> {
270+
unsafe {
271+
let env = (|| {
272+
#[cfg(not(test))]
273+
if RMain::is_initialized() {
274+
if let Some(debug_env) = &RMain::get().debug_env() {
275+
// Mem-Safety: Object protected by `RMain` for the duration of the `r_task()`
276+
return debug_env.sexp;
277+
}
278+
}
279+
280+
R_GlobalEnv
281+
})();
282+
283+
let obj = harp::parse_eval0(topic.as_str(), env)?;
284+
let handler: Option<RObject> =
285+
ArkGenerics::HelpGetHandler.try_dispatch(obj.sexp, vec![])?;
286+
287+
if let Some(handler) = handler {
288+
let mut fun = RFunction::new_inlined(handler);
289+
match fun.call_in(env) {
290+
Err(err) => {
291+
log::error!("Error calling help handler: {:?}", err);
292+
return Err(anyhow!("Error calling help handler: {:?}", err));
293+
},
294+
Ok(result) => {
295+
return Ok(Some(result.try_into()?));
296+
},
297+
}
298+
}
299+
}
300+
301+
Ok(None)
302+
}
303+
227304
pub fn r_start_or_reconnect_to_help_server() -> harp::Result<u16> {
228305
// Start the R help server.
229306
// If it is already started, it just returns the preexisting port number.
@@ -232,3 +309,33 @@ impl RHelp {
232309
.and_then(|x| x.try_into())
233310
}
234311
}
312+
313+
enum HelpTopic {
314+
// no obvious expression syntax — e.g. "abs", "base::abs"
315+
Simple(String),
316+
// contains expression syntax — e.g. "tensorflow::tf$abs", "model@coef"
317+
// such that there will never exist a help topic with that name
318+
Expression(String),
319+
}
320+
321+
impl HelpTopic {
322+
pub fn parse(topic: String) -> Self {
323+
if topic.contains('$') || topic.contains('@') {
324+
Self::Expression(topic)
325+
} else {
326+
Self::Simple(topic)
327+
}
328+
}
329+
}
330+
331+
#[harp::register]
332+
pub unsafe extern "C-unwind" fn ps_help_browse_external_url(
333+
url: SEXP,
334+
) -> Result<SEXP, anyhow::Error> {
335+
RMain::get().send_help_event(HelpEvent::ShowHelpUrl(ShowHelpUrlParams {
336+
url: RObject::view(url).to::<String>()?,
337+
kind: ShowHelpUrlKind::External,
338+
}))?;
339+
340+
Ok(R_NilValue)
341+
}

crates/ark/src/lsp/help_topic.rs

Lines changed: 26 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,7 @@ fn locate_help_node(tree: &Tree, point: Point) -> Option<Node<'_>> {
8989
// Even if they are at `p<>kg::fun`, we assume they really want docs for `fun`.
9090
let node = match node.parent() {
9191
Some(parent) if matches!(parent.node_type(), NodeType::NamespaceOperator(_)) => parent,
92+
Some(parent) if matches!(parent.node_type(), NodeType::ExtractOperator(_)) => parent,
9293
Some(_) => node,
9394
None => node,
9495
};
@@ -110,33 +111,30 @@ mod tests {
110111
.set_language(&tree_sitter_r::LANGUAGE.into())
111112
.expect("failed to create parser");
112113

113-
// On the RHS
114-
let (text, point) = point_from_cursor("dplyr::ac@ross(x:y, sum)");
115-
let tree = parser.parse(text.as_str(), None).unwrap();
116-
let node = locate_help_node(&tree, point).unwrap();
117-
let text = node.utf8_text(text.as_bytes()).unwrap();
118-
assert_eq!(text, "dplyr::across");
119-
120-
// On the LHS (Returns function help for `across()`, not package help for `dplyr`,
121-
// as we assume that is more useful for the user).
122-
let (text, point) = point_from_cursor("dpl@yr::across(x:y, sum)");
123-
let tree = parser.parse(text.as_str(), None).unwrap();
124-
let node = locate_help_node(&tree, point).unwrap();
125-
let text = node.utf8_text(text.as_bytes()).unwrap();
126-
assert_eq!(text, "dplyr::across");
127-
128-
// In the operator
129-
let (text, point) = point_from_cursor("dplyr:@:across(x:y, sum)");
130-
let tree = parser.parse(text.as_str(), None).unwrap();
131-
let node = locate_help_node(&tree, point).unwrap();
132-
let text = node.utf8_text(text.as_bytes()).unwrap();
133-
assert_eq!(text, "dplyr::across");
134-
135-
// Internal `:::`
136-
let (text, point) = point_from_cursor("dplyr:::ac@ross(x:y, sum)");
137-
let tree = parser.parse(text.as_str(), None).unwrap();
138-
let node = locate_help_node(&tree, point).unwrap();
139-
let text = node.utf8_text(text.as_bytes()).unwrap();
140-
assert_eq!(text, "dplyr:::across");
114+
// (text cursor, expected help topic)
115+
let cases = vec![
116+
// On the RHS
117+
("dplyr::ac@ross(x:y, sum)", "dplyr::across"),
118+
// On the LHS (Returns function help for `across()`, not package help for `dplyr`,
119+
// as we assume that is more useful for the user).
120+
("dpl@yr::across(x:y, sum)", "dplyr::across"),
121+
// In the operator
122+
("dplyr:@:across(x:y, sum)", "dplyr::across"),
123+
// Internal `:::`
124+
("dplyr:::ac@ross(x:y, sum)", "dplyr:::across"),
125+
// R6 methods, or reticulate accessors
126+
("tf$a@bs(x)", "tf$abs"),
127+
("t@f$abs(x)", "tf$abs"),
128+
// With the package namespace
129+
("tensorflow::tf$ab@s(x)", "tensorflow::tf$abs"),
130+
];
131+
132+
for (code, expected) in cases {
133+
let (text, point) = point_from_cursor(code);
134+
let tree = parser.parse(text.as_str(), None).unwrap();
135+
let node = locate_help_node(&tree, point).unwrap();
136+
let text = node.utf8_text(text.as_bytes()).unwrap();
137+
assert_eq!(text, expected);
138+
}
141139
}
142140
}

crates/ark/src/methods.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,9 @@ pub enum ArkGenerics {
4242

4343
#[strum(serialize = "ark_positron_variable_has_viewer")]
4444
VariableHasViewer,
45+
46+
#[strum(serialize = "ark_positron_help_get_handler")]
47+
HelpGetHandler,
4548
}
4649

4750
impl ArkGenerics {

crates/ark/src/modules/positron/help.R

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -230,6 +230,11 @@ getHtmlHelpContentsDevImpl <- function(x) {
230230
.ps.Call("ps_browse_url", as.character(url))
231231
}
232232

233+
#' @export
234+
.ps.help.browse_external_url <- function(url) {
235+
.ps.Call("ps_help_browse_external_url", as.character(url))
236+
}
237+
233238
# @param rd_file Path to an `.Rd` file.
234239
# @returns The result of converting that `.Rd` to HTML and concatenating to a
235240
# string.

0 commit comments

Comments
 (0)