From 2089cbc7aa680a4350f920fc6890652c521c3141 Mon Sep 17 00:00:00 2001 From: Andras Schmelczer Date: Sat, 21 Jun 2025 13:58:06 +0100 Subject: [PATCH] Fix with new string builder --- README.md | 25 +++--- src/operation_transformation.rs | 2 +- src/operation_transformation/edited_text.rs | 59 +++++++------- src/operation_transformation/merge_context.rs | 26 +++--- src/operation_transformation/operation.rs | 80 +++++++++---------- .../ordered_operation.rs | 9 ++- tests/examples/various.yml | 2 +- 7 files changed, 101 insertions(+), 102 deletions(-) diff --git a/README.md b/README.md index 4735cf5..a3d3d7f 100644 --- a/README.md +++ b/README.md @@ -1,26 +1,22 @@ # VaultLink self-hosted Obsidian plugin for file syncing -[![Check](https://github.com/schmelczer/vault-link/actions/workflows/check.yml/badge.svg)](https://github.com/schmelczer/vault-link/actions/workflows/check.yml) -[![E2E tests](https://github.com/schmelczer/vault-link/actions/workflows/e2e.yml/badge.svg)](https://github.com/schmelczer/vault-link/actions/workflows/e2e.yml) -[![Publish server Docker image](https://github.com/schmelczer/vault-link/actions/workflows/publish-docker.yml/badge.svg)](https://github.com/schmelczer/vault-link/actions/workflows/publish-docker.yml) -[![Publish Obsidian plugin](https://github.com/schmelczer/vault-link/actions/workflows/publish-plugin.yml/badge.svg)](https://github.com/schmelczer/vault-link/actions/workflows/publish-plugin.yml) - +[![Check](https://github.com/schmelczer/reconcile/actions/workflows/check.yml/badge.svg)](https://github.com/schmelczer/reconcile/actions/workflows/check.yml) +[![Publish to GitHub Pages](https://github.com/schmelczer/reconcile/actions/workflows/gh-pages.yml/badge.svg)](https://github.com/schmelczer/reconcile/actions/workflows/gh-pages.yml) ## Develop ### Install [nvm](https://github.com/nvm-sh/nvm) -- `curl -o- https://raw.githubusercontent.com/nvm-sh/nvm/v0.40.1/install.sh | bash` -- `nvm install 22` -- `nvm use 22` -- Optionally set the system-wide default: `nvm alias default 22` - +- `curl -o- https://raw.githubusercontent.com/nvm-sh/nvm/v0.40.1/install.sh | bash` +- `nvm install 22` +- `nvm use 22` +- Optionally set the system-wide default: `nvm alias default 22` ### Set up Rust -- Install [`rustup`](https://rustup.rs): `curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh` -- Install [`wasm-pack`](https://rustwasm.github.io/wasm-pack/installer): `curl https://rustwasm.github.io/wasm-pack/installer/init.sh -sSf | sh` -- `cargo install cargo-insta sqlx-cli cargo-edit` +- Install [`rustup`](https://rustup.rs): `curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh` +- Install [`wasm-pack`](https://rustwasm.github.io/wasm-pack/installer): `curl https://rustwasm.github.io/wasm-pack/installer/init.sh -sSf | sh` +- `cargo install cargo-insta sqlx-cli cargo-edit` ### Install Obsidian on Linux @@ -45,7 +41,6 @@ scripts/update-api-types.sh scripts/bump-version.sh patch ``` - #### Run E2E tests ```sh @@ -56,4 +51,4 @@ And to clean up the logs & database files, run `scripts/clean-up.sh` ## Projects -- [Sync server](./backend/sync_server/README.md) +- [Sync server](./backend/sync_server/README.md) diff --git a/src/operation_transformation.rs b/src/operation_transformation.rs index fc280ec..9fd5a5d 100644 --- a/src/operation_transformation.rs +++ b/src/operation_transformation.rs @@ -111,7 +111,7 @@ mod test { }, // inside of "s|ample" because "text" got replaced by "sample" CursorPosition { id: 3, - char_index: 43 + char_index: 42 }, // before "cursor movements" ] ) diff --git a/src/operation_transformation/edited_text.rs b/src/operation_transformation/edited_text.rs index 236e4c5..4548a41 100644 --- a/src/operation_transformation/edited_text.rs +++ b/src/operation_transformation/edited_text.rs @@ -1,14 +1,14 @@ #[cfg(feature = "serde")] use serde::{Deserialize, Serialize}; -use super::{ordered_operation::OrderedOperation, CursorPosition, Operation, TextWithCursors}; +use super::{CursorPosition, Operation, TextWithCursors, ordered_operation::OrderedOperation}; use crate::{ diffs::{myers::diff, raw_operation::RawOperation}, operation_transformation::{ merge_context::MergeContext, utils::{cook_operations::cook_operations, elongate_operations::elongate_operations}, }, - tokenizer::{word_tokenizer::word_tokenizer, Tokenizer}, + tokenizer::{Tokenizer, word_tokenizer::word_tokenizer}, utils::{side::Side, string_builder::StringBuilder}, }; @@ -93,6 +93,7 @@ where #[must_use] pub fn merge(self, other: Self) -> Self { + println!("\n\n\n\n\n\n----\n"); debug_assert_eq!( self.text, other.text, "`EditedText`-s must be derived from the same text to be mergable" @@ -138,20 +139,11 @@ where operation, Operation::Insert { .. } | Operation::Equal { .. } ); - println!(" {operation:?}"); + println!("{side} {operation:?} ({maybe_left_op:?}, {maybe_right_op:?})"); let original_length = operation.len() as i64; let result = match side { Side::Left => { - let are_equal = matches!(operation, Operation::Equal { .. }) - && maybe_right_op - .as_ref() - .map(|op| { - matches!(op.operation, Operation::Equal { .. }) - && op.operation.eq(&operation) - }) - .unwrap_or_default(); - let result = operation.merge_operations_with_context( &mut right_merge_context, &mut left_merge_context, @@ -160,6 +152,8 @@ where if let Some(ref op @ (Operation::Insert { .. } | Operation::Equal { .. })) = result { + println!("merged_length {merged_length}, idx {}", op.start_index()); + // assert_eq!(op.start_index() as i64, merged_length as i64); let shift = op.start_index() as i64 - seen_left_length as i64 + op.len() as i64 - original_length; @@ -180,11 +174,7 @@ where maybe_left_op = left_iter.next(); - if are_equal { - None - } else { - result - } + result } Side::Right => { let result = operation.merge_operations_with_context( @@ -196,6 +186,8 @@ where result { println!("merged_length {merged_length}, idx {}", op.start_index()); + // assert_eq!(op.start_index() as i64, merged_length as i64); + let shift = op.start_index() as i64 - seen_right_length as i64 + op.len() as i64 - original_length; @@ -220,25 +212,30 @@ where } }; + println!(" = {result:?}"); + if let Some(operation) = result { - merged_length += operation.len() as usize; + let last_operation = merged_operations.last(); + if let Some(last_operation) = last_operation { + if matches!(last_operation.operation, Operation::Equal { .. }) + && matches!(operation, Operation::Equal { .. }) + && last_operation.operation.eq(&operation) + { + println!("skipping equal"); + continue; + } + } + + if is_advancing_operation { + merged_length += operation.len(); + } + merged_operations.push(OrderedOperation { order, operation }); } } - let last_index = merged_operations - .iter() - .filter(|operation| { - matches!( - operation.operation, - Operation::Insert { .. } | Operation::Equal { .. } - ) - }) - .next_back() - .map_or(0, |op| op.operation.end_index()); - for cursor in left_cursors.chain(right_cursors) { - merged_cursors.push(cursor.with_index(last_index)); + merged_cursors.push(cursor.with_index(merged_length)); } Self::new(self.text, merged_operations, merged_cursors) @@ -250,6 +247,8 @@ where let mut builder: StringBuilder<'_> = StringBuilder::new(self.text); for OrderedOperation { operation, .. } in &self.operations { + println!("applying operation {operation:?}"); + builder = operation.apply(builder); } diff --git a/src/operation_transformation/merge_context.rs b/src/operation_transformation/merge_context.rs index 5cf0972..e397226 100644 --- a/src/operation_transformation/merge_context.rs +++ b/src/operation_transformation/merge_context.rs @@ -52,22 +52,24 @@ where /// threshold operation. This updates the `shift` in case the last operation /// was a delete. pub fn consume_last_operation_if_it_is_too_behind(&mut self, threshold_index: i64) { - if let Some(last_operation) = self.last_operation.as_ref() { - if let Operation::Delete { - deleted_character_count, - .. - } = last_operation - { - if threshold_index + self.shift > last_operation.end_index() as i64 { + match self.last_operation.as_ref() { + Some( + op @ Operation::Delete { + deleted_character_count, + .. + }, + ) => { + if threshold_index + self.shift > op.end_index() as i64 { self.shift -= *deleted_character_count as i64; self.last_operation = None; } - } else if let Operation::Insert { .. } = last_operation - && threshold_index + self.shift - last_operation.len() as i64 - > last_operation.end_index() as i64 - { - self.last_operation = None; } + Some(op @ Operation::Insert { .. }) | Some(op @ Operation::Equal { .. }) => { + if threshold_index + self.shift - op.len() as i64 > op.end_index() as i64 { + self.last_operation = None; + } + } + _ => {} } } } diff --git a/src/operation_transformation/operation.rs b/src/operation_transformation/operation.rs index 827da77..862a5b6 100644 --- a/src/operation_transformation/operation.rs +++ b/src/operation_transformation/operation.rs @@ -6,16 +6,16 @@ use serde::{Deserialize, Serialize}; use super::merge_context::MergeContext; use crate::{ - Token, utils::{ find_longest_prefix_contained_within::find_longest_prefix_contained_within, string_builder::StringBuilder, }, + Token, }; /// Represents a change that can be applied on a `StringBuilder`. #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] -#[derive(Clone)] +#[derive(Clone, PartialEq)] pub enum Operation where T: PartialEq + Clone + std::fmt::Debug, @@ -42,39 +42,39 @@ where }, } -impl PartialEq for Operation -where - T: PartialEq + Clone + std::fmt::Debug, -{ - fn eq(&self, other: &Self) -> bool { - match (self, other) { - ( - Operation::Equal { length, .. }, - Operation::Equal { - length: other_length, - .. - }, - ) => length == other_length, - ( - Operation::Insert { text, .. }, - Operation::Insert { - text: other_text, .. - }, - ) => text == other_text, - ( - Operation::Delete { - deleted_character_count, - .. - }, - Operation::Delete { - deleted_character_count: other_deleted_character_count, - .. - }, - ) => deleted_character_count == other_deleted_character_count, - _ => false, - } - } -} +// impl PartialEq for Operation +// where +// T: PartialEq + Clone + std::fmt::Debug, +// { +// fn eq(&self, other: &Self) -> bool { +// match (self, other) { +// ( +// Operation::Equal { length, .. }, +// Operation::Equal { +// length: other_length, +// .. +// }, +// ) => length == other_length, +// ( +// Operation::Insert { text, .. }, +// Operation::Insert { +// text: other_text, .. +// }, +// ) => text == other_text, +// ( +// Operation::Delete { +// deleted_character_count, +// .. +// }, +// Operation::Delete { +// deleted_character_count: other_deleted_character_count, +// .. +// }, +// ) => deleted_character_count == other_deleted_character_count, +// _ => false, +// } +// } +// } impl Operation where @@ -192,7 +192,7 @@ where deleted_text .as_ref() .is_none_or(|text| builder.get_slice(self.range()) == *text), - "Text to delete (`{}`) does not match the text in the range: {}", + "Text to-be-deleted `{}` does not match the text in the range: `{}`", deleted_text.as_ref().unwrap_or(&"".to_owned()), builder.get_slice(self.range()) ); @@ -531,11 +531,9 @@ mod tests { #[test] #[should_panic(expected = "Shifted index must be non-negative")] fn test_shifting_error() { - insta::assert_debug_snapshot!( - Operation::create_insert(1, vec!["hi".into()]) - .unwrap() - .with_shifted_index(-2) - ); + insta::assert_debug_snapshot!(Operation::create_insert(1, vec!["hi".into()]) + .unwrap() + .with_shifted_index(-2)); } #[test] diff --git a/src/operation_transformation/ordered_operation.rs b/src/operation_transformation/ordered_operation.rs index ac4b0d6..c9c0b67 100644 --- a/src/operation_transformation/ordered_operation.rs +++ b/src/operation_transformation/ordered_operation.rs @@ -1,7 +1,7 @@ #[cfg(feature = "serde")] use serde::{Deserialize, Serialize}; -use crate::{Token, operation_transformation::Operation}; +use crate::{operation_transformation::Operation, Token}; #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] #[derive(Debug, Clone, PartialEq)] @@ -17,9 +17,14 @@ impl OrderedOperation where T: PartialEq + Clone + std::fmt::Debug, { - pub fn get_sort_key(&self) -> (usize, usize, String) { + pub fn get_sort_key(&self) -> (usize, usize, usize, String) { ( self.order, + match &self.operation { + Operation::Delete { .. } => 1, + Operation::Insert { .. } => 2, + Operation::Equal { .. } => 3, + }, self.operation.start_index(), // Make sure that the ordering is deterministic regardless of which text // is left or right. diff --git a/tests/examples/various.yml b/tests/examples/various.yml index cfbeb42..7a87e4e 100644 --- a/tests/examples/various.yml +++ b/tests/examples/various.yml @@ -67,7 +67,7 @@ expected: market| placemarket|space parent: A B C D left: A X B D| right: A B Y| -expected: A X B |Y| +expected: A X B Y|| --- parent: Please submit your assignment by Friday