Refactor & simplify merging logic #3

Merged
schmelczer merged 15 commits from asch/simplify into main 2025-06-22 15:07:04 +01:00
7 changed files with 101 additions and 102 deletions
Showing only changes of commit 2089cbc7aa - Show all commits

View file

@ -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)

View file

@ -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"
]
)

View file

@ -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);
}

View file

@ -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;
}
}
_ => {}
}
}
}

View file

@ -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<T>
where
T: PartialEq + Clone + std::fmt::Debug,
@ -42,39 +42,39 @@ where
},
}
impl<T> PartialEq for Operation<T>
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<T> PartialEq for Operation<T>
// 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<T> Operation<T>
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]

View file

@ -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<T> OrderedOperation<T>
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.

View file

@ -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