Browse Source

fix warnings

appflowy 3 years ago
parent
commit
c8502151ed

+ 29 - 24
rust-lib/flowy-ot/src/client/document.rs

@@ -1,7 +1,7 @@
 use crate::{
     client::{view::View, History, RevId, UndoResult},
     core::*,
-    errors::{ErrorBuilder, OTError, OTErrorCode::*},
+    errors::{ErrorBuilder, OTError, OTErrorCode, OTErrorCode::*},
 };
 
 pub const RECORD_THRESHOLD: usize = 400; // in milliseconds
@@ -30,26 +30,16 @@ impl Document {
         }
     }
 
-    pub fn insert(
-        &mut self,
-        index: usize,
-        text: &str,
-        replace_len: usize,
-    ) -> Result<Delta, OTError> {
-        if self.delta.target_len < index {
-            log::error!(
-                "{} out of bounds. should 0..{}",
-                index,
-                self.delta.target_len
-            );
-        }
-
-        let delta = self.view.insert(&self.delta, text, index, replace_len)?;
+    pub fn insert(&mut self, index: usize, text: &str) -> Result<Delta, OTError> {
+        let interval = Interval::new(index, index);
+        let _ = validate_interval(&self.delta, &interval)?;
+        let delta = self.view.insert(&self.delta, text, interval)?;
         self.add_delta(&delta)?;
         Ok(delta)
     }
 
     pub fn delete(&mut self, interval: Interval) -> Result<Delta, OTError> {
+        let _ = validate_interval(&self.delta, &interval)?;
         debug_assert_eq!(interval.is_empty(), false);
         let delete = self.view.delete(&self.delta, interval)?;
         if !delete.is_empty() {
@@ -59,6 +49,7 @@ impl Document {
     }
 
     pub fn format(&mut self, interval: Interval, attribute: Attribute) -> Result<(), OTError> {
+        let _ = validate_interval(&self.delta, &interval)?;
         log::debug!("format with {} at {}", attribute, interval);
         let format_delta = self
             .view
@@ -69,10 +60,12 @@ impl Document {
         Ok(())
     }
 
-    pub fn replace(&mut self, interval: Interval, s: &str) -> Result<Delta, OTError> {
+    pub fn replace(&mut self, interval: Interval, text: &str) -> Result<Delta, OTError> {
+        let _ = validate_interval(&self.delta, &interval)?;
         let mut delta = Delta::default();
-        if !s.is_empty() {
-            delta = self.insert(interval.start, s, interval.size())?;
+        if !text.is_empty() {
+            delta = self.view.insert(&self.delta, text, interval)?;
+            self.add_delta(&delta)?;
         }
 
         if !interval.is_empty() {
@@ -124,10 +117,9 @@ impl Document {
     pub fn data(&self) -> &Delta { &self.delta }
 
     pub fn set_data(&mut self, data: Delta) { self.delta = data; }
+}
 
-    #[allow(dead_code)]
-    fn next_rev_id(&self) -> RevId { RevId(self.rev_id_counter) }
-
+impl Document {
     fn add_delta(&mut self, delta: &Delta) -> Result<(), OTError> {
         log::debug!("👉invert change {}", delta);
         let composed_delta = self.delta.compose(delta)?;
@@ -163,8 +155,21 @@ impl Document {
         log::debug!("👉invert change {}", change);
         let new_delta = self.delta.compose(change)?;
         let inverted_delta = change.invert(&self.delta);
-        // trim(&mut inverted_delta);
-
         Ok((new_delta, inverted_delta))
     }
+
+    #[allow(dead_code)]
+    fn next_rev_id(&self) -> RevId { RevId(self.rev_id_counter) }
+}
+
+fn validate_interval(delta: &Delta, interval: &Interval) -> Result<(), OTError> {
+    if delta.target_len < interval.end {
+        log::error!(
+            "{:?} out of bounds. should 0..{}",
+            interval,
+            delta.target_len
+        );
+        return Err(ErrorBuilder::new(OTErrorCode::IntervalOutOfBound).build());
+    }
+    Ok(())
 }

+ 2 - 2
rust-lib/flowy-ot/src/client/view/delete_ext.rs

@@ -1,6 +1,6 @@
 use crate::{
     client::view::DeleteExt,
-    core::{Attributes, Delta, DeltaBuilder, Interval},
+    core::{Delta, DeltaBuilder, Interval},
 };
 
 pub struct DefaultDeleteExt {}
@@ -8,7 +8,7 @@ impl DeleteExt for DefaultDeleteExt {
     fn apply(&self, _delta: &Delta, interval: Interval) -> Option<Delta> {
         Some(
             DeltaBuilder::new()
-                .retain(interval.start, Attributes::empty())
+                .retain(interval.start)
                 .delete(interval.size())
                 .build(),
         )

+ 4 - 0
rust-lib/flowy-ot/src/client/view/extension.rs

@@ -1,5 +1,9 @@
 use crate::core::{Attribute, Delta, Interval};
 
+pub type InsertExtension = Box<dyn InsertExt>;
+pub type FormatExtension = Box<dyn FormatExt>;
+pub type DeleteExtension = Box<dyn DeleteExt>;
+
 pub trait InsertExt {
     fn apply(&self, delta: &Delta, replace_len: usize, text: &str, index: usize) -> Option<Delta>;
 }

+ 3 - 7
rust-lib/flowy-ot/src/client/view/format_ext.rs

@@ -12,7 +12,6 @@ use crate::{
         Interval,
         Operation,
     },
-    errors::OTError,
 };
 
 pub struct FormatLinkAtCaretPositionExt {}
@@ -51,8 +50,8 @@ impl FormatExt for FormatLinkAtCaretPositionExt {
 
         Some(
             DeltaBuilder::new()
-                .retain(start, Attributes::default())
-                .retain(retain, (attribute.clone()).into())
+                .retain(start)
+                .retain_with_attributes(retain, (attribute.clone()).into())
                 .build(),
         )
     }
@@ -82,10 +81,7 @@ impl FormatExt for ResolveInlineFormatExt {
         if attribute.scope != AttributeScope::Inline {
             return None;
         }
-        let mut new_delta = DeltaBuilder::new()
-            .retain(interval.start, Attributes::default())
-            .build();
-
+        let mut new_delta = DeltaBuilder::new().retain(interval.start).build();
         let mut iter = DeltaIter::new(delta);
         iter.seek::<CharMetric>(interval.start);
 

+ 14 - 23
rust-lib/flowy-ot/src/client/view/insert_ext.rs

@@ -1,6 +1,6 @@
 use crate::{
     client::view::InsertExt,
-    core::{AttributeKey, Attributes, CharMetric, Delta, DeltaBuilder, DeltaIter, Operation},
+    core::{AttributeKey, Attributes, CharMetric, Delta, DeltaBuilder, DeltaIter},
 };
 
 pub const NEW_LINE: &'static str = "\n";
@@ -92,11 +92,7 @@ impl InsertExt for PreserveInlineStylesExt {
         }
 
         let mut iter = DeltaIter::new(delta);
-        let prev = iter.next_op_with_len(index);
-        if prev.is_none() {
-            return None;
-        }
-        let prev = prev.unwrap();
+        let prev = iter.next_op_with_len(index)?;
         if prev.get_data().contains(NEW_LINE) {
             return None;
         }
@@ -105,16 +101,16 @@ impl InsertExt for PreserveInlineStylesExt {
         if attributes.is_empty() || !attributes.contains_key(&AttributeKey::Link) {
             return Some(
                 DeltaBuilder::new()
-                    .retain(index + replace_len, Attributes::empty())
-                    .insert(text, attributes)
+                    .retain(index + replace_len)
+                    .insert_with_attributes(text, attributes)
                     .build(),
             );
         }
 
         attributes.remove(&AttributeKey::Link);
         let new_delta = DeltaBuilder::new()
-            .retain(index + replace_len, Attributes::empty())
-            .insert(text, attributes)
+            .retain(index + replace_len)
+            .insert_with_attributes(text, attributes)
             .build();
 
         return Some(new_delta);
@@ -130,27 +126,22 @@ impl InsertExt for ResetLineFormatOnNewLineExt {
 
         let mut iter = DeltaIter::new(delta);
         iter.seek::<CharMetric>(index);
-        let maybe_next_op = iter.next();
-        if maybe_next_op.is_none() {
-            return None;
-        }
-
-        let op = maybe_next_op.unwrap();
-        if !op.get_data().starts_with(NEW_LINE) {
+        let next_op = iter.next()?;
+        if !next_op.get_data().starts_with(NEW_LINE) {
             return None;
         }
 
         let mut reset_attribute = Attributes::new();
-        if op.get_attributes().contains_key(&AttributeKey::Header) {
+        if next_op.get_attributes().contains_key(&AttributeKey::Header) {
             reset_attribute.add(AttributeKey::Header.with_value(""));
         }
 
         let len = index + replace_len;
         Some(
             DeltaBuilder::new()
-                .retain(len, Attributes::default())
-                .insert(NEW_LINE, op.get_attributes())
-                .retain(1, reset_attribute)
+                .retain(len)
+                .insert_with_attributes(NEW_LINE, next_op.get_attributes())
+                .retain_with_attributes(1, reset_attribute)
                 .trim()
                 .build(),
         )
@@ -162,8 +153,8 @@ impl InsertExt for DefaultInsertExt {
     fn apply(&self, _delta: &Delta, replace_len: usize, text: &str, index: usize) -> Option<Delta> {
         Some(
             DeltaBuilder::new()
-                .retain(index + replace_len, Attributes::default())
-                .insert(text, Attributes::default())
+                .retain(index + replace_len)
+                .insert(text)
                 .build(),
         )
     }

+ 7 - 15
rust-lib/flowy-ot/src/client/view/view.rs

@@ -1,13 +1,9 @@
 use crate::{
-    client::view::{DeleteExt, FormatExt, InsertExt, *},
+    client::view::*,
     core::{Attribute, Delta, Interval},
     errors::{ErrorBuilder, OTError, OTErrorCode},
 };
 
-type InsertExtension = Box<dyn InsertExt>;
-type FormatExtension = Box<dyn FormatExt>;
-type DeleteExtension = Box<dyn DeleteExt>;
-
 pub struct View {
     insert_exts: Vec<InsertExtension>,
     format_exts: Vec<FormatExtension>,
@@ -16,13 +12,10 @@ pub struct View {
 
 impl View {
     pub(crate) fn new() -> Self {
-        let insert_exts = construct_insert_exts();
-        let format_exts = construct_format_exts();
-        let delete_exts = construct_delete_exts();
         Self {
-            insert_exts,
-            format_exts,
-            delete_exts,
+            insert_exts: construct_insert_exts(),
+            format_exts: construct_format_exts(),
+            delete_exts: construct_delete_exts(),
         }
     }
 
@@ -30,12 +23,11 @@ impl View {
         &self,
         delta: &Delta,
         text: &str,
-        index: usize,
-        replace_len: usize,
+        interval: Interval,
     ) -> Result<Delta, OTError> {
         let mut new_delta = None;
         for ext in &self.insert_exts {
-            if let Some(delta) = ext.apply(delta, replace_len, text, index) {
+            if let Some(delta) = ext.apply(delta, interval.size(), text, interval.start) {
                 new_delta = Some(delta);
                 break;
             }
@@ -57,7 +49,7 @@ impl View {
         }
 
         match new_delta {
-            None => Err(ErrorBuilder::new(OTErrorCode::ApplyInsertFail).build()),
+            None => Err(ErrorBuilder::new(OTErrorCode::ApplyDeleteFail).build()),
             Some(new_delta) => Ok(new_delta),
         }
     }

+ 12 - 2
rust-lib/flowy-ot/src/core/delta/builder.rs

@@ -11,21 +11,31 @@ impl DeltaBuilder {
         }
     }
 
-    pub fn retain(mut self, n: usize, attrs: Attributes) -> Self {
+    pub fn retain_with_attributes(mut self, n: usize, attrs: Attributes) -> Self {
         self.delta.retain(n, attrs);
         self
     }
 
+    pub fn retain(mut self, n: usize) -> Self {
+        self.delta.retain(n, Attributes::empty());
+        self
+    }
+
     pub fn delete(mut self, n: usize) -> Self {
         self.delta.delete(n);
         self
     }
 
-    pub fn insert(mut self, s: &str, attrs: Attributes) -> Self {
+    pub fn insert_with_attributes(mut self, s: &str, attrs: Attributes) -> Self {
         self.delta.insert(s, attrs);
         self
     }
 
+    pub fn insert(mut self, s: &str) -> Self {
+        self.delta.insert(s, Attributes::empty());
+        self
+    }
+
     pub fn trim(mut self) -> Self {
         trim(&mut self.delta);
         self

+ 2 - 6
rust-lib/flowy-ot/src/core/delta/iterator.rs

@@ -1,15 +1,11 @@
 use super::cursor::*;
-use crate::{
-    core::{Attributes, Delta, Interval, Operation},
-    errors::OTError,
-};
+use crate::core::{Attributes, Delta, Interval, Operation};
 use std::ops::{Deref, DerefMut};
 
 pub(crate) const MAX_IV_LEN: usize = i32::MAX as usize;
 
 pub struct DeltaIter<'a> {
     cursor: Cursor<'a>,
-    interval: Interval,
 }
 
 impl<'a> DeltaIter<'a> {
@@ -20,7 +16,7 @@ impl<'a> DeltaIter<'a> {
 
     pub fn from_interval(delta: &'a Delta, interval: Interval) -> Self {
         let cursor = Cursor::new(delta, interval);
-        Self { cursor, interval }
+        Self { cursor }
     }
 
     pub fn ops(&mut self) -> Vec<Operation> { self.collect::<Vec<_>>() }

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

@@ -65,8 +65,8 @@ impl Operation {
     #[allow(dead_code)]
     pub fn split(&self, index: usize) -> (Option<Operation>, Option<Operation>) {
         debug_assert!(index < self.length());
-        let mut left = None;
-        let mut right = None;
+        let left;
+        let right;
         match self {
             Operation::Delete(n) => {
                 left = Some(OpBuilder::delete(index).build());

+ 2 - 0
rust-lib/flowy-ot/src/errors.rs

@@ -27,8 +27,10 @@ impl Error for OTError {
 pub enum OTErrorCode {
     IncompatibleLength,
     ApplyInsertFail,
+    ApplyDeleteFail,
     ApplyFormatFail,
     ComposeOperationFail,
+    IntervalOutOfBound,
     UndoFail,
     RedoFail,
 }

+ 0 - 6
rust-lib/flowy-ot/tests/attribute_test.rs

@@ -146,12 +146,6 @@ fn delta_add_bold_consecutive() {
     OpTester::new().run_script(ops);
 }
 
-#[test]
-fn delta_add_bold_empty_str() {
-    let ops = vec![Bold(0, Interval::new(0, 4), true)];
-    OpTester::new().run_script(ops);
-}
-
 #[test]
 fn delta_add_bold_italic() {
     let ops = vec![

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

@@ -72,7 +72,7 @@ impl OpTester {
         match op {
             TestOp::Insert(delta_i, s, index) => {
                 let document = &mut self.documents[*delta_i];
-                document.insert(*index, s, 0).unwrap();
+                document.insert(*index, s).unwrap();
             },
             TestOp::Delete(delta_i, interval) => {
                 let document = &mut self.documents[*delta_i];
@@ -84,7 +84,7 @@ impl OpTester {
             },
             TestOp::InsertBold(delta_i, s, interval) => {
                 let document = &mut self.documents[*delta_i];
-                document.insert(interval.start, s, 0).unwrap();
+                document.insert(interval.start, s).unwrap();
                 document
                     .format(*interval, AttributeKey::Bold.with_value("true".to_owned()))
                     .unwrap();