Skip to content

Commit a3d077e

Browse files
lionel-lionel-
committed
Fix a few style issues
1 parent a2f22e5 commit a3d077e

4 files changed

Lines changed: 34 additions & 72 deletions

File tree

‎crates/ark/ark_macros/src/lib.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ extern crate proc_macro;
5555
pub fn register(_attr: TokenStream, item: TokenStream) -> TokenStream {
5656
let function = parse_macro_input!(item as syn::ItemFn);
5757
match register_impl(function) {
58-
Ok(tokens) => tokens.into(),
58+
Ok(tokens) => tokens,
5959
Err(err) => err.to_compile_error().into(),
6060
}
6161
}

‎crates/ark/src/comm_handler.rs‎

Lines changed: 12 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ pub fn handle_comm_message<Reqs, Reps, Evts>(
126126
parent_header,
127127
data,
128128
} => {
129-
let json = dispatch_rpc(comm_name, &data, |req| {
129+
let json = dispatch_rpc(comm_name, data, |req| {
130130
let _span = tracing::trace_span!("comm handler", name = comm_name, request = ?req)
131131
.entered();
132132
rpc_handler(req)
@@ -139,7 +139,7 @@ pub fn handle_comm_message<Reqs, Reps, Evts>(
139139
outgoing_tx.send(response).log_err();
140140
},
141141
CommMsg::Data(data) => {
142-
dispatch_event(comm_name, &data, |evt| {
142+
dispatch_event(comm_name, data, |evt| {
143143
let _span =
144144
tracing::trace_span!("comm handler", name = comm_name, event = ?evt).entered();
145145
event_handler(evt)
@@ -176,7 +176,7 @@ pub fn handle_rpc_request<Reqs, Reps>(
176176
},
177177
};
178178

179-
let json = dispatch_rpc(comm_name, &data, |req| {
179+
let json = dispatch_rpc(comm_name, data, |req| {
180180
let _span =
181181
tracing::trace_span!("comm handler", name = comm_name, request = ?req).entered();
182182
request_handler(req)
@@ -192,36 +192,32 @@ pub fn handle_rpc_request<Reqs, Reps>(
192192

193193
fn dispatch_rpc<Reqs, Reps>(
194194
comm_name: &str,
195-
data: &serde_json::Value,
195+
data: serde_json::Value,
196196
handler: impl FnOnce(Reqs) -> anyhow::Result<Reps>,
197197
) -> serde_json::Value
198198
where
199199
Reqs: DeserializeOwned + Debug,
200200
Reps: Serialize,
201201
{
202-
match serde_json::from_value::<Reqs>(data.clone()) {
202+
match serde_json::from_value::<Reqs>(data) {
203203
Ok(m) => match handler(m) {
204204
Ok(reply) => match serde_json::to_value(reply) {
205205
Ok(value) => value,
206206
Err(err) => {
207-
let message = format!(
208-
"Failed to serialise reply for {comm_name} request: {err} (request: {data})"
209-
);
207+
let message =
208+
format!("Failed to serialise reply for {comm_name} request: {err}");
210209
log::warn!("{message}");
211210
json_rpc_error(JsonRpcErrorCode::InternalError, message)
212211
},
213212
},
214213
Err(err) => {
215-
let message =
216-
format!("Failed to process {comm_name} request: {err} (request: {data})");
214+
let message = format!("Failed to process {comm_name} request: {err}");
217215
log::warn!("{message}");
218216
json_rpc_error(JsonRpcErrorCode::InternalError, message)
219217
},
220218
},
221219
Err(err) => {
222-
let message = format!(
223-
"No handler for {comm_name} request (method not found): {err} (request: {data})"
224-
);
220+
let message = format!("No handler for {comm_name} request (method not found): {err}");
225221
log::warn!("{message}");
226222
json_rpc_error(JsonRpcErrorCode::MethodNotFound, message)
227223
},
@@ -230,17 +226,17 @@ where
230226

231227
fn dispatch_event<Evts>(
232228
comm_name: &str,
233-
data: &serde_json::Value,
229+
data: serde_json::Value,
234230
handler: impl FnOnce(Evts) -> anyhow::Result<()>,
235231
) where
236232
Evts: DeserializeOwned + Debug,
237233
{
238-
match serde_json::from_value::<Evts>(data.clone()) {
234+
match serde_json::from_value::<Evts>(data) {
239235
Ok(event) => {
240236
handler(event).log_err();
241237
},
242238
Err(err) => {
243-
log::warn!("Failed to parse {comm_name} event: {err} (data: {data})");
239+
log::warn!("Failed to parse {comm_name} event: {err}");
244240
},
245241
}
246242
}

‎crates/ark/src/plots/graphics_device.rs‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -294,7 +294,7 @@ impl DeviceContext {
294294
self.clear_pending_origin();
295295
}
296296

297-
/// Should plot events be sent over [CommSocket]s to the frontend?
297+
/// Should plot events be sent over comm channels to the frontend?
298298
///
299299
/// This allows plots to be dynamically resized by their `id`. Only possible if the UI
300300
/// comm is connected (i.e. we are connected to Positron) and if we are in
@@ -530,7 +530,7 @@ impl DeviceContext {
530530
let mime_type = Self::get_mime_type(&plot_meta.format);
531531

532532
Ok(PlotBackendReply::RenderReply(PlotResult {
533-
data: data.to_string(),
533+
data,
534534
mime_type: mime_type.to_string(),
535535
settings: Some(settings),
536536
}))
@@ -557,13 +557,13 @@ impl DeviceContext {
557557
}
558558
}
559559

560-
fn get_mime_type(format: &PlotRenderFormat) -> String {
560+
fn get_mime_type(format: &PlotRenderFormat) -> &'static str {
561561
match format {
562-
PlotRenderFormat::Png => "image/png".to_string(),
563-
PlotRenderFormat::Svg => "image/svg+xml".to_string(),
564-
PlotRenderFormat::Pdf => "application/pdf".to_string(),
565-
PlotRenderFormat::Jpeg => "image/jpeg".to_string(),
566-
PlotRenderFormat::Tiff => "image/tiff".to_string(),
562+
PlotRenderFormat::Png => "image/png",
563+
PlotRenderFormat::Svg => "image/svg+xml",
564+
PlotRenderFormat::Pdf => "application/pdf",
565+
PlotRenderFormat::Jpeg => "image/jpeg",
566+
PlotRenderFormat::Tiff => "image/tiff",
567567
}
568568
}
569569

@@ -653,7 +653,7 @@ impl DeviceContext {
653653
let mime_type = Self::get_mime_type(&PlotRenderFormat::Png);
654654

655655
let pre_render = PlotResult {
656-
data: pre_render.to_string(),
656+
data: pre_render,
657657
mime_type: mime_type.to_string(),
658658
settings: Some(settings),
659659
};
@@ -708,7 +708,7 @@ impl DeviceContext {
708708
return;
709709
};
710710

711-
log::info!("Sending display data to IOPub.");
711+
log::trace!("Sending display data to IOPub.");
712712

713713
self.iopub_tx
714714
.send(IOPubMessage::DisplayData(DisplayData {
@@ -765,7 +765,7 @@ impl DeviceContext {
765765
let mime_type = Self::get_mime_type(&settings.format);
766766

767767
let pre_render = PlotResult {
768-
data: pre_render.to_string(),
768+
data: pre_render,
769769
mime_type: mime_type.to_string(),
770770
settings: Some(settings),
771771
};
@@ -809,7 +809,7 @@ impl DeviceContext {
809809
data: None,
810810
};
811811

812-
log::info!("Sending update display data to IOPub for `id` {id}.");
812+
log::trace!("Sending update display data to IOPub for `id` {id}.");
813813

814814
self.iopub_tx
815815
.send(IOPubMessage::UpdateDisplayData(UpdateDisplayData {

‎crates/ark/tests/plots.rs‎

Lines changed: 9 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -372,22 +372,9 @@ fn test_plot_get_metadata() {
372372
// [1] "plot(1:10)"
373373

374374
// Verify execution_id matches the msg_id of the execute_request
375-
assert!(
376-
result.contains(&msg_id),
377-
"Metadata should contain execution_id '{msg_id}', got:\n{result}"
378-
);
379-
380-
// Verify code matches
381-
assert!(
382-
result.contains(code),
383-
"Metadata should contain code '{code}', got:\n{result}"
384-
);
385-
386-
// Verify kind is "plot" for base R plots
387-
assert!(
388-
result.contains("$kind") && result.contains("\"plot\""),
389-
"Metadata should contain kind 'plot', got:\n{result}"
390-
);
375+
assert!(result.contains(&format!("$execution_id\n[1] \"{msg_id}\"")));
376+
assert!(result.contains(&format!("$code\n[1] \"{code}\"")));
377+
assert!(result.contains("$kind\n[1] \"plot\""));
391378
}
392379

393380
/// Test that plot metadata includes origin when code_location is provided.
@@ -443,10 +430,7 @@ fn test_plot_get_metadata_with_origin() {
443430
frontend.recv_shell_execute_reply();
444431

445432
// Verify origin_uri is present in the metadata
446-
assert!(
447-
result.contains(origin_uri),
448-
"Metadata should contain origin_uri '{origin_uri}', got:\n{result}"
449-
);
433+
assert!(result.contains(&format!("$origin_uri\n[1] \"{origin_uri}\"")));
450434
}
451435

452436
/// Test that plots are emitted when created inside source().
@@ -585,16 +569,8 @@ fn test_plot_source_context_stacking() {
585569
frontend.recv_shell_execute_reply();
586570

587571
// The origin_uri should point to file B, not file A
588-
assert!(
589-
result_b.contains(&file_b.uri_id),
590-
"Plot from file B should have origin_uri pointing to file B '{}', got:\n{result_b}",
591-
file_b.uri_id,
592-
);
593-
assert!(
594-
!result_b.contains(&file_a.uri_id),
595-
"Plot from file B should NOT have origin_uri pointing to file A '{}', got:\n{result_b}",
596-
file_a.uri_id,
597-
);
572+
assert!(result_b.contains(&format!("$origin_uri\n[1] \"{}\"", file_b.uri_id)));
573+
assert!(!result_b.contains(&file_a.uri_id));
598574

599575
// Query metadata for the second plot (created by file A)
600576
let query_a = format!(".ps.graphics.get_metadata('{display_id_a}')");
@@ -606,11 +582,7 @@ fn test_plot_source_context_stacking() {
606582
frontend.recv_shell_execute_reply();
607583

608584
// The origin_uri should point to file A
609-
assert!(
610-
result_a.contains(&file_a.uri_id),
611-
"Plot from file A should have origin_uri pointing to file A '{}', got:\n{result_a}",
612-
file_a.uri_id,
613-
);
585+
assert!(result_a.contains(&format!("$origin_uri\n[1] \"{}\"", file_a.uri_id)));
614586
}
615587

616588
/// Test that plots rendered with fig-width/fig-height metadata produce
@@ -777,14 +749,8 @@ fn test_plot_without_execution_context_has_empty_metadata() {
777749

778750
// execution_id and code should be empty since the plot was created
779751
// outside of an execute request's execution context
780-
assert!(
781-
result.contains("$execution_id") && result.contains("[1] \"\""),
782-
"execution_id should be empty, got:\n{result}"
783-
);
784-
assert!(
785-
result.contains("$code") && result.contains("[1] \"\""),
786-
"code should be empty, got:\n{result}"
787-
);
752+
assert!(result.contains("$execution_id\n[1] \"\""));
753+
assert!(result.contains("$code\n[1] \"\""));
788754
}
789755

790756
/// Test that `dev.hold()` suppresses intermediate plot output.

0 commit comments

Comments
 (0)