take the retain attributes when composing (insert, retain)

rust-lib/flowy-ot/Cargo.toml

@@ -9,6 +9,7 @@ edition = "2018"
 bytecount = "0.6.0"
 serde = { version = "1.0", features = ["derive"] }
 serde_json = {version = "1.0"}
+derive_more = {version = "0.99", features = ["display"]}
 log = "0.4"
 
 [dev-dependencies]

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

@@ -182,69 +182,41 @@ pub(crate) fn attributes_from(operation: &Option<Operation>) -> Option<Attribute
     }
 }
 
-pub fn compose_attributes(left: &Option<Operation>, right: &Option<Operation>) -> Attributes {
+pub fn compose_operation(left: &Option<Operation>, right: &Option<Operation>) -> Attributes {
     if left.is_none() && right.is_none() {
         return Attributes::Empty;
     }
     let attr_l = attributes_from(left);
     let attr_r = attributes_from(right);
-    log::trace!("compose_attributes: a: {:?}, b: {:?}", attr_l, attr_r);
 
-    let mut attr = match (&attr_l, &attr_r) {
-        (_, Some(Attributes::Custom(_))) => match attr_l {
-            None => attr_r.unwrap(),
-            Some(attr_l) => merge_attributes(attr_l, attr_r),
-        },
-        (Some(Attributes::Custom(_)), Some(Attributes::Follow))
-        | (Some(Attributes::Custom(_)), Some(Attributes::Custom(_))) => {
-            merge_attributes(attr_l.unwrap(), attr_r)
-        },
-        (Some(Attributes::Follow), Some(Attributes::Follow)) => Attributes::Follow,
-        _ => Attributes::Empty,
-    };
+    if attr_l.is_none() {
+        return attr_r.unwrap();
+    }
 
-    log::trace!("composed_attributes: a: {:?}", attr);
-    attr.apply_rule()
+    if attr_r.is_none() {
+        return attr_l.unwrap();
+    }
+
+    compose_attributes(attr_l.unwrap(), attr_r.unwrap())
 }
 
-pub fn transform_op_attributes(
-    left: &Option<Operation>,
-    right: &Option<Operation>,
-    priority: bool,
-) -> Attributes {
+pub fn transform_operation(left: &Option<Operation>, right: &Option<Operation>) -> Attributes {
     let attr_l = attributes_from(left);
     let attr_r = attributes_from(right);
-    transform_attributes(attr_l, attr_r, priority)
-}
 
-pub fn transform_attributes(
-    left: Option<Attributes>,
-    right: Option<Attributes>,
-    priority: bool,
-) -> Attributes {
-    if left.is_none() {
-        if right.is_none() {
+    if attr_l.is_none() {
+        if attr_r.is_none() {
             return Attributes::Empty;
         }
 
-        return match right.as_ref().unwrap() {
+        return match attr_r.as_ref().unwrap() {
             Attributes::Follow => Attributes::Follow,
-            Attributes::Custom(_) => right.unwrap(),
+            Attributes::Custom(_) => attr_r.unwrap(),
             Attributes::Empty => Attributes::Empty,
         };
     }
 
-    if !priority {
-        return right.unwrap();
-    }
-
-    match (left.unwrap(), right.unwrap()) {
-        (Attributes::Custom(attr_data_l), Attributes::Custom(attr_data_r)) => {
-            let result = transform_attribute_data(attr_data_l, attr_data_r);
-            Attributes::Custom(result)
-        },
-        _ => Attributes::Empty,
-    }
+    transform_attributes(attr_l.unwrap(), attr_r.unwrap())
 }
 
 pub fn invert_attributes(attr: Attributes, base: Attributes) -> Attributes {
@@ -278,20 +250,7 @@ pub fn invert_attributes(attr: Attributes, base: Attributes) -> Attributes {
     return Attributes::Custom(inverted);
 }
 
-fn transform_attribute_data(left: AttributesData, right: AttributesData) -> AttributesData {
-    let result = right
-        .iter()
-        .fold(AttributesData::new(), |mut new_attr_data, (k, v)| {
-            if !left.contains_key(k) {
-                new_attr_data.insert(k.clone(), v.clone());
-            }
-            new_attr_data
-        });
-    result
-}
-
-pub fn merge_attributes(attributes: Attributes, other: Option<Attributes>) -> Attributes {
-    let other = other.unwrap_or(Attributes::Empty);
+pub fn merge_attributes(attributes: Attributes, other: Attributes) -> Attributes {
     match (&attributes, &other) {
         (Attributes::Custom(data), Attributes::Custom(o_data)) => {
             let mut data = data.clone();
@@ -302,3 +261,35 @@ pub fn merge_attributes(attributes: Attributes, other: Option<Attributes>) -> At
         _ => other,
     }
 }
+
+fn compose_attributes(left: Attributes, right: Attributes) -> Attributes {
+    log::trace!("compose_attributes: a: {:?}, b: {:?}", left, right);
+
+    let attr = match (&left, &right) {
+        (_, Attributes::Empty) => Attributes::Empty,
+        (_, Attributes::Custom(_)) => merge_attributes(left, right),
+        (Attributes::Custom(_), _) => merge_attributes(left, right),
+        _ => Attributes::Follow,
+    };
+
+    log::trace!("composed_attributes: a: {:?}", attr);
+    attr.apply_rule()
+}
+
+fn transform_attributes(left: Attributes, right: Attributes) -> Attributes {
+    match (left, right) {
+        (Attributes::Custom(data_l), Attributes::Custom(data_r)) => {
+            let result = data_r
+                .iter()
+                .fold(AttributesData::new(), |mut new_attr_data, (k, v)| {
+                    if !data_l.contains_key(k) {
+                        new_attr_data.insert(k.clone(), v.clone());
+                    }
+                    new_attr_data
+                });
+
+            Attributes::Custom(result)
+        },
+        _ => Attributes::Empty,
+    }
+}

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

@@ -168,7 +168,7 @@ impl Delta {
                     return Err(OTError);
                 },
                 (Some(Operation::Retain(retain)), Some(Operation::Retain(o_retain))) => {
-                    let composed_attrs = compose_attributes(&next_op1, &next_op2);
+                    let composed_attrs = compose_operation(&next_op1, &next_op2);
                     log::debug!(
                         "[retain:{} - retain:{}]: {:?}",
                         retain.n,
@@ -219,7 +219,7 @@ impl Delta {
                     }
                 },
                 (Some(Operation::Insert(insert)), Some(Operation::Retain(o_retain))) => {
-                    let composed_attrs = compose_attributes(&next_op1, &next_op2);
+                    let composed_attrs = compose_operation(&next_op1, &next_op2);
                     log::debug!(
                         "[insert:{} - retain:{}]: {:?}",
                         insert.s,
@@ -231,7 +231,7 @@ impl Delta {
                             new_delta.insert(&insert.s, composed_attrs.clone());
                             next_op2 = Some(
                                 OpBuilder::retain(o_retain.n - insert.num_chars())
-                                    .attributes(composed_attrs.clone())
+                                    .attributes(o_retain.attributes.clone())
                                     .build(),
                             );
                             next_op1 = ops1.next();
@@ -249,7 +249,10 @@ impl Delta {
                             );
                             next_op1 = Some(
                                 OpBuilder::insert(&chars.collect::<String>())
-                                    //maybe o_retain attributes
+                                    // consider this situation: 
+                                    // [insert:12345678 - retain:4], 
+                                    //      the attributes of "5678" should be empty and the following 
+                                    //      retain will bringing the attributes. 
                                     .attributes(Attributes::Empty)
                                     .build(),
                             );
@@ -313,7 +316,7 @@ impl Delta {
                     next_op1 = ops1.next();
                 },
                 (_, Some(Operation::Insert(o_insert))) => {
-                    let composed_attrs = transform_op_attributes(&next_op1, &next_op2, true);
+                    let composed_attrs = transform_operation(&next_op1, &next_op2);
                     a_prime.retain(o_insert.num_chars(), composed_attrs.clone());
                     b_prime.insert(&o_insert.s, composed_attrs);
                     next_op2 = ops2.next();
@@ -325,7 +328,7 @@ impl Delta {
                     return Err(OTError);
                 },
                 (Some(Operation::Retain(retain)), Some(Operation::Retain(o_retain))) => {
-                    let composed_attrs = transform_op_attributes(&next_op1, &next_op2, true);
+                    let composed_attrs = transform_operation(&next_op1, &next_op2);
                     match retain.cmp(&o_retain) {
                         Ordering::Less => {
                             a_prime.retain(retain.n, composed_attrs.clone());
@@ -474,22 +477,29 @@ impl Delta {
         }
 
         let inverted_from_other =
-            |inverted: &mut Delta, operation: &Operation, index: usize, op_len: usize| {
-                let ops = other.ops_in_interval(Interval::new(index, op_len));
+            |inverted: &mut Delta, operation: &Operation, start: usize, end: usize| {
+                let ops = other.ops_in_interval(Interval::new(start, end));
                 ops.into_iter().for_each(|other_op| {
                     match operation {
                         Operation::Delete(_) => {
                             inverted.add(other_op);
                         },
                         Operation::Retain(_) => {
+                            log::debug!(
+                                "Start invert attributes: {:?}, {:?}",
+                                operation.get_attributes(),
+                                other_op.get_attributes()
+                            );
                             let inverted_attrs = invert_attributes(
                                 operation.get_attributes(),
                                 other_op.get_attributes(),
                             );
+                            log::debug!("End invert attributes: {:?}", inverted_attrs);
                             inverted.retain(other_op.length(), inverted_attrs);
                         },
                         Operation::Insert(_) => {
                             // Impossible to here
+                            panic!()
                         },
                     }
                 });
@@ -500,13 +510,13 @@ impl Delta {
             let len: usize = op.length() as usize;
             match op {
                 Operation::Delete(_) => {
-                    inverted_from_other(&mut inverted, op, index, len);
+                    inverted_from_other(&mut inverted, op, index, index + len);
                     index += len;
                 },
                 Operation::Retain(_) => {
                     match op.has_attribute() {
                         true => inverted.retain(len as u64, op.get_attributes()),
-                        false => inverted_from_other(&mut inverted, op, index, len as usize),
+                        false => inverted_from_other(&mut inverted, op, index, index + len),
                     }
                     index += len;
                 },
@@ -575,8 +585,8 @@ impl Delta {
                 match &insert.attributes {
                     Attributes::Follow => {},
                     Attributes::Custom(data) => {
-                        log::debug!("extend attributes from op: {:?} at {:?}", op, interval);
                         if interval.contains_range(offset, offset + end) {
+                            log::debug!("Get attributes from op: {:?} at {:?}", op, interval);
                             attributes_data.extend(data.clone());
                         }
                     },
@@ -588,4 +598,6 @@ impl Delta {
 
         attributes_data.into_attributes()
     }
+
+    pub fn to_json(&self) -> String { serde_json::to_string(self).unwrap_or("".to_owned()) }
 }

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

@@ -1,4 +1,4 @@
-use crate::core::{transform_attributes, Attributes};
+use crate::core::Attributes;
 use bytecount::num_chars;
 use std::{
     fmt,
@@ -145,14 +145,17 @@ impl Retain {
 
         match &attributes {
             Attributes::Follow => {
+                log::debug!("Follow attribute: {:?}", self.attributes);
                 self.n += n;
                 None
             },
             Attributes::Custom(_) | Attributes::Empty => {
                 if self.attributes == attributes {
+                    log::debug!("Attribute equal");
                     self.n += n;
                     None
                 } else {
+                    log::debug!("New retain op");
                     Some(OpBuilder::retain(n).attributes(attributes).build())
                 }
             },

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

@@ -212,18 +212,27 @@ fn delta_add_bold_italic_delete() {
         Insert(0, "123456789", 0),
         Bold(0, Interval::new(0, 5), true),
         Italic(0, Interval::new(0, 2), true),
+        // AssertOpsJson(
+        //     0,
+        //     r#"[{"insert":"12","attributes":{"italic":"true","bold":"true"}},{"insert":"345","
+        // attributes":{"bold":"true"}},{"insert":"6789"}]"#, ),
         Italic(0, Interval::new(2, 4), true),
-        Bold(0, Interval::new(7, 9), true),
-        AssertOpsJson(
-            0,
-            r#"[{"insert":"1234","attributes":{"bold":"true","italic":"true"}},{"insert":"5","attributes":{"bold":"true"}},{"insert":"67"},{"insert":"89","attributes":{"bold":"true"}}]"#,
-        ),
-        Delete(0, Interval::new(0, 5)),
         AssertOpsJson(
             0,
-            r#"[{"insert":"67"},{"insert":"89","attributes":{"bold":"true"}}]"#,
+            r#"[{"insert":"1234","attributes":{"bold":"true","italic":"true"}},{"insert":"5","attributes":{"bold":"true"}},{"insert":"6789"}]"#,
         ),
     ];
+    // Bold(0, Interval::new(7, 9), true),
+    // AssertOpsJson(
+    //     0,
+    //     r#"[{"insert":"1234","attributes":{"bold":"true","italic":"true"}},{"
+    // insert":"5","attributes":{"bold":"true"}},{"insert":"67"},{"insert":"89","
+    // attributes":{"bold":"true"}}]"#, ),
+    // Delete(0, Interval::new(0, 5)),
+    // AssertOpsJson(
+    //     0,
+    //     r#"[{"insert":"67"},{"insert":"89","attributes":{"bold":"true"}}]"#,
+    // ),
 
     OpTester::new().run_script(ops);
 }

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

@@ -1,20 +1,38 @@
+use derive_more::Display;
 use flowy_ot::core::*;
 use rand::{prelude::*, Rng as WrappedRng};
 use std::sync::Once;
 
-#[derive(Clone, Debug)]
+#[derive(Clone, Debug, Display)]
 pub enum TestOp {
+    #[display(fmt = "Insert")]
     Insert(usize, &'static str, usize),
+
     // delta_i, s, start, length,
+    #[display(fmt = "InsertBold")]
     InsertBold(usize, &'static str, Interval),
+
     // delta_i, start, length, enable
+    #[display(fmt = "Bold")]
     Bold(usize, Interval, bool),
+
+    #[display(fmt = "Delete")]
     Delete(usize, Interval),
+
+    #[display(fmt = "Italic")]
     Italic(usize, Interval, bool),
+
+    #[display(fmt = "Transform")]
     Transform(usize, usize),
+
     // invert the delta_a base on the delta_b
+    #[display(fmt = "Invert")]
     Invert(usize, usize),
+
+    #[display(fmt = "AssertStr")]
     AssertStr(usize, &'static str),
+
+    #[display(fmt = "AssertOpsJson")]
     AssertOpsJson(usize, &'static str),
 }
 
@@ -39,6 +57,7 @@ impl OpTester {
     }
 
     pub fn run_op(&mut self, op: &TestOp) {
+        log::debug!("***************** 😈{} *******************", &op);
         match op {
             TestOp::Insert(delta_i, s, index) => {
                 self.update_delta_with_insert(*delta_i, s, *index);
@@ -75,12 +94,21 @@ impl OpTester {
             TestOp::Invert(delta_a_i, delta_b_i) => {
                 let delta_a = &self.deltas[*delta_a_i];
                 let delta_b = &self.deltas[*delta_b_i];
+                log::debug!("Invert: ");
+                log::debug!("a: {}", delta_a.to_json());
+                log::debug!("b: {}", delta_b.to_json());
 
                 let (_, b_prime) = delta_a.transform(delta_b).unwrap();
                 let undo = b_prime.invert_delta(&delta_a);
+
                 let new_delta = delta_a.compose(&b_prime).unwrap();
+                log::debug!("new delta: {}", new_delta.to_json());
+                log::debug!("undo delta: {}", undo.to_json());
+
                 let new_delta_after_undo = new_delta.compose(&undo).unwrap();
 
+                log::debug!("inverted delta a: {}", new_delta_after_undo.to_string());
+
                 assert_eq!(delta_a, &new_delta_after_undo);
 
                 self.deltas[*delta_a_i] = new_delta_after_undo;
@@ -138,6 +166,11 @@ impl OpTester {
     ) {
         let old_delta = &self.deltas[delta_index];
         let old_attributes = old_delta.attributes_in_interval(*interval);
+        log::debug!(
+            "merge attributes: {:?}, with old: {:?}",
+            attributes,
+            old_attributes
+        );
         let new_attributes = match &mut attributes {
             Attributes::Follow => old_attributes,
             Attributes::Custom(attr_data) => {
@@ -147,12 +180,13 @@ impl OpTester {
             Attributes::Empty => Attributes::Empty,
         };
 
+        log::debug!("new attributes: {:?}", new_attributes);
         let retain = OpBuilder::retain(interval.size() as u64)
             .attributes(new_attributes)
             .build();
 
         log::debug!(
-            "Update delta with attributes: {:?} at: {:?}",
+            "Update delta with new attributes: {:?} at: {:?}",
             retain,
             interval
         );
@@ -187,6 +221,7 @@ fn new_delta_with_op(delta: &Delta, op: Operation, interval: Interval) -> Delta
         });
     }
 
+    log::debug!("add new op: {:?}", op);
     new_delta.add(op);
 
     // suffix

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

@@ -54,10 +54,12 @@ fn delta_invert_delta() {
             0,
             r#"[{"insert":"12","attributes":{"bold":"true"}},{"insert":"34","attributes":{"bold":"true","italic":"true"}},{"insert":"56","attributes":{"bold":"true"}}]"#,
         ),
-        /* Insert(1, "4567", 0),
-         *
-         * Invert(0, 1),
-         * AssertOpsJson(0, r#"[{"insert":"123","attributes":{"bold":"true"}}]"#), */
+        Insert(1, "abc", 0),
+        Invert(0, 1),
+        AssertOpsJson(
+            0,
+            r#"[{"insert":"12","attributes":{"bold":"true"}},{"insert":"34","attributes":{"bold":"true","italic":"true"}},{"insert":"56","attributes":{"bold":"true"}}]"#,
+        ),
     ];
     OpTester::new().run_script(ops);
 }