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

Direct sync #11

Merged
merged 17 commits into from
Apr 24, 2024
Merged

Direct sync #11

merged 17 commits into from
Apr 24, 2024

Conversation

paulsonnentag
Copy link
Collaborator

The current implementation of automerge-codemirror inverts Codemirror transactions to sync the state with an Automerge document, which breaks the history plugin of Codemirror. This was a performance optimization, so we don't need to create a new change for each keystroke and instead can batch multiple keystrokes together. This optimization is no longer necessary because Automerge is fast enough.

This new implementation simplifies this by changing the automerge document directly on each keystroke and remembering the resulting heads. If a remote change comes in, we diff the new version against the stored heads and apply the patches to automerge

I'm also simplifying the API. automerge-codemirror is just a single Codemirror plugin now and doesn't need any external setup anymore. The only thing you need to do is set the initial source to the value of the document at initialization

const source = handle.docSync()
const view = new EditorView({
  doc: source,
  extensions: [
    basicSetup,
    automergeSyncPlugin({
      handle,
      path: ["text"],
    }),
  ],
  parent: containerRef.current,
}))

@paulsonnentag paulsonnentag force-pushed the clean-transactions branch 2 times, most recently from 2f73c6a to 3a4a696 Compare April 24, 2024 06:56
The keyboard shortcuts for undo/redo have inconsistent behaviours across platforms so we use buttons that trigger undo / redo commands in codemirror instead
@alexjg
Copy link
Contributor

alexjg commented Apr 24, 2024

Overall looks great, enormously simplifies things!

The only thing I think this PR needs is an assertion in the cypress tests that after all the undo/redo stuff has happened the state of the document (automerge document that is) is what we would expect.

@paulsonnentag
Copy link
Collaborator Author

That's a good point. I will add that

@alexjg alexjg merged commit 749aac8 into main Apr 24, 2024
2 checks passed
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.

2 participants