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

Peritext algorithm may attach deleted mark to new inserted text #32

Open
zxch3n opened this issue May 17, 2023 · 1 comment
Open

Peritext algorithm may attach deleted mark to new inserted text #32

zxch3n opened this issue May 17, 2023 · 1 comment

Comments

@zxch3n
Copy link

zxch3n commented May 17, 2023

The Peritext algorithm may attach a non-exist ghost mark to a newly inserted string. It is also reproducible in Automerge's implementation. But it can be fixed without affecting the validity of the historical data.

CleanShot 2023-05-18 at 00 54 25@2x

Steps to reproduce the issue in this repo

This can be replicated by adding this micromerge test

    it("doesn't create ghost style", () => {
      testConcurrentWrites({
        inputOps1: [],
        inputOps2: [
          {
            action: "addMark",
            startIndex: 4,
            endIndex: 12,
            markType: "link",
            attrs: {
              url: "inkandswitch.com",
            },
          },
          {
            action: "addMark",
            startIndex: 8,
            endIndex: 12,
            markType: "strong",
          },
          {
            action: "delete",
            index: 3,
            count: 8,
          },
          {
            action: "insert",
            index: 3,
            values: ["!"],
          },
        ],
        //
        expectedResult: [
          { marks: {}, text: "The!editor" },
        ],
      });
    });

Why

The problem is caused by this fix in the paper. In this example, when Peritext inserts the new char '!', it prefers the positions after the tombstone with the end of the link. And the tombstone is after the beginning of the bold mark. So '!' turns out to be bold.

In my implementation, I've fixed this by adding a new rule for choosing the insertion position among tombstones:

  1. Insertions occur before tombstones that contain the beginning of new marks. (new)
  2. Insertions occur before tombstones that contain the end of bold-like marks
  3. Insertions occur after tombstones that contain the end of link-like marks

Rule 1 should be satisfied before rules 2 and 3 to avoid this problem.

This solution can clearly fix the example given above and make it satisfy all of the rules. The only downside is it might make a "link-like mark" grow forward unexpectedly a bit more often. But it seems to be more tolerable.

@zxch3n zxch3n changed the title Peritext algorithm may attach deleted mark to the new inserted text Peritext algorithm may attach deleted mark to new inserted text May 17, 2023
@ept
Copy link
Member

ept commented May 18, 2023

Hi @zxch3n, well spotted, and thank you for reporting this issue. Your proposed fix seems reasonable.

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

No branches or pull requests

2 participants