Skip to content

Commit 45fbbe8

Browse files
authored
Switch back to the viewer when clicking a recording URI (#9064)
### Related - Follow-up to #9018 ### What So far, clicking on a recording URI in the redap browser would silently load the recording in the background. With this PR, the UI switches back to viewer mode whenever a new recording is loaded, unless the user cmd/ctrl- or middle-clicked the link, in which case the UI remains in redap browser mode (mimicking the load tab in the background behaviour of browser). This involved a number of changes: - `RecordingUri` is now removed, because the default `arrow_ui` handling of links is sufficient. - `rerun://` links are globally handled by the viewer, taking into account the same-tab/new-tab behaviour in egui `OpenUrl` event. - `ui.re_hyperlink` can now be specified is the new-tab behaviour is forced or not. It is the case for our documentation link, but not for generic urls (e.g. from `arrow_ui()` - Some cleanup around `redap::Scheme` happened.
1 parent b64aaf3 commit 45fbbe8

File tree

43 files changed

+215
-452
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

43 files changed

+215
-452
lines changed

Cargo.lock

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5910,10 +5910,8 @@ dependencies = [
59105910
"egui_plot",
59115911
"itertools 0.13.0",
59125912
"nohash-hasher",
5913-
"re_data_source",
59145913
"re_data_ui",
59155914
"re_format",
5916-
"re_log",
59175915
"re_log_types",
59185916
"re_tracing",
59195917
"re_types",
@@ -6447,7 +6445,6 @@ dependencies = [
64476445
"re_log_types",
64486446
"re_protos",
64496447
"re_sorbet",
6450-
"re_types",
64516448
"re_types_core",
64526449
"re_ui",
64536450
"re_view_dataframe",
@@ -7220,6 +7217,7 @@ dependencies = [
72207217
"re_data_source",
72217218
"re_entity_db",
72227219
"re_format",
7220+
"re_grpc_client",
72237221
"re_log",
72247222
"re_log_types",
72257223
"re_math",

crates/store/re_data_source/src/data_source.rs

Lines changed: 51 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ use re_smart_channel::{Receiver, SmartChannelSource, SmartMessageSource};
44

55
#[cfg(not(target_arch = "wasm32"))]
66
use anyhow::Context as _;
7+
use re_grpc_client::redap;
78

89
/// Somewhere we can get Rerun data from.
910
#[derive(Debug, Clone)]
@@ -49,63 +50,67 @@ impl DataSource {
4950
///
5051
/// Tries to figure out if it looks like a local path,
5152
/// a web-socket address, or a http url.
52-
#[cfg(not(target_arch = "wasm32"))]
53-
pub fn from_uri(file_source: re_log_types::FileSource, mut uri: String) -> Self {
54-
use itertools::Itertools as _;
53+
#[cfg_attr(target_arch = "wasm32", expect(clippy::needless_pass_by_value))]
54+
pub fn from_uri(_file_source: re_log_types::FileSource, mut uri: String) -> Self {
55+
#[cfg(not(target_arch = "wasm32"))]
56+
{
57+
use itertools::Itertools as _;
5558

56-
fn looks_like_windows_abs_path(path: &str) -> bool {
57-
let path = path.as_bytes();
58-
// "C:/" etc
59-
path.get(1).copied() == Some(b':') && path.get(2).copied() == Some(b'/')
60-
}
59+
fn looks_like_windows_abs_path(path: &str) -> bool {
60+
let path = path.as_bytes();
61+
// "C:/" etc
62+
path.get(1).copied() == Some(b':') && path.get(2).copied() == Some(b'/')
63+
}
6164

62-
fn looks_like_a_file_path(uri: &str) -> bool {
63-
// How do we distinguish a file path from a web url? "example.zip" could be either.
65+
fn looks_like_a_file_path(uri: &str) -> bool {
66+
// How do we distinguish a file path from a web url? "example.zip" could be either.
6467

65-
if uri.starts_with('/') {
66-
return true; // Unix absolute path
67-
}
68-
if looks_like_windows_abs_path(uri) {
69-
return true;
68+
if uri.starts_with('/') {
69+
return true; // Unix absolute path
70+
}
71+
if looks_like_windows_abs_path(uri) {
72+
return true;
73+
}
74+
75+
// We use a simple heuristic here: if there are multiple dots, it is likely an url,
76+
// like "example.com/foo.zip".
77+
// If there is only one dot, we treat it as an extension and look it up in a list of common
78+
// file extensions:
79+
80+
let parts = uri.split('.').collect_vec();
81+
if parts.len() <= 1 {
82+
true // No dots. Weird. Let's assume it is a file path.
83+
} else if parts.len() == 2 {
84+
// Extension or `.com` etc?
85+
re_data_loader::is_supported_file_extension(parts[1])
86+
} else {
87+
false // Too many dots; assume an url
88+
}
7089
}
7190

72-
// We use a simple heuristic here: if there are multiple dots, it is likely an url,
73-
// like "example.com/foo.zip".
74-
// If there is only one dot, we treat it as an extension and look it up in a list of common
75-
// file extensions:
76-
77-
let parts = uri.split('.').collect_vec();
78-
if parts.len() <= 1 {
79-
true // No dots. Weird. Let's assume it is a file path.
80-
} else if parts.len() == 2 {
81-
// Extension or `.com` etc?
82-
re_data_loader::is_supported_file_extension(parts[1])
83-
} else {
84-
false // Too many dots; assume an url
91+
// Reading from standard input in non-TTY environments (e.g. GitHub Actions, but I'm sure we can
92+
// come up with more convoluted than that…) can lead to many unexpected,
93+
// platform-specific problems that aren't even necessarily consistent across runs.
94+
//
95+
// In order to avoid having to swallow errors based on unreliable heuristics (or inversely:
96+
// throwing errors when we shouldn't), we just make reading from standard input explicit.
97+
if uri == "-" {
98+
return Self::Stdin;
8599
}
86-
}
87100

88-
// Reading from standard input in non-TTY environments (e.g. GitHub Actions, but I'm sure we can
89-
// come up with more convoluted than that…) can lead to many unexpected,
90-
// platform-specific problems that aren't even necessarily consistent across runs.
91-
//
92-
// In order to avoid having to swallow errors based on unreliable heuristics (or inversely:
93-
// throwing errors when we shouldn't), we just make reading from standard input explicit.
94-
if uri == "-" {
95-
return Self::Stdin;
96-
}
101+
let path = std::path::Path::new(&uri).to_path_buf();
97102

98-
let path = std::path::Path::new(&uri).to_path_buf();
103+
if uri.starts_with("file://") || path.exists() {
104+
return Self::FilePath(_file_source, path);
105+
}
99106

100-
if uri.starts_with("rerun://")
101-
|| uri.starts_with("rerun+http://")
102-
|| uri.starts_with("rerun+https://")
103-
{
104-
return Self::RerunGrpcUrl { url: uri };
107+
if looks_like_a_file_path(&uri) {
108+
return Self::FilePath(_file_source, path);
109+
}
105110
}
106111

107-
if uri.starts_with("file://") || path.exists() {
108-
Self::FilePath(file_source, path)
112+
if redap::Scheme::try_from(uri.as_ref()).is_ok() {
113+
Self::RerunGrpcUrl { url: uri }
109114
} else if (uri.starts_with("http://") || uri.starts_with("https://"))
110115
&& (uri.ends_with(".rrd") || uri.ends_with(".rbl"))
111116
{
@@ -115,8 +120,6 @@ impl DataSource {
115120
}
116121
} else if uri.starts_with("http://") || uri.starts_with("https://") {
117122
Self::MessageProxy { url: uri }
118-
} else if looks_like_a_file_path(&uri) {
119-
Self::FilePath(file_source, path)
120123
} else if uri.ends_with(".rrd") || uri.ends_with(".rbl") {
121124
Self::RrdHttpUrl {
122125
url: uri,

crates/store/re_grpc_client/src/redap/address.rs

Lines changed: 42 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,8 @@ pub enum ConnectionError {
2020
#[error("server is expecting an unencrypted connection (try `rerun+http://` if you are sure)")]
2121
UnencryptedServer,
2222

23-
#[error("invalid or missing scheme (expected `rerun(+http|+https)://`)")]
24-
InvalidScheme,
23+
#[error(transparent)]
24+
InvalidScheme(#[from] InvalidScheme),
2525

2626
#[error("unexpected endpoint: {0}")]
2727
UnexpectedEndpoint(String),
@@ -36,6 +36,10 @@ pub enum ConnectionError {
3636
CannotLoadUrlAsRecording { url: String },
3737
}
3838

39+
#[derive(thiserror::Error, Debug)]
40+
#[error("invalid or missing scheme (expected `rerun(+http|+https)://`)")]
41+
pub struct InvalidScheme;
42+
3943
/// The different schemes supported by Rerun.
4044
///
4145
/// We support `rerun`, `rerun+http`, and `rerun+https`.
@@ -64,6 +68,40 @@ impl Scheme {
6468
Self::RerunHttp => "http",
6569
}
6670
}
71+
72+
/// Converts a rerun url into a canonical http or https url.
73+
fn canonical_url(&self, url: &str) -> String {
74+
match self {
75+
Self::Rerun => {
76+
debug_assert!(url.starts_with("rerun://"));
77+
url.replace("rerun://", "https://")
78+
}
79+
Self::RerunHttp => {
80+
debug_assert!(url.starts_with("rerun+http://"));
81+
url.replace("rerun+http://", "http://")
82+
}
83+
Self::RerunHttps => {
84+
debug_assert!(url.starts_with("rerun+https://"));
85+
url.replace("rerun+https://", "https://")
86+
}
87+
}
88+
}
89+
}
90+
91+
impl TryFrom<&str> for Scheme {
92+
type Error = InvalidScheme;
93+
94+
fn try_from(url: &str) -> Result<Self, Self::Error> {
95+
if url.starts_with("rerun://") {
96+
Ok(Self::Rerun)
97+
} else if url.starts_with("rerun+http://") {
98+
Ok(Self::RerunHttp)
99+
} else if url.starts_with("rerun+https://") {
100+
Ok(Self::RerunHttps)
101+
} else {
102+
Err(InvalidScheme)
103+
}
104+
}
67105
}
68106

69107
#[derive(Debug, PartialEq, Eq, Clone, Hash)]
@@ -139,18 +177,8 @@ impl Origin {
139177
/// Parses a URL and returns the [`Origin`] and the canonical URL (i.e. one that
140178
/// starts with `http://` or `https://`).
141179
fn replace_and_parse(value: &str) -> Result<(Origin, url::Url), ConnectionError> {
142-
let (scheme, rewritten) = if value.starts_with("rerun://") {
143-
Ok((Scheme::Rerun, value.replace("rerun://", "https://")))
144-
} else if value.starts_with("rerun+http://") {
145-
Ok((Scheme::RerunHttp, value.replace("rerun+http://", "http://")))
146-
} else if value.starts_with("rerun+https://") {
147-
Ok((
148-
Scheme::RerunHttps,
149-
value.replace("rerun+https://", "https://"),
150-
))
151-
} else {
152-
Err(ConnectionError::InvalidScheme)
153-
}?;
180+
let scheme = Scheme::try_from(value)?;
181+
let rewritten = scheme.canonical_url(value);
154182

155183
// We have to first rewrite the endpoint, because `Url` does not allow
156184
// `.set_scheme()` for non-opaque origins, nor does it return a proper

crates/store/re_grpc_client/src/redap/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ use re_protos::{
1919

2020
mod address;
2121

22-
pub use address::{ConnectionError, Origin, RedapAddress};
22+
pub use address::{ConnectionError, InvalidScheme, Origin, RedapAddress, Scheme};
2323

2424
use crate::spawn_future;
2525
use crate::StreamError;

crates/store/re_types/definitions/rerun/components.fbs

Lines changed: 0 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/store/re_types/definitions/rerun/components/recording_uri.fbs

Lines changed: 0 additions & 10 deletions
This file was deleted.

crates/store/re_types/src/components/.gitattributes

Lines changed: 0 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/store/re_types/src/components/mod.rs

Lines changed: 0 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/store/re_types/src/components/recording_uri.rs

Lines changed: 0 additions & 104 deletions
This file was deleted.

0 commit comments

Comments
 (0)