fix warnings

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

@@ -159,9 +159,9 @@ impl Document {
         let new_attributes = match &mut attributes {
             Attributes::Follow => old_attributes,
             Attributes::Custom(attr_data) => {
-                attr_data.extend(old_attributes.data(), true);
+                attr_data.merge(old_attributes.data());
                 log::debug!("combine with old result : {:?}", attr_data);
-                attr_data.clone().into_attributes()
+                attr_data.clone().into()
             },
         };
 
@@ -209,7 +209,7 @@ impl Document {
         // a = c.compose(d)
         log::debug!("👉invert change {}", change);
         let new_delta = self.data.compose(change)?;
-        let mut inverted_delta = change.invert(&self.data);
+        let inverted_delta = change.invert(&self.data);
         // trim(&mut inverted_delta);
 
         Ok((new_delta, inverted_delta))

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

@@ -1,5 +1,22 @@
-use crate::core::{Attribute, AttributesData, AttributesRule, Operation};
-use std::{collections::HashMap, fmt, fmt::Formatter};
+use crate::core::{AttributesData, Operation};
+use std::{fmt, fmt::Formatter};
+
+pub trait AttributesRule {
+    // Remove the empty attribute that its value is empty. e.g. {bold: ""}.
+    fn remove_empty(self) -> Attributes;
+}
+
+impl AttributesRule for Attributes {
+    fn remove_empty(self) -> Attributes {
+        match self {
+            Attributes::Follow => self,
+            Attributes::Custom(mut data) => {
+                data.remove_empty_value();
+                data.into()
+            },
+        }
+    }
+}
 
 #[derive(Debug, PartialEq, Clone, serde::Serialize, serde::Deserialize)]
 #[serde(untagged)]
@@ -49,18 +66,27 @@ pub fn compose_operation(left: &Option<Operation>, right: &Option<Operation>) ->
     if left.is_none() && right.is_none() {
         return Attributes::default();
     }
-    let attr_l = attributes_from(left);
-    let attr_r = attributes_from(right);
+    let attr_left = attributes_from(left);
+    let attr_right = attributes_from(right);
 
-    if attr_l.is_none() {
-        return attr_r.unwrap();
+    if attr_left.is_none() {
+        return attr_right.unwrap();
     }
 
-    if attr_r.is_none() {
-        return attr_l.unwrap();
+    if attr_right.is_none() {
+        return attr_left.unwrap();
     }
 
-    compose_attributes(attr_l.unwrap(), attr_r.unwrap())
+    let left = attr_left.unwrap();
+    let right = attr_right.unwrap();
+    log::trace!("compose attributes: a: {:?}, b: {:?}", left, right);
+    let attr = match (&left, &right) {
+        (_, Attributes::Custom(_)) => merge_attributes(left, right),
+        (Attributes::Custom(_), _) => merge_attributes(left, right),
+        _ => Attributes::Follow,
+    };
+    log::trace!("compose attributes result: {:?}", attr);
+    attr
 }
 
 pub fn transform_operation(left: &Option<Operation>, right: &Option<Operation>) -> Attributes {
@@ -78,7 +104,23 @@ pub fn transform_operation(left: &Option<Operation>, right: &Option<Operation>)
         };
     }
 
-    transform_attributes(attr_l.unwrap(), attr_r.unwrap())
+    let left = attr_l.unwrap();
+    let right = attr_r.unwrap();
+    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::default(),
+    }
 }
 
 pub fn invert_attributes(attr: Attributes, base: Attributes) -> Attributes {
@@ -115,41 +157,10 @@ 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(Some(o_data.clone()), false);
+            data.extend(Some(o_data.clone()));
             Attributes::Custom(data)
         },
         (Attributes::Custom(data), _) => Attributes::Custom(data.clone()),
         _ => other,
     }
 }
-
-fn compose_attributes(left: Attributes, right: Attributes) -> Attributes {
-    log::trace!("compose_attributes: a: {:?}, b: {:?}", left, right);
-
-    let attr = match (&left, &right) {
-        (_, Attributes::Custom(_)) => merge_attributes(left, right),
-        (Attributes::Custom(_), _) => merge_attributes(left, right),
-        _ => Attributes::Follow,
-    };
-
-    log::trace!("composed_attributes: a: {:?}", attr);
-    attr
-}
-
-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::default(),
-    }
-}

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

@@ -24,71 +24,38 @@ impl AttributesData {
         }
     }
 
-    pub fn is_plain(&self) -> bool { self.inner.is_empty() }
+    pub fn is_empty(&self) -> bool { self.inner.is_empty() }
+
+    pub fn add(&mut self, attribute: Attribute) { self.inner.insert(attribute, "true".to_owned()); }
 
     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()); }
+    // Remove the key if its value is empty. e.g. { bold: "" }
+    pub fn remove_empty_value(&mut self) { self.inner.retain(|_, v| !should_remove(v)); }
 
