Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make Delta.diff() match the quill-delta.js output, returning a leaner diff in some cases #27

Closed
wants to merge 3 commits into from

Conversation

roughike
Copy link

@roughike roughike commented Mar 31, 2022

What & why

Right now, in certain cases, Delta.diff() returns more stuff than it needs to.

For example, in the following case:

final a = Delta()
  ..insert('Test\n', {'index': 0})
  ..insert({'image': 'https://example.com/image.png'}, {'index': 1})
  ..insert('aaa\n', {'index': 2});

final b = Delta()
  ..insert('Test\n', {'index': 0})
  ..insert('\n', {'index': 0.5})
  ..insert({'image': 'https://example.com/image.png'}, {'index': 1})
  ..insert('aaa\n', {'index': 2});

print(a.diff(b));

the expected output is:

retain⟨ 5 ⟩
insert⟨ ⏎ ⟩ + {index: 0.5}

but the actual output is:

retain⟨ 5 ⟩
insert⟨ ⏎ ⟩ + {index: 0.5}
insert⟨ {image: https://example.com/image.png} ⟩ + {index: 1}
delete⟨ 1

The last insert and delete are redundant. For some reason, our OT implementation will break with the current behavior (the document will contain two images instead of one), but this PR fixes that. The actual problem might be somewhere else, but I think it would be still ideal if this Dart quill-delta library worked the same way as the JS equivalent.

To compare with the output of the JS quill-delta library, I've set up a Replit you can verify this with here.

@codecov
Copy link

codecov bot commented Mar 31, 2022

Codecov Report

Merging #27 (2769cd6) into master (4846633) will increase coverage by 0.32%.
The diff coverage is 100.00%.

❗ Current head 2769cd6 differs from pull request most recent head c267ddf. Consider uploading reports for the commit c267ddf to get more accurate results

@@            Coverage Diff             @@
##           master      #27      +/-   ##
==========================================
+ Coverage   93.56%   93.89%   +0.32%     
==========================================
  Files           1        1              
  Lines         342      344       +2     
==========================================
+ Hits          320      323       +3     
+ Misses         22       21       -1     
Impacted Files Coverage Δ
lib/quill_delta.dart 93.89% <100.00%> (+0.32%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4846633...c267ddf. Read the comment docs.

This reverts commit 2769cd6.
@roughike
Copy link
Author

@pulyaevskiy the CI is not passing because of formatting. I didn't want to create a bigger diff than it needs to be, but let me know how I should proceed :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant