fix merge attributes bug

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

@@ -16,15 +16,24 @@ pub enum Attributes {
 
 impl Attributes {
     pub fn extend(&self, other: Option<Attributes>) -> Attributes {
+        log::debug!("Attribute extend: {:?} with {:?}", self, other);
         let other = other.unwrap_or(Attributes::Empty);
-        match (self, &other) {
+        let result = match (self, &other) {
             (Attributes::Custom(data), Attributes::Custom(o_data)) => {
-                let mut data = data.clone();
-                data.extend(o_data.clone());
-                Attributes::Custom(data)
+                if !data.is_plain() {
+                    let mut data = data.clone();
+                    data.extend(o_data.clone());
+                    Attributes::Custom(data)
+                } else {
+                    Attributes::Custom(data.clone())
+                }
             },
+            (Attributes::Custom(data), _) => Attributes::Custom(data.clone()),
+            // (Attributes::Empty, _) => Attributes::Empty,
             _ => other,
-        }
+        };
+        log::debug!("result {:?}", result);
+        result
     }
     // remove attribute if the value is PLAIN
     // { "k": PLAIN } -> {}
@@ -156,21 +165,29 @@ pub fn compose_attributes(left: &Option<Operation>, right: &Option<Operation>) -
     log::trace!("compose_attributes: a: {:?}, b: {:?}", attr_l, attr_r);
 
     let mut attr = match (&attr_l, &attr_r) {
-        (_, Some(Attributes::Custom(_))) => match &attr_l {
+        (_, Some(Attributes::Custom(_))) => match attr_l {
             None => attr_r.unwrap(),
             Some(_) => attr_l.unwrap().extend(attr_r.clone()),
+            // Some(attr_l) => merge_attributes(attr_l, attr_r),
         },
         (Some(Attributes::Custom(_)), _) => attr_l.unwrap().extend(attr_r),
+        // (Some(Attributes::Custom(_)), _) => merge_attributes(attr_l.unwrap(), attr_r),
         (Some(Attributes::Follow), Some(Attributes::Follow)) => Attributes::Follow,
         _ => Attributes::Empty,
     };
 
     log::trace!("composed_attributes: a: {:?}", attr);
 
-    // remove the attribute if the value is PLAIN
-    attr.remove_plain();
-
-    attr
+    match &mut attr {
+        Attributes::Custom(data) => {
+            data.remove_plain();
+            match data.is_plain() {
+                true => Attributes::Empty,
+                false => attr,
+            }
+        },
+        _ => attr,
+    }
 }
 
 pub fn transform_attributes(
@@ -248,3 +265,16 @@ fn transform_attribute_data(left: AttributesData, right: AttributesData) -> Attr
         });
     result
 }
+
+pub fn merge_attributes(attributes: Attributes, other: Option<Attributes>) -> Attributes {
+    let other = other.unwrap_or(Attributes::Empty);
+    match (&attributes, &other) {
+        (Attributes::Custom(data), Attributes::Custom(o_data)) => {
+            let mut data = data.clone();
+            data.extend(o_data.clone());
+            Attributes::Custom(data)
+        },
+        (Attributes::Custom(data), _) => Attributes::Custom(data.clone()),
+        _ => other,
+    }
+}

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

@@ -472,41 +472,45 @@ impl Delta {
             return inverted;
         }
 
