From 2b88532812431f36fb2d1551e5b4db21397c6a78 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Thu, 14 Dec 2023 16:00:47 +0100 Subject: [PATCH 01/10] Always format paths with a leading slash --- crates/re_log_types/src/path/entity_path_impl.rs | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/crates/re_log_types/src/path/entity_path_impl.rs b/crates/re_log_types/src/path/entity_path_impl.rs index 1ee1cb608877..48b72055ced3 100644 --- a/crates/re_log_types/src/path/entity_path_impl.rs +++ b/crates/re_log_types/src/path/entity_path_impl.rs @@ -83,8 +83,8 @@ where impl std::fmt::Debug for EntityPathImpl { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - // Like `Display`, but with quotes - write!(f, "'{self}'") + // Same as `Display` - since we always prefix paths with a slash, they are easily recognizable. + write!(f, "{self}") } } @@ -92,17 +92,15 @@ impl std::fmt::Display for EntityPathImpl { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { use std::fmt::Write as _; - let mut iter = self.iter(); - if let Some(first_comp) = iter.next() { - // no leading nor trailing slash - first_comp.fmt(f)?; - for comp in iter { + if self.is_root() { + f.write_char('/') + } else { + // We always lead with a slash + for comp in self.iter() { f.write_char('/')?; comp.fmt(f)?; } Ok(()) - } else { - f.write_char('/') // root } } } From 870dc1ad36b7d446f54b95e2f863f1594f98852d Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Thu, 14 Dec 2023 16:04:11 +0100 Subject: [PATCH 02/10] Remove warnings about non-normalized paths --- crates/re_log_types/src/path/parse_path.rs | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/crates/re_log_types/src/path/parse_path.rs b/crates/re_log_types/src/path/parse_path.rs index 27a42898f29a..558203b9262d 100644 --- a/crates/re_log_types/src/path/parse_path.rs +++ b/crates/re_log_types/src/path/parse_path.rs @@ -160,11 +160,6 @@ impl EntityPath { return Err(PathParseError::UnexpectedComponentName(component_name)); } - let normalized = entity_path.to_string(); - if normalized != input { - re_log::warn_once!("The entity path '{input}' was not in the normalized form. It will be interpreted as '{normalized}'. See https://www.rerun.io/docs/concepts/entity-path for more"); - } - Ok(entity_path) } @@ -173,14 +168,7 @@ impl EntityPath { /// For a strict parses, use [`Self::parse_strict`] instead. pub fn parse_forgiving(input: &str) -> Self { let parts = parse_entity_path_forgiving(input); - let entity_path = EntityPath::from(parts); - - let normalized = entity_path.to_string(); - if normalized != input { - re_log::warn_once!("The entity path '{input}' was not in the normalized form. It will be interpreted as '{normalized}'. See https://www.rerun.io/docs/concepts/entity-path for more"); - } - - entity_path + EntityPath::from(parts) } } From 6454a3cccbb75b0f729442cd2d0372ade9675488 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Thu, 14 Dec 2023 16:04:28 +0100 Subject: [PATCH 03/10] Allow leading slash when parsing entity paths --- crates/re_log_types/src/path/parse_path.rs | 31 +++++++++++----------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/crates/re_log_types/src/path/parse_path.rs b/crates/re_log_types/src/path/parse_path.rs index 558203b9262d..01bbe2d1a26c 100644 --- a/crates/re_log_types/src/path/parse_path.rs +++ b/crates/re_log_types/src/path/parse_path.rs @@ -12,9 +12,6 @@ pub enum PathParseError { #[error("No entity path found")] MissingPath, - #[error("Path had leading slash")] - LeadingSlash, - #[error("Double-slashes with no part between")] DoubleSlash, @@ -63,10 +60,12 @@ impl std::str::FromStr for DataPath { /// For instance: /// - /// * `world/points` - /// * `world/points:Color` - /// * `world/points[#42]` - /// * `world/points[#42]:rerun.components.Color` + /// * `/world/points` + /// * `/world/points:Color` + /// * `/world/points[#42]` + /// * `/world/points[#42]:rerun.components.Color` + /// + /// (the leadign slash is optional) fn from_str(path: &str) -> Result { if path.is_empty() { return Err(PathParseError::EmptyString); @@ -219,7 +218,8 @@ fn entity_path_parts_from_tokens_strict(mut tokens: &[&str]) -> Result Date: Thu, 14 Dec 2023 16:15:18 +0100 Subject: [PATCH 04/10] Only warn about unknown escape sequences when parsing entity paths --- .../re_log_types/src/path/entity_path_part.rs | 23 +++++++++++- crates/re_log_types/src/path/parse_path.rs | 35 +++++++++++-------- 2 files changed, 43 insertions(+), 15 deletions(-) diff --git a/crates/re_log_types/src/path/entity_path_part.rs b/crates/re_log_types/src/path/entity_path_part.rs index dba1f0936f09..d84fb726e755 100644 --- a/crates/re_log_types/src/path/entity_path_part.rs +++ b/crates/re_log_types/src/path/entity_path_part.rs @@ -27,8 +27,19 @@ impl EntityPathPart { /// Unescape the string, forgiving any syntax error with a best-effort approach. pub fn parse_forgiving(input: &str) -> Self { + Self::parse_forgiving_with_warning(input, None) + } + + /// Unescape the string, forgiving any syntax error with a best-effort approach. + /// + /// Returns a warnings if there was any unknown escape sequences. + pub fn parse_forgiving_with_warning( + input: &str, + mut warnings: Option<&mut Vec>, + ) -> Self { let mut output = String::with_capacity(input.len()); let mut chars = input.chars(); + while let Some(c) = chars.next() { if c == '\\' { if let Some(c) = chars.next() { @@ -55,7 +66,17 @@ impl EntityPathPart { } }; } - _ => output.push(c), + c if c.is_ascii_punctuation() || c == ' ' => { + output.push(c); + } + _ => { + if let Some(warnings) = warnings.as_mut() { + // We want to warn on this, because it could be a serious mistake, like + // passing a windows file path (`C:\Users\image.jpg`) as an entity path + warnings.push(format!("Unknown escape sequence: '\\{c}'")); + } + output.push(c); + } } } else { // Trailing escape: treat it as a (escaped) backslash diff --git a/crates/re_log_types/src/path/parse_path.rs b/crates/re_log_types/src/path/parse_path.rs index 01bbe2d1a26c..9da0878dd8f2 100644 --- a/crates/re_log_types/src/path/parse_path.rs +++ b/crates/re_log_types/src/path/parse_path.rs @@ -164,10 +164,29 @@ impl EntityPath { /// Parses an entity path, handling any malformed input with a logged warning. /// + /// Things like `foo/Hallå Där!` will be accepted, and transformed into + /// the path `foo/Hallå\ Där\!`. + /// /// For a strict parses, use [`Self::parse_strict`] instead. pub fn parse_forgiving(input: &str) -> Self { - let parts = parse_entity_path_forgiving(input); - EntityPath::from(parts) + let mut warnings = vec![]; + + let parts: Vec<_> = tokenize_entity_path(input) + .into_iter() + .filter(|&part| part != "/") // ignore duplicate slashes + .map(|part| EntityPathPart::parse_forgiving_with_warning(part, Some(&mut warnings))) + .collect(); + + let path = EntityPath::from(parts); + + if let Some(warning) = warnings.first() { + // We want to warn on some things, like + // passing a windows file path (`C:\Users\image.jpg`) as an entity path, + // which would result in a lot of unknown escapes. + re_log::warn_once!("When parsing the entity path {input:?}: {warning}. The path will be interpreted as {path}"); + } + + path } } @@ -196,18 +215,6 @@ impl FromStr for ComponentPath { } } -/// A very forgiving parsing of the given entity path. -/// -/// Things like `foo/Hallå Där!` will be accepted, and transformed into -/// the path `foo/Hallå\ Där\!`. -fn parse_entity_path_forgiving(path: &str) -> Vec { - tokenize_entity_path(path) - .into_iter() - .filter(|&part| part != "/") // ignore duplicate slashes - .map(EntityPathPart::parse_forgiving) - .collect() -} - fn entity_path_parts_from_tokens_strict(mut tokens: &[&str]) -> Result> { if tokens.is_empty() { return Err(PathParseError::MissingPath); From 122bab3674ace8c72172cce277149e0941ae86e5 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Thu, 14 Dec 2023 16:24:56 +0100 Subject: [PATCH 05/10] Remove unnecessary wrapper typ `EntityPathImpl` --- crates/re_log_types/src/path/entity_path.rs | 75 ++++++++----- .../re_log_types/src/path/entity_path_impl.rs | 106 ------------------ crates/re_log_types/src/path/mod.rs | 2 - 3 files changed, 44 insertions(+), 139 deletions(-) delete mode 100644 crates/re_log_types/src/path/entity_path_impl.rs diff --git a/crates/re_log_types/src/path/entity_path.rs b/crates/re_log_types/src/path/entity_path.rs index f219bb922c7c..ea21b38635b7 100644 --- a/crates/re_log_types/src/path/entity_path.rs +++ b/crates/re_log_types/src/path/entity_path.rs @@ -2,7 +2,7 @@ use std::sync::Arc; use re_types_core::SizeBytes; -use crate::{hash::Hash64, path::entity_path_impl::EntityPathImpl, EntityPathPart}; +use crate::{hash::Hash64, EntityPathPart}; // ---------------------------------------------------------------------------- @@ -99,13 +99,13 @@ pub struct EntityPath { // [`Arc`] used for cheap cloning, and to keep down the size of [`EntityPath`]. // We mostly use the hash for lookups and comparisons anyway! - path: Arc, + parts: Arc>, } impl EntityPath { #[inline] pub fn root() -> Self { - Self::from(EntityPathImpl::root()) + Self::from(vec![]) } #[inline] @@ -131,52 +131,52 @@ impl EntityPath { #[inline] pub fn iter(&self) -> impl Iterator { - self.path.iter() + self.parts.iter() } #[inline] pub fn last(&self) -> Option<&EntityPathPart> { - self.path.last() + self.parts.last() } #[inline] pub fn as_slice(&self) -> &[EntityPathPart] { - self.path.as_slice() + self.parts.as_slice() } #[inline] pub fn to_vec(&self) -> Vec { - self.path.to_vec() + self.parts.to_vec() } #[inline] pub fn is_root(&self) -> bool { - self.path.is_root() + self.parts.is_empty() } /// Is this equals to, or a descendant of, the given path. #[inline] pub fn starts_with(&self, prefix: &EntityPath) -> bool { - prefix.len() <= self.len() && self.path.iter().zip(prefix.iter()).all(|(a, b)| a == b) + prefix.len() <= self.len() && self.iter().zip(prefix.iter()).all(|(a, b)| a == b) } /// Is this a strict descendant of the given path. #[inline] pub fn is_descendant_of(&self, other: &EntityPath) -> bool { - other.len() < self.len() && self.path.iter().zip(other.iter()).all(|(a, b)| a == b) + other.len() < self.len() && self.iter().zip(other.iter()).all(|(a, b)| a == b) } /// Is this a direct child of the other path. #[inline] pub fn is_child_of(&self, other: &EntityPath) -> bool { - other.len() + 1 == self.len() && self.path.iter().zip(other.iter()).all(|(a, b)| a == b) + other.len() + 1 == self.len() && self.iter().zip(other.iter()).all(|(a, b)| a == b) } /// Number of parts #[inline] #[allow(clippy::len_without_is_empty)] pub fn len(&self) -> usize { - self.path.len() + self.parts.len() } #[inline] @@ -193,7 +193,10 @@ impl EntityPath { /// Return [`None`] if root. #[must_use] pub fn parent(&self) -> Option { - self.path.parent().map(Self::from) + self.parts + .len() + .checked_sub(1) + .map(|n_minus_1| Self::new(self.parts[..n_minus_1].to_vec())) } pub fn join(&self, other: &Self) -> Self { @@ -231,27 +234,20 @@ impl FromIterator for EntityPath { } } -impl From for EntityPath { +impl From> for EntityPath { #[inline] - fn from(path: EntityPathImpl) -> Self { + fn from(path: Vec) -> Self { Self { hash: EntityPathHash(Hash64::hash(&path)), - path: Arc::new(path), + parts: Arc::new(path), } } } -impl From> for EntityPath { - #[inline] - fn from(path: Vec) -> Self { - Self::from(EntityPathImpl::from(path.iter())) - } -} - impl From<&[EntityPathPart]> for EntityPath { #[inline] fn from(path: &[EntityPathPart]) -> Self { - Self::from(EntityPathImpl::from(path.iter())) + Self::from(path.to_vec()) } } @@ -331,7 +327,7 @@ impl Loggable for EntityPath { re_types_core::datatypes::Utf8::to_arrow( data.into_iter() .map(Into::into) - .map(|ent_path| re_types_core::datatypes::Utf8(ent_path.path.to_string().into())), + .map(|ent_path| re_types_core::datatypes::Utf8(ent_path.to_string().into())), ) } @@ -351,7 +347,7 @@ impl Loggable for EntityPath { impl serde::Serialize for EntityPath { #[inline] fn serialize(&self, serializer: S) -> Result { - self.path.serialize(serializer) + self.parts.serialize(serializer) } } @@ -359,7 +355,8 @@ impl serde::Serialize for EntityPath { impl<'de> serde::Deserialize<'de> for EntityPath { #[inline] fn deserialize>(deserializer: D) -> Result { - EntityPathImpl::deserialize(deserializer).map(Self::from) + let parts = Vec::::deserialize(deserializer)?; + Ok(Self::new(parts)) } } @@ -384,14 +381,16 @@ impl nohash_hasher::IsEnabled for EntityPath {} // ---------------------------------------------------------------------------- impl std::cmp::Ord for EntityPath { + #[inline] fn cmp(&self, other: &Self) -> std::cmp::Ordering { - self.path.cmp(&other.path) + self.parts.cmp(&other.parts) } } impl std::cmp::PartialOrd for EntityPath { + #[inline] fn partial_cmp(&self, other: &Self) -> Option { - Some(self.path.cmp(&other.path)) + Some(self.parts.cmp(&other.parts)) } } @@ -399,16 +398,30 @@ impl std::cmp::PartialOrd for EntityPath { impl std::fmt::Debug for EntityPath { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - self.path.fmt(f) + // Same as `Display` - since we always prefix paths with a slash, they are easily recognizable. + write!(f, "{self}") } } impl std::fmt::Display for EntityPath { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - self.path.fmt(f) + use std::fmt::Write as _; + + if self.is_root() { + f.write_char('/') + } else { + // We always lead with a slash + for comp in self.iter() { + f.write_char('/')?; + comp.fmt(f)?; + } + Ok(()) + } } } +// ---------------------------------------------------------------------------- + #[cfg(test)] mod tests { use super::*; diff --git a/crates/re_log_types/src/path/entity_path_impl.rs b/crates/re_log_types/src/path/entity_path_impl.rs deleted file mode 100644 index 48b72055ced3..000000000000 --- a/crates/re_log_types/src/path/entity_path_impl.rs +++ /dev/null @@ -1,106 +0,0 @@ -use crate::path::EntityPathPart; - -/// `camera/left/points/42` -/// -/// Wrapped by [`crate::EntityPath`] together with a hash. -#[derive(Clone, PartialEq, Eq, Hash, PartialOrd, Ord)] -#[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))] -pub struct EntityPathImpl { - parts: Vec, -} - -impl EntityPathImpl { - #[inline] - pub fn root() -> Self { - Self { parts: vec![] } - } - - #[inline] - pub fn new(parts: Vec) -> Self { - Self { parts } - } - - #[inline] - pub fn as_slice(&self) -> &[EntityPathPart] { - self.parts.as_slice() - } - - #[inline] - pub fn to_vec(&self) -> Vec { - self.parts.clone() - } - - #[inline] - pub fn is_root(&self) -> bool { - self.parts.is_empty() - } - - /// Number of components - #[inline] - #[allow(clippy::len_without_is_empty)] - pub fn len(&self) -> usize { - self.parts.len() - } - - #[inline] - pub fn iter(&self) -> impl Iterator { - self.parts.iter() - } - - #[inline] - pub fn last(&self) -> Option<&EntityPathPart> { - self.parts.last() - } - - #[inline] - pub fn push(&mut self, comp: EntityPathPart) { - self.parts.push(comp); - } - - /// Return [`None`] if root. - #[must_use] - pub fn parent(&self) -> Option { - if self.parts.is_empty() { - None - } else { - Some(Self::new(self.parts[..(self.parts.len() - 1)].to_vec())) - } - } -} - -// ---------------------------------------------------------------------------- - -impl<'a, It> From for EntityPathImpl -where - It: Iterator, -{ - fn from(path: It) -> Self { - Self::new(path.cloned().collect()) - } -} - -// ---------------------------------------------------------------------------- - -impl std::fmt::Debug for EntityPathImpl { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - // Same as `Display` - since we always prefix paths with a slash, they are easily recognizable. - write!(f, "{self}") - } -} - -impl std::fmt::Display for EntityPathImpl { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - use std::fmt::Write as _; - - if self.is_root() { - f.write_char('/') - } else { - // We always lead with a slash - for comp in self.iter() { - f.write_char('/')?; - comp.fmt(f)?; - } - Ok(()) - } - } -} diff --git a/crates/re_log_types/src/path/mod.rs b/crates/re_log_types/src/path/mod.rs index 1f1e6bce8be2..90c7686fdc6a 100644 --- a/crates/re_log_types/src/path/mod.rs +++ b/crates/re_log_types/src/path/mod.rs @@ -8,7 +8,6 @@ mod data_path; mod entity_path; mod entity_path_expr; mod entity_path_filter; -mod entity_path_impl; mod entity_path_part; mod natural_ordering; mod parse_path; @@ -18,7 +17,6 @@ pub use data_path::DataPath; pub use entity_path::{EntityPath, EntityPathHash}; pub use entity_path_expr::EntityPathExpr; pub use entity_path_filter::EntityPathFilter; -pub use entity_path_impl::EntityPathImpl; pub use entity_path_part::EntityPathPart; pub use parse_path::PathParseError; From 37121f06e774226336b4f410c201e963797f33af Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Thu, 14 Dec 2023 16:32:18 +0100 Subject: [PATCH 06/10] Optimization: Use an interned string for `EntityPathPart` --- crates/re_log_types/src/path/entity_path.rs | 3 ++- .../re_log_types/src/path/entity_path_part.rs | 20 +++++++++++++++---- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/crates/re_log_types/src/path/entity_path.rs b/crates/re_log_types/src/path/entity_path.rs index ea21b38635b7..148a0e5a2540 100644 --- a/crates/re_log_types/src/path/entity_path.rs +++ b/crates/re_log_types/src/path/entity_path.rs @@ -1,5 +1,6 @@ use std::sync::Arc; +use re_string_interner::InternedString; use re_types_core::SizeBytes; use crate::{hash::Hash64, EntityPathPart}; @@ -125,7 +126,7 @@ impl EntityPath { /// Treat the string as one opaque string, NOT splitting on any slashes. /// /// The given string is expected to be unescaped, i.e. any `\` is treated as a normal character. - pub fn from_single_string(string: impl Into) -> Self { + pub fn from_single_string(string: impl Into) -> Self { Self::new(vec![EntityPathPart::new(string)]) } diff --git a/crates/re_log_types/src/path/entity_path_part.rs b/crates/re_log_types/src/path/entity_path_part.rs index d84fb726e755..81b72fd5dd2a 100644 --- a/crates/re_log_types/src/path/entity_path_part.rs +++ b/crates/re_log_types/src/path/entity_path_part.rs @@ -1,3 +1,5 @@ +use re_string_interner::InternedString; + use crate::PathParseError; /// The different parts that make up an [`EntityPath`][crate::EntityPath]. @@ -14,14 +16,17 @@ use crate::PathParseError; #[derive(Clone, Debug, Hash, PartialEq, Eq)] #[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))] pub struct EntityPathPart( - // TODO(emilk): consider other string types; e.g. interned strings, `Arc`, … - String, + /// We use an interned string for fast copies, fast hashing, and to save memory. + /// Note that `re_string_interner` never frees memory, but even if a user + /// allocates 100k different entity parts (which is crazy many lot), the memory usage + /// will still only be in the low megabytes. + InternedString, ); impl EntityPathPart { /// The given string is expected to be unescaped, i.e. any `\` is treated as a normal character. #[inline] - pub fn new(unescaped_string: impl Into) -> Self { + pub fn new(unescaped_string: impl Into) -> Self { Self(unescaped_string.into()) } @@ -211,6 +216,13 @@ impl std::fmt::Display for EntityPathPart { } } +impl From for EntityPathPart { + #[inline] + fn from(part: InternedString) -> Self { + Self(part) + } +} + impl From<&str> for EntityPathPart { #[inline] fn from(part: &str) -> Self { @@ -221,7 +233,7 @@ impl From<&str> for EntityPathPart { impl From for EntityPathPart { #[inline] fn from(part: String) -> Self { - Self(part) + Self(part.into()) } } From fc1398c1532719b5e24bca5ad4fbbdaa82eb9c9b Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Thu, 14 Dec 2023 16:34:45 +0100 Subject: [PATCH 07/10] Update docs --- docs/content/concepts/entity-path.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/content/concepts/entity-path.md b/docs/content/concepts/entity-path.md index 9a5d9adcab25..df08c29a61f9 100644 --- a/docs/content/concepts/entity-path.md +++ b/docs/content/concepts/entity-path.md @@ -15,7 +15,7 @@ There _are_ valid reasons to logs different kinds of archetypes to the same enti Rerun treats entity paths as being arranged in a hierarchy with the `/` character acting as a separator between path elements. The conventional path semantics including concepts of *root* and *parent*/*child* generally apply. -When writing paths in logging APIs the leading `/` is omitted. +When writing paths in logging APIs the leading `/` is usually omitted. In the file path analogy, each entity is a folder, and a component is a file. This implies that any entity in a hierarchy can contain components. From feffdae1e890def204c985d7d571c62e3d9a8d0a Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Thu, 14 Dec 2023 16:37:40 +0100 Subject: [PATCH 08/10] Only warn about invalid blueprint once --- crates/re_viewer/src/store_hub.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/re_viewer/src/store_hub.rs b/crates/re_viewer/src/store_hub.rs index 414a7d925ff6..154631401fdc 100644 --- a/crates/re_viewer/src/store_hub.rs +++ b/crates/re_viewer/src/store_hub.rs @@ -332,7 +332,7 @@ impl StoreHub { if store.store_kind() == StoreKind::Blueprint && store.app_id() == Some(app_id) { if !is_valid_blueprint(&store) { - re_log::warn!("Blueprint for {app_id} appears invalid - restoring to default. This is expected if you have just upgraded Rerun versions."); + re_log::warn_once!("Blueprint for {app_id} appears invalid - restoring to default. This is expected if you have just upgraded Rerun versions."); continue; } // We found the blueprint we were looking for; make it active. From 21e630918853ae7442a89aacfdea752124e08597 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Thu, 14 Dec 2023 17:04:37 +0100 Subject: [PATCH 09/10] Warn about not escaping whitespace in an entity path --- crates/re_log_types/src/path/entity_path_part.rs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/crates/re_log_types/src/path/entity_path_part.rs b/crates/re_log_types/src/path/entity_path_part.rs index 81b72fd5dd2a..2998d13c6be4 100644 --- a/crates/re_log_types/src/path/entity_path_part.rs +++ b/crates/re_log_types/src/path/entity_path_part.rs @@ -37,7 +37,9 @@ impl EntityPathPart { /// Unescape the string, forgiving any syntax error with a best-effort approach. /// - /// Returns a warnings if there was any unknown escape sequences. + /// Returns a warnings for potentially serious problems: + /// * any unknown escape sequences + /// * unescaped spaces pub fn parse_forgiving_with_warning( input: &str, mut warnings: Option<&mut Vec>, @@ -88,6 +90,13 @@ impl EntityPathPart { output.push('\\'); } } else { + if c.is_whitespace() { + if let Some(warnings) = warnings.as_mut() { + // This could be a sign of forgetting to split a string containing multiple entity paths. + warnings.push("Unescaped whitespace".to_owned()); + } + } + output.push(c); } } From 14fdfa3f9b4b959f765ea0ed81c0c32ca5697bbd Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Thu, 14 Dec 2023 17:29:48 +0100 Subject: [PATCH 10/10] fix test --- .../re_space_view/src/data_query_blueprint.rs | 54 +++++++++---------- 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/crates/re_space_view/src/data_query_blueprint.rs b/crates/re_space_view/src/data_query_blueprint.rs index e7658e9d57be..8139b2f1e45f 100644 --- a/crates/re_space_view/src/data_query_blueprint.rs +++ b/crates/re_space_view/src/data_query_blueprint.rs @@ -630,10 +630,10 @@ mod tests { exclusions: vec![], outputs: vec![ "/", - "parent/", - "parent", - "parent/skipped/", // Not an exact match and not found in tree - "parent/skipped/child1", // Only child 1 has ViewParts + "/parent/", + "/parent", + "/parent/skipped/", // Not an exact match and not found in tree + "/parent/skipped/child1", // Only child 1 has ViewParts ], }, Scenario { @@ -641,9 +641,9 @@ mod tests { exclusions: vec![], outputs: vec![ "/", - "parent/", // Only included because is a prefix - "parent/skipped/", // Not an exact match and not found in tree - "parent/skipped/child1", // Only child 1 has ViewParts + "/parent/", // Only included because is a prefix + "/parent/skipped/", // Not an exact match and not found in tree + "/parent/skipped/child1", // Only child 1 has ViewParts ], }, Scenario { @@ -651,10 +651,10 @@ mod tests { exclusions: vec![], outputs: vec![ "/", // Trivial intermediate group -- could be collapsed - "parent/", - "parent", - "parent/skipped/", // Trivial intermediate group -- could be collapsed - "parent/skipped/child2", + "/parent/", + "/parent", + "/parent/skipped/", // Trivial intermediate group -- could be collapsed + "/parent/skipped/child2", ], }, Scenario { @@ -662,12 +662,12 @@ mod tests { exclusions: vec![], outputs: vec![ "/", - "parent/", - "parent", - "parent/skipped/", - "parent/skipped", // Included because an exact match - "parent/skipped/child1", // Included because an exact match - "parent/skipped/child2", + "/parent/", + "/parent", + "/parent/skipped/", + "/parent/skipped", // Included because an exact match + "/parent/skipped/child1", // Included because an exact match + "/parent/skipped/child2", ], }, Scenario { @@ -675,27 +675,27 @@ mod tests { exclusions: vec!["parent"], outputs: vec![ "/", - "parent/", // Parent leaf has been excluded - "parent/skipped/", - "parent/skipped", // Included because an exact match - "parent/skipped/child1", // Included because an exact match - "parent/skipped/child2", + "/parent/", // Parent leaf has been excluded + "/parent/skipped/", + "/parent/skipped", // Included because an exact match + "/parent/skipped/child1", // Included because an exact match + "/parent/skipped/child2", ], }, Scenario { inclusions: vec!["parent/"], exclusions: vec!["parent/skipped/"], - outputs: vec!["/", "parent"], // None of the children are hit since excluded + outputs: vec!["/", "/parent"], // None of the children are hit since excluded }, Scenario { inclusions: vec!["parent/", "parent/skipped/child2"], exclusions: vec!["parent/skipped/child1"], outputs: vec![ "/", - "parent/", - "parent", - "parent/skipped/", - "parent/skipped/child2", // No child1 since skipped. + "/parent/", + "/parent", + "/parent/skipped/", + "/parent/skipped/child2", // No child1 since skipped. ], }, Scenario {