Skip to content

Commit

Permalink
Implement fix for duplicate entries in visualisation list. (#5808)
Browse files Browse the repository at this point in the history
Fixes duplicate entries in the visualization chooser. The issue was caused by entries added twice, once matching the `Any` type and once matching the concrete type. Closes #5708.
  • Loading branch information
MichaelMauderer authored Mar 7, 2023
1 parent 299bfd6 commit 3c3d1f0
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 11 deletions.
2 changes: 2 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

18 changes: 11 additions & 7 deletions app/gui/view/graph-editor/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,29 +12,33 @@ analytics = { path = "../../analytics" }
ast = { path = "../../language/ast/impl" }
base64 = "0.13"
bimap = { version = "0.4.0" }
engine-protocol = { path = "../../controller/engine-protocol" }
enso-config = { path = "../../config" }
enso-frp = { path = "../../../../lib/rust/frp" }
enso-prelude = { path = "../../../../lib/rust/prelude" }
engine-protocol = { path = "../../controller/engine-protocol" }
enso-shapely = { path = "../../../../lib/rust/shapely" }
enso-text = { path = "../../../../lib/rust/text" }
ensogl = { path = "../../../../lib/rust/ensogl" }
ensogl-component = { path = "../../../../lib/rust/ensogl/component" }
ensogl-text-msdf = { path = "../../../../lib/rust/ensogl/component/text/src/font/msdf" }
ensogl-hardcoded-theme = { path = "../../../../lib/rust/ensogl/app/theme/hardcoded" }
ensogl-drop-manager = { path = "../../../../lib/rust/ensogl/component/drop-manager" }
ensogl-hardcoded-theme = { path = "../../../../lib/rust/ensogl/app/theme/hardcoded" }
ensogl-text-msdf = { path = "../../../../lib/rust/ensogl/component/text/src/font/msdf" }
failure = { workspace = true }
ordered-float = { workspace = true }
span-tree = { path = "../../language/span-tree" }
indexmap = "1.9.2"
js-sys = { workspace = true }
nalgebra = { workspace = true }
serde_json = { workspace = true }
ordered-float = { workspace = true }
serde = { version = "1.0", features = ["derive"] }
serde-wasm-bindgen = { workspace = true }
serde_json = { workspace = true }
sourcemap = "6.0"
span-tree = { path = "../../language/span-tree" }
uuid = { version = "0.8", features = ["serde", "v4", "wasm-bindgen"] }
wasm-bindgen = { workspace = true }
serde-wasm-bindgen = { workspace = true }

[dependencies.web-sys]
version = "0.3.4"
features = ["TextMetrics", 'CanvasRenderingContext2d', 'HtmlHeadElement']

[dev-dependencies]
wasm-bindgen-test = "0.3.8"
Original file line number Diff line number Diff line change
Expand Up @@ -59,11 +59,13 @@ impl Signature {
/// Generic definition of a visualization. Provides information about the visualization `Signature`,
/// and a way to create new instances.
#[derive(Clone, CloneRef, Derivative)]
#[derivative(Debug)]
#[derivative(Debug, PartialEq, Eq, Hash)]
#[allow(missing_docs)]
pub struct Definition {
pub signature: Signature,
#[derivative(Debug = "ignore")]
#[derivative(PartialEq = "ignore")]
#[derivative(Hash = "ignore")]
pub constructor: Rc<dyn Fn(&Application) -> InstantiationResult>,
}

Expand Down
31 changes: 28 additions & 3 deletions app/gui/view/graph-editor/src/component/visualization/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,14 +66,15 @@ impl Registry {
pub fn valid_sources(&self, tp: &enso::Type) -> Vec<visualization::Definition> {
let type_map = self.type_map.borrow();
let any_type = enso::Type::any();
let mut result: Vec<visualization::Definition> =
type_map.get(tp).cloned().unwrap_or_default();
// IndexSet preserves insertion order.
let mut result: indexmap::IndexSet<visualization::Definition> = default();
result.extend(type_map.get(tp).cloned().unwrap_or_default());
if tp != &any_type {
if let Some(vis_for_any) = type_map.get(&any_type) {
result.extend(vis_for_any.iter().cloned());
}
}
result
result.into_iter().collect()
}

/// Return the `visualization::Definition` that should be used as default for the given type.
Expand Down Expand Up @@ -130,3 +131,27 @@ impl Default for Registry {
Registry::new()
}
}

#[cfg(test)]
mod tests {
use super::*;

use wasm_bindgen_test::wasm_bindgen_test;

fn assert_no_duplicates<T: Eq + Hash + Debug + Clone>(items: &[T]) {
let vec_length = items.len();
let set: HashSet<T> = items.iter().cloned().collect();
assert_eq!(set.len(), vec_length, "Duplicate entries found: {items:?}");
}

#[wasm_bindgen_test]
fn assert_no_duplicate_default_visualisations() {
let registry = Registry::new();
registry.add_default_visualizations();

for entry in registry.type_map.borrow().values() {
let signatures = entry.iter().map(|class| class.signature.clone()).collect_vec();
assert_no_duplicates(&signatures);
}
}
}

0 comments on commit 3c3d1f0

Please sign in to comment.