From fdf41e6b2729a6a5acea06eb0304fa78a58e2949 Mon Sep 17 00:00:00 2001 From: forehalo Date: Fri, 1 Sep 2023 20:19:01 +0800 Subject: [PATCH] refactor: simplify item left/right refs --- y-octo/src/doc/codec/item.rs | 218 ++++++++++----------- y-octo/src/doc/codec/refs.rs | 89 ++++++++- y-octo/src/doc/codec/utils/items.rs | 18 +- y-octo/src/doc/store.rs | 211 ++++++++------------ y-octo/src/doc/types/list/iterator.rs | 2 +- y-octo/src/doc/types/list/mod.rs | 6 +- y-octo/src/doc/types/list/search_marker.rs | 8 +- 7 files changed, 287 insertions(+), 265 deletions(-) diff --git a/y-octo/src/doc/codec/item.rs b/y-octo/src/doc/codec/item.rs index 6f4fdf2..8bfabdb 100644 --- a/y-octo/src/doc/codec/item.rs +++ b/y-octo/src/doc/codec/item.rs @@ -118,10 +118,10 @@ pub(crate) struct Item { pub id: Id, pub origin_left_id: Option, pub origin_right_id: Option, - #[cfg_attr(all(test, not(loom)), proptest(value = "None"))] - pub left: Option, - #[cfg_attr(all(test, not(loom)), proptest(value = "None"))] - pub right: Option, + #[cfg_attr(all(test, not(loom)), proptest(value = "Somr::default()"))] + pub left: Somr, + #[cfg_attr(all(test, not(loom)), proptest(value = "Somr::default()"))] + pub right: Somr, pub parent: Option, pub parent_sub: Option, // make content Arc, so we can share the content between items @@ -143,24 +143,31 @@ impl PartialEq for Item { impl std::fmt::Debug for Item { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.debug_struct("Item") - .field("id", &self.id) + let mut dbg = f.debug_struct("Item"); + dbg.field("id", &self.id) .field("origin_left_id", &self.origin_left_id) - .field("origin_right_id", &self.origin_right_id) - .field("left_id", &self.left.as_ref().map(|i| i.id())) - .field("right_id", &self.right.as_ref().map(|i| i.id())) - .field( - "parent", - &self.parent.as_ref().map(|p| match p { - Parent::Type(_) => "[Type]".to_string(), - Parent::String(name) => format!("Parent({name})"), - Parent::Id(id) => format!("({}, {})", id.client, id.clock), - }), - ) - .field("parent_sub", &self.parent_sub) - .field("content", &self.content) - .field("flags", &self.flags) - .finish() + .field("origin_right_id", &self.origin_right_id); + + if let Some(left) = self.left.get() { + dbg.field("left", &left.id); + } + + if let Some(right) = self.right.get() { + dbg.field("right", &right.id); + } + + dbg.field( + "parent", + &self.parent.as_ref().map(|p| match p { + Parent::Type(_) => "[Type]".to_string(), + Parent::String(name) => format!("Parent({name})"), + Parent::Id(id) => format!("({}, {})", id.client, id.clock), + }), + ) + .field("parent_sub", &self.parent_sub) + .field("content", &self.content) + .field("flags", &self.flags) + .finish() } } @@ -176,8 +183,8 @@ impl Default for Item { id: Id::default(), origin_left_id: None, origin_right_id: None, - left: None, - right: None, + left: Somr::none(), + right: Somr::none(), parent: None, parent_sub: None, content: Arc::new(Content::Deleted(0)), @@ -204,9 +211,9 @@ impl Item { Self { id, origin_left_id: left.get().map(|left| left.last_id()), - left: if left.is_some() { Some(Node::Item(left)) } else { None }, + left, origin_right_id: right.get().map(|right| right.id), - right: if right.is_some() { Some(Node::Item(right)) } else { None }, + right, parent, parent_sub, content: Arc::new(content), @@ -215,15 +222,15 @@ impl Item { } pub fn resolve_parent(&self) -> Option<(Option, Option)> { - if let Some(item) = self.left.as_ref().map(|n| n.as_item()).and_then(|i| i.get().cloned()) { + if let Some(item) = self.left.get() { if item.parent.is_none() { - if let Some(item) = item.right.map(|n| n.as_item()).and_then(|i| i.get().cloned()) { + if let Some(item) = item.right.get() { return Some((item.parent.clone(), item.parent_sub.clone())); } } else { return Some((item.parent.clone(), item.parent_sub.clone())); } - } else if let Some(item) = self.right.as_ref().map(|n| n.as_item()).and_then(|i| i.get().cloned()) { + } else if let Some(item) = self.right.get() { return Some((item.parent.clone(), item.parent_sub.clone())); } None @@ -265,72 +272,57 @@ impl Item { Id::new(client, clock + self.len() - 1) } - pub fn right_item(&self) -> ItemRef { - self.right.as_ref().map(|n| n.as_item()).unwrap_or_default() + pub fn split_at(&self, offset: u64) -> JwstCodecResult<(Self, Self)> { + debug_assert!(offset > 0 && self.len() > 1 && offset < self.len()); + let id = self.id; + let right_id = Id::new(id.client, id.clock + offset); + let (left_content, right_content) = self.content.split(offset)?; + + let left_item = Item::new( + id, + left_content, + // let caller connect left <-> node <-> right + Somr::none(), + Somr::none(), + self.parent.clone(), + self.parent_sub.clone(), + ); + + let right_item = Item::new( + right_id, + right_content, + // let caller connect left <-> node <-> right + Somr::none(), + Somr::none(), + self.parent.clone(), + self.parent_sub.clone(), + ); + + Ok((left_item, right_item)) } - #[allow(dead_code)] - #[cfg(any(debug, test))] - pub(crate) fn print_left(&self) { - let mut ret = vec![format!("Self{}: [{:?}]", self.id, self.content)]; - let mut left = self.left.clone(); + fn get_info(&self) -> u8 { + let mut info = self.content.get_info(); - while let Some(n) = left { - left = n.left(); - if n.deleted() { - continue; - } - match &n { - Node::Item(item) => { - ret.push(format!("{item}")); - } - Node::GC(item) => { - ret.push(format!("GC{}: {}", item.id, item.len)); - break; - } - Node::Skip(item) => { - ret.push(format!("Skip{}: {}", item.id, item.len)); - break; - } - } + if self.origin_left_id.is_some() { + info |= item_flags::ITEM_HAS_LEFT_ID; + } + if self.origin_right_id.is_some() { + info |= item_flags::ITEM_HAS_RIGHT_ID; + } + if self.parent_sub.is_some() { + info |= item_flags::ITEM_HAS_PARENT_SUB; } - ret.reverse(); - println!("{}", ret.join(" <- ")); + info } - #[allow(dead_code)] - #[cfg(any(debug, test))] - pub(crate) fn print_right(&self) { - let mut ret = vec![format!("Self{}: [{:?}]", self.id, self.content)]; - let mut right = self.right.clone(); - - while let Some(n) = right { - right = n.right(); - if n.deleted() { - continue; - } - match &n { - Node::Item(item) => { - ret.push(format!("{item}")); - } - Node::GC(item) => { - ret.push(format!("GC{}: {}", item.id, item.len)); - break; - } - Node::Skip(item) => { - ret.push(format!("Skip{}: {}", item.id, item.len)); - break; - } - } - } - - println!("{}", ret.join(" -> ")); + pub fn is_valid(&self) -> bool { + let has_id = self.origin_left_id.is_some() || self.origin_right_id.is_some(); + !has_id && self.parent.is_some() || has_id && self.parent.is_none() && self.parent_sub.is_none() } -} -impl Item { - pub(crate) fn read(decoder: &mut R, id: Id, info: u8, first_5_bit: u8) -> JwstCodecResult { + pub fn read(decoder: &mut R, id: Id, info: u8, first_5_bit: u8) -> JwstCodecResult { let flags: ItemFlags = info.into(); let has_left_id = flags.check(item_flags::ITEM_HAS_LEFT_ID); let has_right_id = flags.check(item_flags::ITEM_HAS_RIGHT_ID); @@ -374,8 +366,8 @@ impl Item { debug_assert_ne!(first_5_bit, 10); Arc::new(Content::read(decoder, first_5_bit)?) }, - left: None, - right: None, + left: Somr::none(), + right: Somr::none(), flags: ItemFlags::from(0), }; @@ -392,28 +384,7 @@ impl Item { Ok(item) } - fn get_info(&self) -> u8 { - let mut info = self.content.get_info(); - - if self.origin_left_id.is_some() { - info |= item_flags::ITEM_HAS_LEFT_ID; - } - if self.origin_right_id.is_some() { - info |= item_flags::ITEM_HAS_RIGHT_ID; - } - if self.parent_sub.is_some() { - info |= item_flags::ITEM_HAS_PARENT_SUB; - } - - info - } - - pub(crate) fn is_valid(&self) -> bool { - let has_id = self.origin_left_id.is_some() || self.origin_right_id.is_some(); - !has_id && self.parent.is_some() || has_id && self.parent.is_none() && self.parent_sub.is_none() - } - - pub(crate) fn write(&self, encoder: &mut W) -> JwstCodecResult { + pub fn write(&self, encoder: &mut W) -> JwstCodecResult { let info = self.get_info(); let has_not_sibling = info & item_flags::ITEM_HAS_SIBLING == 0; @@ -464,6 +435,35 @@ impl Item { } } +#[allow(dead_code)] +#[cfg(any(debug, test))] +impl Item { + pub fn print_left(&self) { + let mut ret = vec![format!("Self{}: [{:?}]", self.id, self.content)]; + let mut left: Somr = self.left.clone(); + + while let Some(item) = left.get() { + ret.push(format!("{item}")); + left = item.left.clone(); + } + ret.reverse(); + + println!("{}", ret.join(" <- ")); + } + + pub fn print_right(&self) { + let mut ret = vec![format!("Self{}: [{:?}]", self.id, self.content)]; + let mut right = self.right.clone(); + + while let Some(item) = right.get() { + ret.push(format!("{item}")); + right = item.right.clone(); + } + + println!("{}", ret.join(" -> ")); + } +} + #[cfg(test)] mod tests { use proptest::{collection::vec, prelude::*}; diff --git a/y-octo/src/doc/codec/refs.rs b/y-octo/src/doc/codec/refs.rs index ac83196..0dc7daa 100644 --- a/y-octo/src/doc/codec/refs.rs +++ b/y-octo/src/doc/codec/refs.rs @@ -1,3 +1,5 @@ +use sync::Arc; + use super::*; // make fields Copy + Clone without much effort @@ -136,7 +138,7 @@ impl Node { pub fn left(&self) -> Option { if let Node::Item(item) = self { - item.get().and_then(|item| item.left.clone()) + item.get().map(|item| Node::Item(item.left.clone())) } else { None } @@ -144,7 +146,7 @@ impl Node { pub fn right(&self) -> Option { if let Node::Item(item) = self { - item.get().and_then(|item| item.right.clone()) + item.get().map(|item| Node::Item(item.right.clone())) } else { None } @@ -188,10 +190,12 @@ impl Node { } } - pub fn last_id(&self) -> Id { - let Id { client, clock } = self.id(); - - Id::new(client, clock + self.len() - 1) + pub fn last_id(&self) -> Option { + if let Node::Item(item) = self { + item.get().map(|item| item.last_id()) + } else { + None + } } pub fn split_at(&self, offset: u64) -> JwstCodecResult<(Self, Self)> { @@ -238,6 +242,79 @@ impl Node { pub fn deleted(&self) -> bool { self.flags().deleted() } + + pub fn merge(&mut self, right: Self) -> bool { + match (self, right) { + (Node::GC(left), Node::GC(right)) => { + left.len += right.len; + } + (Node::Skip(left), Node::Skip(right)) => { + left.len += right.len; + } + (Node::Item(lref), Node::Item(rref)) => { + let mut litem = unsafe { lref.get_mut_unchecked() }; + let mut ritem = unsafe { rref.get_mut_unchecked() }; + let llen = litem.len(); + + if litem.id.client != ritem.id.client + // not same delete status + || litem.deleted() != ritem.deleted() + // not clock continuous + || litem.id.clock + litem.len() != ritem.id.clock + // not insertion continuous + || Some(litem.last_id()) != ritem.origin_left_id + // not insertion continuous + || litem.origin_right_id != ritem.origin_right_id + // not runtime continuous + || litem.right != rref + { + return false; + } + + match (Arc::make_mut(&mut litem.content), Arc::make_mut(&mut ritem.content)) { + (Content::Deleted(l), Content::Deleted(r)) => { + *l += *r; + } + (Content::Json(l), Content::Json(r)) => { + l.extend(r.drain(0..)); + } + (Content::String(l), Content::String(r)) => { + *l += r; + } + (Content::Any(l), Content::Any(r)) => { + l.extend(r.drain(0..)); + } + _ => { + return false; + } + } + + if let Some(Parent::Type(p)) = &litem.parent { + if let Some(parent) = p.ty_mut() { + if let Some(markers) = &parent.markers { + markers.replace_marker(rref.clone(), lref.clone(), -(llen as i64)); + } + } + } + + if ritem.keep() { + litem.flags.set_keep() + } + + litem.right = ritem.right.clone(); + unsafe { + if litem.right.is_some() { + litem.right.get_mut_unchecked().left = lref.clone(); + } + } + } + _ => { + return false; + } + } + + true + } } impl From> for Somr { diff --git a/y-octo/src/doc/codec/utils/items.rs b/y-octo/src/doc/codec/utils/items.rs index e167710..4b1d219 100644 --- a/y-octo/src/doc/codec/utils/items.rs +++ b/y-octo/src/doc/codec/utils/items.rs @@ -16,17 +16,19 @@ impl ItemBuilder { self } - pub fn left(mut self, left: Option) -> ItemBuilder { - let origin_id = left.as_ref().map(|i| i.last_id()); - self.item.left = left; - self.item.origin_left_id = origin_id; + pub fn left(mut self, left: Somr) -> ItemBuilder { + if let Some(l) = left.get() { + self.item.origin_left_id = Some(l.last_id()); + self.item.left = left; + } self } - pub fn right(mut self, right: Option) -> ItemBuilder { - let origin_id = right.as_ref().map(|i| i.id()); - self.item.right = right; - self.item.origin_right_id = origin_id; + pub fn right(mut self, right: Somr) -> ItemBuilder { + if let Some(r) = right.get() { + self.item.origin_right_id = Some(r.id); + self.item.right = right; + } self } diff --git a/y-octo/src/doc/store.rs b/y-octo/src/doc/store.rs index 97c4c2e..e92f770 100644 --- a/y-octo/src/doc/store.rs +++ b/y-octo/src/doc/store.rs @@ -217,40 +217,43 @@ impl DocStore { let node = items.get(idx).unwrap().clone(); debug_assert!(node.is_item()); - let (left_node, right_node) = node.split_at(diff)?; - match (&node, &left_node, &right_node) { - (Node::Item(raw_left_item), Node::Item(new_left_item), Node::Item(right_item)) => { - // SAFETY: - // we make sure store is the only entry of mutating an item, - // and we already hold mutable reference of store, so it's safe to do so - unsafe { - let mut left_item = raw_left_item.get_mut_unchecked(); - let mut right_item = right_item.get_mut_unchecked(); - left_item.content = new_left_item.get_unchecked().content.clone(); - - // we had the correct left/right content - // now build the references - let right_right_ref = ItemRef::from(&left_item.right); - right_item.left = if right_right_ref.is_some() { - let mut right_right = right_right_ref.get_mut_unchecked(); - right_right.left.replace(right_node.clone()) - } else { - Some(node.clone()) - }; - right_item.right = left_item.right.replace(right_node.clone()); - right_item.origin_left_id = Some(left_item.last_id()); - right_item.origin_right_id = left_item.origin_right_id; + if let Node::Item(item_ref) = &node { + let item = item_ref.get().unwrap(); + + let (left, right) = item.split_at(diff)?; + + let left_ref = Somr::new(left); + let right_ref = Somr::new(right); + + // SAFETY: + // we make sure store is the only entry of mutating an item, + // and we already hold mutable reference of store, so it's safe to do so + unsafe { + let mut left_item = item_ref.get_mut_unchecked(); + let mut right_item = right_ref.get_mut_unchecked(); + left_item.content = left_ref.get_unchecked().content.clone(); + + // we had the correct left/right content + // now build the references + let right_right_ref = left_item.right.clone(); + right_item.left = if right_right_ref.is_some() { + let mut right_right = right_right_ref.get_mut_unchecked(); + mem::replace(&mut right_right.left, right_ref.clone()) + } else { + item_ref.clone() }; - } - _ => { - items[idx] = left_node; - } + right_item.right = mem::replace(&mut left_item.right, right_ref.clone()); + right_item.origin_left_id = Some(left_item.last_id()); + right_item.origin_right_id = left_item.origin_right_id; + }; + + let right = Node::Item(right_ref); + let right_ref = right.clone(); + items.insert(idx + 1, right); + Ok((node, right_ref)) + } else { + Err(JwstCodecError::ItemSplitNotSupport) } - - let right_ref = right_node.clone(); - items.insert(idx + 1, right_node); - - Ok((node, right_ref)) } pub fn split_at_and_get_right>(&mut self, id: I) -> JwstCodecResult { @@ -337,11 +340,21 @@ impl DocStore { /// - [None] means borrow left.parent or right.parent pub fn repair(&mut self, item: &mut Item, store_ref: StoreRef) -> JwstCodecResult { if let Some(left_id) = item.origin_left_id { - item.left = Some(self.split_at_and_get_left(left_id)?); + if let Node::Item(left_ref) = self.split_at_and_get_left(left_id)? { + item.origin_left_id = left_ref.get().map(|left| left.last_id()); + item.left = left_ref; + } else { + item.origin_left_id = None; + } } if let Some(right_id) = item.origin_right_id { - item.right = Some(self.split_at_and_get_right(right_id)?); + if let Node::Item(right_ref) = self.split_at_and_get_right(right_id)? { + item.origin_right_id = right_ref.get().map(|right| right.id); + item.right = right_ref; + } else { + item.origin_right_id = None; + } } match &item.parent { @@ -417,9 +430,9 @@ impl DocStore { pub fn integrate(&mut self, mut node: Node, offset: u64, parent: Option<&mut YType>) -> JwstCodecResult { match &mut node { - Node::Item(item) => { + Node::Item(item_owner_ref) => { assert!( - item.is_owned(), + item_owner_ref.is_owned(), "Required a owned Item type but got an shared reference" ); @@ -427,13 +440,16 @@ impl DocStore { // before we integrate struct into store, // the struct => Arc is owned reference actually, // no one else refer to such item yet, we can safely mutable refer to it now. - let this = &mut *unsafe { item.get_mut_unchecked() }; + let this = &mut *unsafe { item_owner_ref.get_mut_unchecked() }; if offset > 0 { this.id.clock += offset; - let left = self.split_at_and_get_left(Id::new(this.id.client, this.id.clock - 1))?; - this.origin_left_id = Some(left.last_id()); - this.left = Some(left); + if let Node::Item(left_ref) = + self.split_at_and_get_left(Id::new(this.id.client, this.id.clock - 1))? + { + this.origin_left_id = left_ref.get().map(|left| left.last_id()); + this.left = left_ref; + } this.content = Arc::new(this.content.split(offset)?.1); } @@ -448,16 +464,16 @@ impl DocStore { return Ok(()); }; - let mut left: ItemRef = this.left.as_ref().into(); - let mut right: ItemRef = this.right.as_ref().into(); + let mut left = this.left.clone(); + let mut right = this.right.clone(); let right_is_null_or_has_left = match right.get() { None => true, - Some(r) => ItemRef::from(&r.left).is_some(), + Some(r) => r.left.is_some(), }; let left_has_other_right_than_self = match left.get() { - Some(left) => ItemRef::from(&left.right) != right, + Some(left) => left.right != right, _ => false, }; @@ -465,7 +481,7 @@ impl DocStore { if (left.is_none() && right_is_null_or_has_left) || left_has_other_right_than_self { // set the first conflicting item let mut conflict = if let Some(left) = left.get() { - left.right.as_ref().into() + left.right.clone() } else if let Some(parent_sub) = &this.parent_sub { parent.map.get(parent_sub).cloned().unwrap_or(Somr::none()) } else { @@ -507,7 +523,7 @@ impl DocStore { break; } - conflict = ItemRef::from(&conflict_item.right); + conflict = conflict_item.right.clone(); } else { break; } @@ -520,17 +536,18 @@ impl DocStore { unsafe { // SAFETY: we get store write lock, no way the left get dropped by owner let mut left = left.get_mut_unchecked(); - right = left.right.replace(Node::Item(item.clone())).into(); + right = left.right.clone(); + left.right = item_owner_ref.clone(); } - this.left = Some(Node::Item(left)); + this.left = left.clone(); } else { // no left, parent.start = this right = if let Some(parent_sub) = &this.parent_sub { parent.map.get(parent_sub).map(|n| Node::Item(n.clone()).head()).into() } else { - mem::replace(&mut parent.start, item.clone()) + mem::replace(&mut parent.start, item_owner_ref.clone()) }; - this.left = None; + this.left = Somr::none(); } // has right, connect @@ -538,16 +555,16 @@ impl DocStore { unsafe { // SAFETY: we get store write lock, no way the left get dropped by owner let mut right = right.get_mut_unchecked(); - right.left = Some(Node::Item(item.clone())); + right.left = item_owner_ref.clone(); } - this.right = Some(Node::Item(right)) + this.right = right.clone(); } else { // no right, parent.start = this, delete this.left if let Some(parent_sub) = &this.parent_sub { - parent.map.insert(parent_sub.to_string(), item.clone()); + parent.map.insert(parent_sub.to_string(), item_owner_ref.clone()); - if let Some(left) = &this.left { - self.delete_node(left, Some(parent)); + if let Some(left) = this.left.get() { + self.delete_item(left, Some(parent)); } } } @@ -556,7 +573,7 @@ impl DocStore { // should delete if parent_deleted || this.parent_sub.is_some() && this.right.is_some() { - self.delete_node(&Node::Item(item.clone()), Some(parent)); + self.delete_node(&Node::Item(item_owner_ref.clone()), Some(parent)); } else { // adjust parent length if this.parent_sub.is_none() { @@ -607,14 +624,9 @@ impl DocStore { // items in ty are all refs, not owned let mut item_ref = ty.start.clone(); while let Some(item) = item_ref.get() { - // delete_set Self::delete_item_inner(delete_set, item, Some(&mut ty)); - if let Some(right) = &item.right { - item_ref = right.as_item(); - } else { - item_ref = Somr::none(); - } + item_ref = item.right.clone(); } let map_values = ty.map.values().cloned().collect::>(); @@ -857,11 +869,7 @@ impl DocStore { // we need to iter to right first, because we may delete the owned item // by replacing it with [Node::GC] - if let Some(right) = &item.right { - item_ref = right.as_item(); - } else { - item_ref = Somr::none(); - } + item_ref = item.right.clone(); Self::gc_item_by_id(items, id, true)?; } @@ -912,73 +920,8 @@ impl DocStore { let right = nodes.get(pos).unwrap().clone(); let left = nodes.get_mut(pos - 1).unwrap(); - match (left, right) { - (Node::GC(left), Node::GC(right)) => { - left.len += right.len; - } - (Node::Skip(left), Node::Skip(right)) => { - left.len += right.len; - } - (Node::Item(lref), Node::Item(rref)) => { - let mut litem = unsafe { lref.get_mut_unchecked() }; - let mut ritem = unsafe { rref.get_mut_unchecked() }; - let llen = litem.len(); - - if litem.id.client != ritem.id.client - // not same delete status - || litem.deleted() != ritem.deleted() - // not clock continuous - || litem.id.clock + litem.len() != ritem.id.clock - // not insertion continuous - || Some(litem.last_id()) != ritem.origin_left_id - // not insertion continuous - || litem.origin_right_id != ritem.origin_right_id - // not runtime continuous - || litem.right_item() != rref - { - break; - } - - match (Arc::make_mut(&mut litem.content), Arc::make_mut(&mut ritem.content)) { - (Content::Deleted(l), Content::Deleted(r)) => { - *l += *r; - } - (Content::Json(l), Content::Json(r)) => { - l.extend(r.drain(0..)); - } - (Content::String(l), Content::String(r)) => { - *l += r; - } - (Content::Any(l), Content::Any(r)) => { - l.extend(r.drain(0..)); - } - _ => { - break; - } - } - - if let Some(Parent::Type(p)) = &litem.parent { - if let Some(parent) = p.ty_mut() { - if let Some(markers) = &parent.markers { - markers.replace_marker(rref.clone(), lref.clone(), -(llen as i64)); - } - } - } - - if ritem.keep() { - litem.flags.set_keep() - } - - litem.right = ritem.right.clone(); - - if let Some(Node::Item(right)) = &litem.right { - let mut right = unsafe { right.get_mut_unchecked() }; - right.left = Some(Node::Item(lref.clone())) - } - } - _ => { - break; - } + if !left.merge(right) { + break; } pos -= 1; diff --git a/y-octo/src/doc/types/list/iterator.rs b/y-octo/src/doc/types/list/iterator.rs index c74ce88..1908ae1 100644 --- a/y-octo/src/doc/types/list/iterator.rs +++ b/y-octo/src/doc/types/list/iterator.rs @@ -9,7 +9,7 @@ impl Iterator for ListIterator { fn next(&mut self) -> Option { while let Some(item) = self.cur.clone().get() { - let cur = std::mem::replace(&mut self.cur, item.right.clone().into()); + let cur = std::mem::replace(&mut self.cur, item.right.clone()); if item.deleted() { continue; } diff --git a/y-octo/src/doc/types/list/mod.rs b/y-octo/src/doc/types/list/mod.rs index bdd05c6..fc0055a 100644 --- a/y-octo/src/doc/types/list/mod.rs +++ b/y-octo/src/doc/types/list/mod.rs @@ -22,7 +22,7 @@ impl ItemPosition { } self.left = self.right.clone(); - self.right = right.right.clone().into(); + self.right = right.right.clone(); } else { // FAIL } @@ -89,7 +89,7 @@ pub(crate) trait ListType: AsInner { remaining -= marker.index; } pos.index = marker.index; - pos.left = marker.ptr.get().and_then(|ptr| ptr.left.clone()).into(); + pos.left = marker.ptr.get().map(|ptr| ptr.left.clone()).unwrap_or_default(); pos.right = marker.ptr; } }; @@ -108,7 +108,7 @@ pub(crate) trait ListType: AsInner { } pos.left = pos.right.clone(); - pos.right = item.right.clone().into(); + pos.right = item.right.clone(); } else { return None; } diff --git a/y-octo/src/doc/types/list/search_marker.rs b/y-octo/src/doc/types/list/search_marker.rs index 16318e9..e498fd4 100644 --- a/y-octo/src/doc/types/list/search_marker.rs +++ b/y-octo/src/doc/types/list/search_marker.rs @@ -87,7 +87,7 @@ impl MarkerList { if len > 0 { while let Some(ptr) = marker.ptr.get() { if !ptr.indexable() { - let left_ref: ItemRef = ptr.left.as_ref().into(); + let left_ref = ptr.left.clone(); if let Some(left) = left_ref.get() { if left.indexable() { marker.index -= left.len(); @@ -138,7 +138,7 @@ impl MarkerList { break; } - let right_ref: ItemRef = item.right.clone().into(); + let right_ref: ItemRef = item.right.clone(); if right_ref.is_some() { if item.indexable() { if index < marker_index + item.len() { @@ -159,7 +159,7 @@ impl MarkerList { break; } - let left_ref: ItemRef = item.left.clone().into(); + let left_ref: ItemRef = item.left.clone(); if let Some(left) = left_ref.get() { if left.indexable() { marker_index -= left.len(); @@ -175,7 +175,7 @@ impl MarkerList { // (it is most likely the best marker anyway) iterate to left until // item_ptr can't be merged with left while let Some(item) = item_ptr.clone().get() { - let left_ref: ItemRef = item.left.clone().into(); + let left_ref: ItemRef = item.left.clone(); if let Some(left) = left_ref.get() { if left.id.client == item.id.client && left.id.clock + left.len() == item.id.clock { if left.indexable() {