fix invert attribute bugs

rust-lib/flowy-ot/src/client/document.rs

@@ -55,8 +55,8 @@ impl Document {
         enable: bool,
     ) -> Result<(), OTError> {
         let attributes = match enable {
-            true => AttrsBuilder::new().add_attribute(attribute).build(),
-            false => AttrsBuilder::new().remove_attribute(attribute).build(),
+            true => AttrsBuilder::new().add(attribute).build(),
+            false => AttrsBuilder::new().remove(&attribute).build(),
         };
 
         self.update_with_attribute(attributes, interval)
@@ -161,7 +161,7 @@ impl Document {
         let new_attributes = match &mut attributes {
             Attributes::Follow => old_attributes,
             Attributes::Custom(attr_data) => {
-                attr_data.merge(old_attributes.data());
+                attr_data.extend(old_attributes.data(), true);
                 attr_data.clone().into_attributes()
             },
             Attributes::Empty => Attributes::Empty,

rust-lib/flowy-ot/src/core/attributes/attributes.rs

@@ -1,4 +1,4 @@
-use crate::core::{should_remove, Operation};
+use crate::core::{Attribute, AttributesData, AttributesRule, Operation};
 use std::{collections::HashMap, fmt};
 
 #[derive(Debug, PartialEq, Clone, serde::Serialize, serde::Deserialize)]
@@ -50,91 +50,6 @@ impl fmt::Display for Attributes {
     }
 }
 
-#[derive(Debug, Clone, Default, PartialEq, serde::Serialize, serde::Deserialize)]
-pub struct AttributesData {
-    #[serde(skip_serializing_if = "HashMap::is_empty")]
-    #[serde(flatten)]
-    inner: HashMap<String, String>,
-}
-
-impl AttributesData {
-    pub fn new() -> Self {
-        AttributesData {
-            inner: HashMap::new(),
-        }
-    }
-    pub fn is_empty(&self) -> bool {
-        self.inner.values().filter(|v| !should_remove(v)).count() == 0
-    }
-
-    fn remove_empty(&mut self) { self.inner.retain(|_, v| !should_remove(v)); }
-
-    pub fn extend(&mut self, other: AttributesData) { self.inner.extend(other.inner); }
-
-    pub fn merge(&mut self, other: Option<AttributesData>) {
-        if other.is_none() {
-            return;
-        }
-
-        let mut new_attributes = other.unwrap().inner;
-        self.inner.iter().for_each(|(k, v)| {
-            if should_remove(v) {
-                new_attributes.remove(k);
-            } else {
-                new_attributes.insert(k.clone(), v.clone());
-            }
-        });
-        self.inner = new_attributes;
-    }
-}
-
-pub trait AttributesDataRule {
-    fn apply_rule(&mut self);
-
-    fn into_attributes(self) -> Attributes;
-}
-impl AttributesDataRule for AttributesData {
-    fn apply_rule(&mut self) { self.remove_empty(); }
-
-    fn into_attributes(mut self) -> Attributes {
-        self.apply_rule();
-
-        if self.is_empty() {
-            Attributes::Empty
-        } else {
-            Attributes::Custom(self)
-        }
-    }
-}
-
-pub trait AttributesRule {
-    fn apply_rule(self) -> Attributes;
-}
-
-impl AttributesRule for Attributes {
-    fn apply_rule(self) -> Attributes {
-        match self {
-            Attributes::Follow => self,
-            Attributes::Custom(data) => data.into_attributes(),
-            Attributes::Empty => self,
-        }
-    }
-}
-
-impl std::convert::From<HashMap<String, String>> for AttributesData {
-    fn from(attributes: HashMap<String, String>) -> Self { AttributesData { inner: attributes } }
-}
-
-impl std::ops::Deref for AttributesData {
-    type Target = HashMap<String, String>;
-
-    fn deref(&self) -> &Self::Target { &self.inner }
-}
-
-impl std::ops::DerefMut for AttributesData {
-    fn deref_mut(&mut self) -> &mut Self::Target { &mut self.inner }
-}
-
 pub(crate) fn attributes_from(operation: &Option<Operation>) -> Option<Attributes> {
     match operation {
         None => None,
@@ -180,6 +95,7 @@ pub fn transform_operation(left: &Option<Operation>, right: &Option<Operation>)
 }
 
 pub fn invert_attributes(attr: Attributes, base: Attributes) -> Attributes {
+    log::info!("Invert attributes: {:?} : {:?}", attr, base);
     let attr = attr.data();
     let base = base.data();
 
@@ -213,7 +129,7 @@ pub fn merge_attributes(attributes: Attributes, other: Attributes) -> Attributes
     match (&attributes, &other) {
         (Attributes::Custom(data), Attributes::Custom(o_data)) => {
             let mut data = data.clone();
-            data.extend(o_data.clone());
+            data.extend(Some(o_data.clone()), false);
             Attributes::Custom(data)
         },
         (Attributes::Custom(data), _) => Attributes::Custom(data.clone()),

rust-lib/flowy-ot/src/core/attributes/builder.rs

@@ -1,9 +1,8 @@
 use crate::core::{Attributes, AttributesData};
 use derive_more::Display;
-const REMOVE_FLAG: &'static str = "";
-pub(crate) fn should_remove(s: &str) -> bool { s == REMOVE_FLAG }
 
-#[derive(Clone, Display)]
+#[derive(Clone, Debug, Display, Hash, Eq, PartialEq, serde::Serialize, serde::Deserialize)]
+#[serde(rename_all = "camelCase")]
 pub enum Attribute {
     #[display(fmt = "bold")]
     Bold,
@@ -22,29 +21,27 @@ impl AttrsBuilder {
         }
     }
 
-    pub fn add_attribute(mut self, attribute: Attribute) -> Self {
-        self.inner
-            .insert(format!("{}", attribute), "true".to_owned());
+    pub fn add(mut self, attribute: Attribute) -> Self {
+        self.inner.add(attribute);
         self
     }
 
-    pub fn remove_attribute(mut self, attribute: Attribute) -> Self {
-        self.inner
-            .insert(format!("{}", attribute), REMOVE_FLAG.to_owned());
+    pub fn remove(mut self, attribute: &Attribute) -> Self {
+        self.inner.remove(attribute);
         self
     }
 
     pub fn bold(self, bold: bool) -> Self {
         match bold {
-            true => self.add_attribute(Attribute::Bold),
-            false => self.remove_attribute(Attribute::Bold),
+            true => self.add(Attribute::Bold),
+            false => self.remove(&Attribute::Bold),
         }
     }
 
     pub fn italic(self, italic: bool) -> Self {
         match italic {
-            true => self.add_attribute(Attribute::Italic),
-            false => self.remove_attribute(Attribute::Italic),
+            true => self.add(Attribute::Italic),
+            false => self.remove(&Attribute::Italic),
         }
     }
 

rust-lib/flowy-ot/src/core/attributes/data.rs

@@ -0,0 +1,95 @@
+use crate::core::{Attribute, Attributes};
+use std::{collections::HashMap, fmt};
+
+pub(crate) const REMOVE_FLAG: &'static str = "";
+pub(crate) fn should_remove(s: &str) -> bool { s == REMOVE_FLAG }
+
+#[derive(Debug, Clone, Default, PartialEq, serde::Serialize, serde::Deserialize)]
+pub struct AttributesData {
+    #[serde(skip_serializing_if = "HashMap::is_empty")]
+    #[serde(flatten)]
+    pub(crate) inner: HashMap<Attribute, String>,
+}
+
+impl AttributesData {
+    pub fn new() -> Self {
+        AttributesData {
+            inner: HashMap::new(),
+        }
+    }
+    pub fn is_empty(&self) -> bool {
+        self.inner.values().filter(|v| !should_remove(v)).count() == 0
+    }
+
+    pub fn remove(&mut self, attribute: &Attribute) {
+        self.inner.insert(attribute.clone(), REMOVE_FLAG.to_owned());
+    }
+
+    pub fn add(&mut self, attribute: Attribute) { self.inner.insert(attribute, "true".to_owned()); }
+
+    pub fn extend(&mut self, other: Option<AttributesData>, prune: bool) {
+        if other.is_none() {
+            return;
+        }
+
+        if prune {
+            let mut new_attributes = other.unwrap().inner;
+            self.inner.iter().for_each(|(k, v)| {
+                if should_remove(v) {
+                    new_attributes.remove(k);
+                } else {
+                    new_attributes.insert(k.clone(), v.clone());
+                }
+            });
+            self.inner = new_attributes;
+        } else {
+            self.inner.extend(other.unwrap().inner);
+        }
+    }
+}
+
+pub trait AttributesDataRule {
+    fn apply_rule(&mut self);
+
+    fn into_attributes(self) -> Attributes;
+}
+impl AttributesDataRule for AttributesData {
+    fn apply_rule(&mut self) { self.inner.retain(|_, v| !should_remove(v)); }
+
+    fn into_attributes(mut self) -> Attributes {
+        self.apply_rule();
+
+        if self.is_empty() {
+            Attributes::Empty
+        } else {
+            Attributes::Custom(self)
+        }
+    }
+}
+
+pub trait AttributesRule {
+    fn apply_rule(self) -> Attributes;
+}
+
+impl AttributesRule for Attributes {
+    fn apply_rule(self) -> Attributes {
+        match self {
+            Attributes::Follow => self,
+            Attributes::Custom(data) => data.into_attributes(),
+            Attributes::Empty => self,
+        }
+    }
+}
+// impl std::convert::From<HashMap<String, String>> for AttributesData {
+//     fn from(attributes: HashMap<String, String>) -> Self { AttributesData {
+// inner: attributes } } }
+
+impl std::ops::Deref for AttributesData {
+    type Target = HashMap<Attribute, String>;
+
+    fn deref(&self) -> &Self::Target { &self.inner }
+}
+
+impl std::ops::DerefMut for AttributesData {
+    fn deref_mut(&mut self) -> &mut Self::Target { &mut self.inner }
+}

rust-lib/flowy-ot/src/core/attributes/mod.rs

@@ -1,5 +1,7 @@
 mod attributes;
 mod builder;
+mod data;
 
 pub use attributes::*;
 pub use builder::*;
+pub use data::*;

rust-lib/flowy-ot/src/core/delta.rs

@@ -515,6 +515,7 @@ impl Delta {
         let mut index = 0;
         for op in &self.ops {
             let len: usize = op.length() as usize;
+            log::info!("{:?}", op);
             match op {
                 Operation::Delete(_) => {
                     inverted_from_other(&mut inverted, op, index, index + len);
@@ -600,7 +601,7 @@ impl Delta {
                     Attributes::Custom(data) => {
                         if interval.contains_range(offset, offset + end) {
                             log::debug!("Get attributes from op: {:?} at {:?}", op, interval);
-                            attributes_data.extend(data.clone());
+                            attributes_data.extend(Some(data.clone()), false);
                         }
                     },
                     Attributes::Empty => {},

rust-lib/flowy-ot/src/core/operation/operation.rs

@@ -54,7 +54,7 @@ impl Operation {
     pub fn has_attribute(&self) -> bool {
         match self.get_attributes() {
             Attributes::Follow => false,
-            Attributes::Custom(data) => data.is_empty(),
+            Attributes::Custom(data) => !data.is_empty(),
             Attributes::Empty => false,
         }
     }

rust-lib/flowy-ot/tests/helper/mod.rs

@@ -50,7 +50,7 @@ impl OpTester {
     pub fn new() -> Self {
         static INIT: Once = Once::new();
         INIT.call_once(|| {
-            std::env::set_var("RUST_LOG", "info");
+            std::env::set_var("RUST_LOG", "debug");
             env_logger::init();
         });
 

rust-lib/flowy-ot/tests/undo_redo_test.rs

@@ -1,11 +1,11 @@
 pub mod helper;
 
 use crate::helper::{TestOp::*, *};
+use flowy_ot::core::Interval;
 
 #[test]
 fn delta_undo_insert() {
     let ops = vec![
-        //
         Insert(0, "\n", 0),
         Insert(0, "123", 0),
         Undo(0),
@@ -59,3 +59,15 @@ fn delta_redo_insert2() {
     ];
     OpTester::new().run_script(ops);
 }
+
+#[test]
+fn delta_undo_attributes() {
+    let ops = vec![
+        Insert(0, "\n", 0),
+        Insert(0, "123", 0),
+        Bold(0, Interval::new(0, 3), true),
+        Undo(0),
+        AssertOpsJson(0, r#"[{"insert":"123\n"}]"#),
+    ];
+    OpTester::new().run_script(ops);
+}