diff --git a/Cargo.lock b/Cargo.lock index 33d7964..e42c2c6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4463,6 +4463,7 @@ dependencies = [ "log", "octocrab", "octocrab-wasm", + "percent-encoding", "re_ui", "reqwest", "serde", diff --git a/Cargo.toml b/Cargo.toml index 1183d73..34b1fff 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -33,6 +33,7 @@ image = "0.25.8" log = "0.4.28" octocrab = { version = "0.49.7", default-features = false, features = ["stream", "jwt-rust-crypto"] } octocrab-wasm = { path = "crates/octocrab-wasm" } +percent-encoding = "2.3.2" re_ui = { git = "https://github.com/rerun-io/rerun", branch = "main" } reqwest = { version = "0.13.2", default-features = false, features = [] } serde = "1.0" diff --git a/src/diff_image_loader.rs b/src/diff_image_loader.rs index aee81e2..f970fd7 100644 --- a/src/diff_image_loader.rs +++ b/src/diff_image_loader.rs @@ -72,14 +72,13 @@ impl DiffImageLoader { } } - pub fn diff_info(&self, uri: &str) -> Option { - if let Some(image) = self.diffs.lock().get(uri) { - match image { - Ok(Poll::Ready(result)) => Some(result.clone()), - _ => None, - } - } else { - None + /// `Some(Ok(info))` once the diff is ready, `None` while pending or before it has + /// started, and `Some(Err(msg))` if the diff failed to load (e.g. mismatched image sizes). + pub fn diff_info(&self, uri: &str) -> Option> { + match self.diffs.lock().get(uri) { + Some(Ok(Poll::Ready(info))) => Some(Ok(info.clone())), + Some(Ok(Poll::Pending)) | None => None, + Some(Err(err)) => Some(Err(err.to_string())), } } } @@ -122,17 +121,17 @@ impl ImageLoader for DiffImageLoader { std::thread::Builder::new() .name(format!("diff for {uri}")) .spawn(move || { - ctx.request_repaint(); let result = load_diffs(&ctx, &old_image, &new_image, size_hint, &diff_uri); cache.lock().insert(uri, result.map(Poll::Ready)); + ctx.request_repaint(); }) .expect("Failed to spawn diff thread"); #[cfg(target_arch = "wasm32")] { wasm_bindgen_futures::spawn_local(async move { - ctx.request_repaint(); let result = load_diffs(&ctx, &old_image, &new_image, size_hint, &diff_uri); cache.lock().insert(uri, result.map(Poll::Ready)); + ctx.request_repaint(); }); } } diff --git a/src/loaders/pr_loader.rs b/src/loaders/pr_loader.rs index 9d9d3d8..6ec70ce 100644 --- a/src/loaders/pr_loader.rs +++ b/src/loaders/pr_loader.rs @@ -9,9 +9,23 @@ use egui_inbox::{UiInbox, UiInboxSender}; use futures::{StreamExt as _, TryStreamExt as _}; use octocrab::models::repos::DiffEntryStatus; use octocrab::{Octocrab, Result}; +use percent_encoding::{AsciiSet, CONTROLS, utf8_percent_encode}; use std::pin::pin; use std::task::Poll; +/// Percent-encode path segments for use in a URL path, preserving `/`. +/// Mirrors the `PATH` set from the URL spec but lets `utf8_percent_encode` handle non-ASCII. +const PATH_SEGMENT: &AsciiSet = &CONTROLS + .add(b' ') + .add(b'"') + .add(b'#') + .add(b'<') + .add(b'>') + .add(b'?') + .add(b'`') + .add(b'{') + .add(b'}'); + type Sender = UiInboxSender>>; pub struct PrLoader { @@ -114,18 +128,23 @@ async fn resolve_url( file_path: &str, logged_in: bool, ) -> Option { + let encoded_path = utf8_percent_encode(file_path, PATH_SEGMENT).to_string(); if logged_in { let content = repo_client .repos() .get_content() - .path(file_path) + .path(&encoded_path) .r#ref(commit_sha) .send() .await .ok()?; content.items.first()?.download_url.clone() } else { - Some(create_media_url(repo_client.repo(), commit_sha, file_path)) + Some(create_media_url( + repo_client.repo(), + commit_sha, + &encoded_path, + )) } } diff --git a/src/viewer/diff_view.rs b/src/viewer/diff_view.rs index d973e17..8d90895 100644 --- a/src/viewer/diff_view.rs +++ b/src/viewer/diff_view.rs @@ -10,21 +10,33 @@ pub fn diff_view(ui: &mut Ui, state: &ViewerAppStateRef<'_>) { state.app.settings.options, ); - if let Some(info) = - diff_uri.and_then(|diff_uri| state.app.diff_image_loader.diff_info(&diff_uri)) - { - if info.diff == 0 { - ui.strong("All differences below threshold!"); - } else { + let diff_info = diff_uri + .as_deref() + .and_then(|uri| state.app.diff_image_loader.diff_info(uri)); + + match &diff_info { + Some(Ok(info)) => { + if info.diff == 0 { + ui.strong("All differences below threshold!"); + } else { + ui.label( + RichText::new(format!("Diff pixels: {}", info.diff)) + .color(ui.visuals().warn_fg_color), + ); + } + } + Some(Err(err)) => { ui.label( - RichText::new(format!("Diff pixels: {}", info.diff)) - .color(ui.visuals().warn_fg_color), + RichText::new(format!("Diff failed: {err}")).color(ui.visuals().error_fg_color), ); } - } else { - ui.label("No diff info yet..."); + None => { + ui.label("No diff info yet..."); + } } + let diff_failed = matches!(diff_info, Some(Err(_))); + let rect = ui.available_rect_before_wrap(); let old = snapshot.old_image(state.app); @@ -51,7 +63,9 @@ pub fn diff_view(ui: &mut Ui, state: &ViewerAppStateRef<'_>) { ui.place(rect, new); } - if let Some(diff) = diff { + if let Some(diff) = diff + && !diff_failed + { ui.place(rect, diff); }