-        let a = |inverted: &mut Delta, op: &Operation, index: usize, op_len: usize| {
-            let ops = other.ops_in_interval(Interval::new(index, op_len));
-            ops.into_iter().for_each(|other_op| match op {
-                Operation::Delete(_) => {
-                    inverted.add(other_op);
-                },
-                Operation::Retain(_) => {},
-                Operation::Insert(_) => {
-                    if !op.is_plain() {
-                        let inverted_attrs =
-                            invert_attributes(op.get_attributes(), other_op.get_attributes());
-                        inverted.retain(other_op.length(), inverted_attrs);
+        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));
+                ops.into_iter().for_each(|other_op| {
+                    match operation {
+                        Operation::Delete(_) => {
+                            inverted.add(other_op);
+                        },
+                        Operation::Retain(_) => {
+                            let inverted_attrs = invert_attributes(
+                                operation.get_attributes(),
+                                other_op.get_attributes(),
+                            );
+                            inverted.retain(other_op.length(), inverted_attrs);
+                        },
+                        Operation::Insert(_) => {
+                            // Impossible to here
+                        },
                     }
-                },
-            });
-        };
+                });
+            };
 
         let mut index = 0;
         for op in &self.ops {
-            let op_len: usize = op.length() as usize;
+            let len: usize = op.length() as usize;
             match op {
                 Operation::Delete(_) => {
-                    a(&mut inverted, op, index, op_len);
-                    index += op_len;
+                    inverted_from_other(&mut inverted, op, index, len);
+                    index += len;
                 },
                 Operation::Retain(_) => {
-                    if op.is_plain() {
-                        inverted.retain(op_len as u64, op.get_attributes());
-                    } else {
-                        a(&mut inverted, op, index, op_len as usize);
+                    match op.is_plain() {
+                        true => inverted.retain(len as u64, op.get_attributes()),
+                        false => inverted_from_other(&mut inverted, op, index, len as usize),
                     }
-                    index += op_len;
+                    index += len;
                 },
-                Operation::Insert(insert) => {
-                    inverted.delete(op_len as u64);
+                Operation::Insert(_) => {
+                    inverted.delete(len as u64);
                 },
             }
         }
@@ -565,16 +569,19 @@ impl Delta {
                 //     }
                 // }
             },
-            Operation::Insert(insert) => match &insert.attributes {
-                Attributes::Follow => {},
-                Attributes::Custom(data) => {
-                    let end = insert.num_chars() as usize;
-                    if interval.contains_range(offset, offset + end) {
-                        attributes_data.extend(data.clone());
-                    }
-                    offset += end;
-                },
-                Attributes::Empty => {},
+            Operation::Insert(insert) => {
+                let end = insert.num_chars() as usize;
+                match &insert.attributes {
+                    Attributes::Follow => {},
+                    Attributes::Custom(data) => {
+                        log::debug!("get attributes from op: {:?} at {:?}", op, interval);
+                        if interval.contains_range(offset, offset + end) {
+                            attributes_data.extend(data.clone());
+                        }
+                    },
+                    Attributes::Empty => {},
+                }
+                offset += end
             },
         });
 

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

@@ -40,10 +40,12 @@ impl Operation {
         match self {
             Operation::Delete(_) => {},
             Operation::Retain(retain) => {
-                retain.attributes.extend(Some(attributes));
+                let a = retain.attributes.extend(Some(attributes));
+                retain.attributes = a;
             },
             Operation::Insert(insert) => {
-                insert.attributes.extend(Some(attributes));
+                let a = insert.attributes.extend(Some(attributes));
+                insert.attributes = a;
             },
         }
     }

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

@@ -174,7 +174,7 @@ fn delta_add_bold_italic2() {
         Italic(0, Interval::new(4, 6), true),
         AssertOpsJson(
             0,
-            r#"[{"insert":"12","attributes":{"bold":"true","italic":"true"}},{"insert":"34","attributes":{"bold":"true"}},{"insert":"56","attributes":{"italic":"true"}}]"#,
+            r#"[{"insert":"12","attributes":{"italic":"true","bold":"true"}},{"insert":"34","attributes":{"bold":"true"}},{"insert":"56","attributes":{"italic":"true","bold":"true"}}]"#,
         ),
     ];
 
@@ -221,7 +221,7 @@ fn delta_add_bold_italic_delete() {
         Delete(0, Interval::new(0, 5)),
         AssertOpsJson(
             0,
-            r#"[{"insert":"67","attributes":{"bold":"true"}},{"insert":"89"}]"#,
+            r#"[{"insert":"67"},{"insert":"89","attributes":{"bold":"true"}}]"#,
         ),
     ];
 
@@ -263,18 +263,18 @@ fn delta_compose_attr_delta_with_attr_delta_test2() {
         Italic(0, Interval::new(4, 6), true),
         AssertOpsJson(
             0,
-            r#"[{"insert":"12","attributes":{"bold":"true","italic":"true"}},{"insert":"34","attributes":{"bold":"true"}},{"insert":"56","attributes":{"italic":"true"}}]"#,
+            r#"[{"insert":"12","attributes":{"bold":"true","italic":"true"}},{"insert":"34","attributes":{"bold":"true"}},{"insert":"56","attributes":{"italic":"true","bold":"true"}}]"#,
         ),
         InsertBold(1, "7", Interval::new(0, 1)),
         AssertOpsJson(1, r#"[{"insert":"7","attributes":{"bold":"true"}}]"#),
         Transform(0, 1),
         AssertOpsJson(
             0,
-            r#"[{"insert":"12","attributes":{"italic":"true","bold":"true"}},{"insert":"34","attributes":{"bold":"true"}},{"insert":"56","attributes":{"italic":"true"}},{"insert":"7","attributes":{"bold":"true"}}]"#,
+            r#"[{"insert":"12","attributes":{"italic":"true","bold":"true"}},{"insert":"34","attributes":{"bold":"true"}},{"insert":"56","attributes":{"italic":"true","bold":"true"}},{"insert":"7","attributes":{"bold":"true"}}]"#,
         ),
         AssertOpsJson(
             1,
-            r#"[{"insert":"12","attributes":{"italic":"true","bold":"true"}},{"insert":"34","attributes":{"bold":"true"}},{"insert":"56","attributes":{"italic":"true"}},{"insert":"7","attributes":{"bold":"true"}}]"#,
+            r#"[{"insert":"12","attributes":{"italic":"true","bold":"true"}},{"insert":"34","attributes":{"bold":"true"}},{"insert":"56","attributes":{"italic":"true","bold":"true"}},{"insert":"7","attributes":{"bold":"true"}}]"#,
         ),
     ];
 

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

@@ -91,6 +91,7 @@ impl OpTester {
             },
 
             TestOp::AssertOpsJson(delta_i, expected) => {
+                log::debug!("AssertOpsJson: {:?}", self.deltas[*delta_i]);
                 let delta_i_json = serde_json::to_string(&self.deltas[*delta_i]).unwrap();
 
                 let expected_delta: Delta = serde_json::from_str(expected).unwrap();
@@ -140,8 +141,13 @@ impl OpTester {
             .attributes(attributes)
             .build();
 
-        let attributes = old_delta.attributes_in_interval(*interval);
-        retain.extend_attributes(attributes);
+        let attributes_in_interval = old_delta.attributes_in_interval(*interval);
+        retain.extend_attributes(attributes_in_interval);
+        log::debug!(
+            "Update delta with attributes: {:?} at: {:?}",
+            retain,
+            interval
+        );
 
         let new_delta = new_delta_with_op(old_delta, retain, *interval);
         self.deltas[delta_index] = new_delta;
@@ -149,10 +155,7 @@ impl OpTester {
 
     pub fn update_delta_with_delete(&mut self, delta_index: usize, interval: &Interval) {
         let old_delta = &self.deltas[delta_index];
-        let mut delete = OpBuilder::delete(interval.size() as u64).build();
-        let attributes = old_delta.attributes_in_interval(*interval);
-        delete.extend_attributes(attributes);
-
+        let delete = OpBuilder::delete(interval.size() as u64).build();
         let new_delta = new_delta_with_op(old_delta, delete, *interval);
         self.deltas[delta_index] = new_delta;
     }
@@ -165,9 +168,14 @@ fn new_delta_with_op(delta: &Delta, op: Operation, interval: Interval) -> Delta
     // prefix
     if prefix.is_empty() == false && prefix != interval {
         let intervals = split_interval_with_delta(delta, &prefix);
-        intervals.into_iter().for_each(|interval| {
-            let attributes = delta.attributes_in_interval(interval);
-            new_delta.retain(interval.size() as u64, attributes);
+        intervals.into_iter().for_each(|p_interval| {
+            let attributes = delta.attributes_in_interval(p_interval);
+            log::debug!(
+                "prefix attribute: {:?}, interval: {:?}",
+                attributes,
+                p_interval
+            );
+            new_delta.retain(p_interval.size() as u64, attributes);
         });
     }
 
@@ -176,9 +184,14 @@ fn new_delta_with_op(delta: &Delta, op: Operation, interval: Interval) -> Delta
     // suffix
     if suffix.is_empty() == false {
         let intervals = split_interval_with_delta(delta, &suffix);
-        intervals.into_iter().for_each(|interval| {
-            let attributes = delta.attributes_in_interval(interval);
-            new_delta.retain(interval.size() as u64, attributes);
+        intervals.into_iter().for_each(|s_interval| {
+            let attributes = delta.attributes_in_interval(s_interval);
+            log::debug!(
+                "suffix attribute: {:?}, interval: {:?}",
+                attributes,
+                s_interval
+            );
+            new_delta.retain(s_interval.size() as u64, attributes);
         });
     }
 

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

@@ -21,10 +21,43 @@ fn delta_invert_delta_test() {
 #[test]
 fn delta_invert_delta_test2() {
     let ops = vec![
-        Insert(0, "1234", 0),
+        Insert(0, "123", 0),
         Insert(1, "4567", 0),
         Invert(0, 1),
-        AssertOpsJson(0, r#"[{"insert":"1234"}]"#),
+        AssertOpsJson(0, r#"[{"insert":"123"}]"#),
+    ];
+    OpTester::new().run_script(ops);
+}
+
+#[test]
+fn delta_invert_delta_with_attribute() {
+    let ops = vec![
+        Insert(0, "123", 0),
+        Bold(0, Interval::new(0, 3), true),
+        AssertOpsJson(0, r#"[{"insert":"123","attributes":{"bold":"true"}}]"#),
+        Insert(1, "4567", 0),
+        Invert(0, 1),
+        AssertOpsJson(0, r#"[{"insert":"123","attributes":{"bold":"true"}}]"#),
+    ];
+    OpTester::new().run_script(ops);
+}
+
+#[test]
+fn delta_invert_delta() {
+    let ops = vec![
+        Insert(0, "123", 0),
+        Bold(0, Interval::new(0, 3), true),
+        Insert(0, "456", 3),
+        AssertOpsJson(0, r#"[{"insert":"123456","attributes":{"bold":"true"}}]"#),
+        Italic(0, Interval::new(2, 4), true),
+        AssertOpsJson(
+            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"}}]"#), */
     ];
     OpTester::new().run_script(ops);
 }