diff --git a/rslib/src/tags.rs b/rslib/src/tags.rs index 977649f0c..2a0355f14 100644 --- a/rslib/src/tags.rs +++ b/rslib/src/tags.rs @@ -12,11 +12,7 @@ use crate::{ use regex::{NoExpand, Regex, Replacer}; use std::cmp::Ordering; -use std::{ - borrow::Cow, - collections::{HashMap, HashSet}, - iter::Peekable, -}; +use std::{borrow::Cow, collections::HashSet, iter::Peekable}; use unicase::UniCase; #[derive(Debug, Clone)] @@ -120,30 +116,50 @@ fn normalize_tag_name(name: &str) -> String { out.trim_end_matches("::").into() } -fn fill_missing_tags(tags: Vec) -> Vec { - let mut filled_tags: HashMap = HashMap::with_capacity(tags.len()); - for tag in tags.into_iter() { - let name = tag.name.to_owned(); - let split: Vec<&str> = (&tag.name).split("::").collect(); - for i in 0..split.len() - 1 { - let comp = split[0..i + 1].join("::"); - let t = Tag::new(comp.to_owned(), Usn(-1)); - if filled_tags.get(&comp).is_none() { - filled_tags.insert(comp, t); - } - } - if filled_tags.get(&name).is_none() { - filled_tags.insert(name, tag); - } - } - let mut tags: Vec = filled_tags.values().map(|t| (*t).clone()).collect(); - tags.sort_unstable(); - - tags +fn immediate_parent_name(tag_name: UniCase<&str>) -> Option> { + tag_name.rsplitn(2, '\x1f').nth(1).map(UniCase::new) } -fn tags_to_tree(tags: Vec) -> TagTreeNode { - let tags = fill_missing_tags(tags); +/// For the given tag, check if immediate parent exists. If so, add +/// tag and return. +/// If the immediate parent is missing, check and add any missing parents. +/// This should ensure that if an immediate parent is found, all ancestors +/// are guaranteed to already exist. +fn add_self_and_missing_parents<'a, 'b>( + all: &'a mut HashSet>, + missing: &'a mut Vec>, + tag_name: UniCase<&'b str>, +) { + if let Some(parent) = immediate_parent_name(tag_name) { + if !all.contains(&parent) { + missing.push(parent.clone()); + add_self_and_missing_parents(all, missing, parent); + } + } + // finally, add self + all.insert(tag_name); +} + +/// Append any missing parents. Caller must sort afterwards. +fn add_missing_parents(tags: &mut Vec) { + let mut all_names: HashSet> = HashSet::new(); + let mut missing = vec![]; + for tag in &*tags { + add_self_and_missing_parents(&mut all_names, &mut missing, UniCase::new(&tag.name)) + } + let mut missing: Vec<_> = missing + .into_iter() + .map(|n| Tag::new(n.to_string(), Usn(0))) + .collect(); + tags.append(&mut missing); +} + +fn tags_to_tree(mut tags: Vec) -> TagTreeNode { + for tag in &mut tags { + tag.name = tag.name.replace("::", "\x1f"); + } + add_missing_parents(&mut tags); + tags.sort_unstable_by(|a, b| UniCase::new(&a.name).cmp(&UniCase::new(&b.name))); let mut top = TagTreeNode::default(); let mut it = tags.into_iter().peekable(); add_child_nodes(&mut it, &mut top); @@ -153,7 +169,7 @@ fn tags_to_tree(tags: Vec) -> TagTreeNode { fn add_child_nodes(tags: &mut Peekable>, parent: &mut TagTreeNode) { while let Some(tag) = tags.peek() { - let split_name: Vec<_> = tag.name.split("::").collect(); + let split_name: Vec<_> = tag.name.split('\x1f').collect(); match split_name.len() as u32 { l if l <= parent.level => { // next item is at a higher level @@ -477,8 +493,8 @@ mod test { let mut col = open_test_collection(); let nt = col.get_notetype_by_name("Basic")?.unwrap(); let mut note = nt.new_note(); - note.tags.push("foo::a".into()); - note.tags.push("foo::b".into()); + note.tags.push("foo::bar::a".into()); + note.tags.push("foo::bar::b".into()); col.add_note(&mut note, DeckID(1))?; // missing parents are added @@ -487,21 +503,64 @@ mod test { node( "", 0, - vec![node("foo", 1, vec![leaf("a", 2), leaf("b", 2)])] + vec![node( + "foo", + 1, + vec![node("bar", 2, vec![leaf("a", 3), leaf("b", 3)])] + )] ) ); // differing case should result in only one parent case being added - // the first one - *(&mut note.tags[1]) = "FOO::b".into(); col.storage.clear_tags()?; + *(&mut note.tags[0]) = "foo::BAR::a".into(); + *(&mut note.tags[1]) = "FOO::bar::b".into(); col.update_note(&mut note)?; assert_eq!( col.tag_tree()?, node( "", 0, - vec![node("foo", 1, vec![leaf("a", 2), leaf("b", 2)])] + vec![node( + "foo", + 1, + vec![node("BAR", 2, vec![leaf("a", 3), leaf("b", 3)])] + )] + ) + ); + + // things should work even if the immediate parent is not missing + col.storage.clear_tags()?; + *(&mut note.tags[0]) = "foo::bar::baz".into(); + *(&mut note.tags[1]) = "foo::bar::baz::quux".into(); + col.update_note(&mut note)?; + assert_eq!( + col.tag_tree()?, + node( + "", + 0, + vec![node( + "foo", + 1, + vec![node("bar", 2, vec![node("baz", 3, vec![leaf("quux", 4)])])] + )] + ) + ); + + // numbers have a smaller ascii number than ':', so a naive sort on + // '::' would result in one::two being nested under one1. + col.storage.clear_tags()?; + *(&mut note.tags[0]) = "one".into(); + *(&mut note.tags[1]) = "one1".into(); + note.tags.push("one::two".into()); + col.update_note(&mut note)?; + assert_eq!( + col.tag_tree()?, + node( + "", + 0, + vec![node("one", 1, vec![leaf("two", 2)]), leaf("one1", 1)] ) );