-    pub fn extend(&mut self, other: Option<AttributesData>, prune: bool) {
+    pub fn extend(&mut self, other: Option<AttributesData>) {
         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());
-                // }
-                new_attributes.insert(k.clone(), v.clone());
-            });
-            self.inner = new_attributes;
-        } else {
-            self.inner.extend(other.unwrap().inner);
-        }
+        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 {
-        if self.is_plain() {
-            Attributes::default()
-        } else {
-            Attributes::Custom(self)
+    // Update self attributes by constructing new attributes from the other if it's
+    // not None and replace the key/value with self key/value.
+    pub fn merge(&mut self, other: Option<AttributesData>) {
+        if other.is_none() {
+            return;
         }
-    }
-}
 
-pub trait AttributesRule {
-    fn apply_rule(self) -> Attributes;
-}
-
-impl AttributesRule for Attributes {
-    fn apply_rule(self) -> Attributes {
-        match self {
-            Attributes::Follow => self,
-            Attributes::Custom(mut data) => {
-                data.apply_rule();
-                data.into_attributes()
-            },
-        }
+        let mut new_attributes = other.unwrap().inner;
+        self.inner.iter().for_each(|(k, v)| {
+            new_attributes.insert(k.clone(), v.clone());
+        });
+        self.inner = new_attributes;
     }
 }
-// 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>;
@@ -99,3 +66,7 @@ impl std::ops::Deref for AttributesData {
 impl std::ops::DerefMut for AttributesData {
     fn deref_mut(&mut self) -> &mut Self::Target { &mut self.inner }
 }
+
+impl std::convert::Into<Attributes> for AttributesData {
+    fn into(self) -> Attributes { Attributes::Custom(self) }
+}

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

@@ -4,7 +4,7 @@ use crate::{
 };
 use bytecount::num_chars;
 use std::{
-    cmp::{max, min, Ordering},
+    cmp::{min, Ordering},
     fmt,
     iter::FromIterator,
     str::FromStr,
@@ -46,7 +46,7 @@ impl fmt::Display for Delta {
         // f.write_str(&serde_json::to_string(self).unwrap_or("".to_owned()))?;
         f.write_str("[ ")?;
         for op in &self.ops {
-            f.write_fmt(format_args!("{} ", op));
+            f.write_fmt(format_args!("{} ", op))?;
         }
         f.write_str("]")?;
         Ok(())
@@ -232,7 +232,7 @@ impl Delta {
                 },
                 (Some(Operation::Insert(insert)), Some(Operation::Retain(o_retain))) => {
                     let mut composed_attrs = compose_operation(&next_op1, &next_op2);
-                    composed_attrs = composed_attrs.apply_rule();
+                    composed_attrs = composed_attrs.remove_empty();
 
                     log::debug!(
                         "compose: [{} - {}], composed_attrs: {}",
@@ -589,35 +589,28 @@ impl Delta {
         log::debug!("Get attributes at {:?}", interval);
         self.ops.iter().for_each(|op| match op {
             Operation::Delete(_n) => {},
-            Operation::Retain(_retain) => {
-                unimplemented!()
-                // if interval.contains(retain.n as usize) {
-                //     match &retain.attributes {
-                //         Attributes::Follow => {},
-                //         Attributes::Custom(data) => {
-                //             attributes_data.extend(data.clone());
-                //         },
-                //         Attributes::Empty => {},
-                //     }
-                // }
+            Operation::Retain(retain) => {
+                if let Attributes::Custom(data) = &retain.attributes {
+                    if interval.contains_range(offset, offset + retain.n) {
+                        log::debug!("extend retain attributes with {} ", &data);
+                        attributes_data.extend(Some(data.clone()));
+                    }
+                }
+                offset += retain.n;
             },
             Operation::Insert(insert) => {
                 let end = insert.num_chars() as usize;
-                match &insert.attributes {
-                    Attributes::Follow => {},
-                    Attributes::Custom(data) => {
-                        if interval.contains_range(offset, offset + end) {
-                            log::debug!("extend attributes with {} ", &data);
-                            attributes_data.extend(Some(data.clone()), false);
-                        }
-                    },
+                if let Attributes::Custom(data) = &insert.attributes {
+                    if interval.contains_range(offset, offset + end) {
+                        log::debug!("extend insert attributes with {} ", &data);
+                        attributes_data.extend(Some(data.clone()));
+                    }
                 }
-                offset += end
+                offset += end;
             },
         });
 
-        attributes_data.apply_rule();
-        let attribute = attributes_data.into_attributes();
+        let attribute: Attributes = attributes_data.into();
         log::debug!("Get attributes result: {} ", &attribute);
         attribute
     }

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

@@ -55,7 +55,7 @@ impl Operation {
     pub fn has_attribute(&self) -> bool {
         match self.get_attributes() {
             Attributes::Follow => false,
-            Attributes::Custom(data) => !data.is_plain(),
+            Attributes::Custom(data) => !data.is_empty(),
         }
     }
 
@@ -96,7 +96,7 @@ impl Operation {
 
 impl fmt::Display for Operation {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        f.write_str("{");
+        f.write_str("{")?;
         match self {
             Operation::Delete(n) => {
                 f.write_fmt(format_args!("delete: {}", n))?;
@@ -108,7 +108,7 @@ impl fmt::Display for Operation {
                 f.write_fmt(format_args!("{}", i))?;
             },
         }
-        f.write_str("}");
+        f.write_str("}")?;
         Ok(())
     }
 }
@@ -159,7 +159,7 @@ impl Retain {
     pub fn is_plain(&self) -> bool {
         match &self.attributes {
             Attributes::Follow => true,
-            Attributes::Custom(data) => data.is_plain(),
+            Attributes::Custom(data) => data.is_empty(),
         }
     }
 }
@@ -250,6 +250,6 @@ impl std::convert::From<&str> for Insert {
 fn is_empty(attributes: &Attributes) -> bool {
     match attributes {
         Attributes::Follow => true,
-        Attributes::Custom(data) => data.is_plain(),
+        Attributes::Custom(data) => data.is_empty(),
     }
 }

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

@@ -1,7 +1,7 @@
 use derive_more::Display;
 use flowy_ot::{client::Document, core::*};
 use rand::{prelude::*, Rng as WrappedRng};
-use std::{sync::Once, thread::sleep_ms, time::Duration};
+use std::{sync::Once, time::Duration};
 
 #[derive(Clone, Debug, Display)]
 pub enum TestOp {