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

XML and Text formatting support #184

Merged
merged 10 commits into from
Oct 19, 2024

Conversation

ColonelThirtyTwo
Copy link
Contributor

@ColonelThirtyTwo ColonelThirtyTwo commented Oct 18, 2024

Closes #151.

Add support for YJS XML fragment, binding to yrs XmlFragment, XmlElement, XmlText, and XmlEvent. Also add support for [Xml]Text embeds and attributes.

This should make pycrdt usable with collaborative rich text editors such as prosemirror. I am testing this with TipTap.

Also add the ability to apply_update in a transaction. While unrelated to the above, trying to do so without this patch will cause pycrdt to deadlock, which I found annoying and inefficient.

Add bindings to XMLFragment, XMLElement, XMLText, and XMLEvent. Allows
reading and writing of XML fragments from the document, which rich text
editors use heavily.

Closes jupyter-server#151
Rich text editors often store their markup in attributes and embeds.
Currently trying to use apply_update in a transaction causes a deadlock,
as apply_update tries to make its own new transaction but is locked out.
This reuses the existing transaction, which also allows applying
multiple updates in a single transaction.
Allows comparing whether objects refer to the same element in a doc.
Important since every access generates a new, unique Python wrapper
instance.

__eq__ is based directly from the underlying rust objects' `Eq`
implementation. __hash__ is based off of the `Hash` implementation of
the `BranchID` of the rust objects.
Like Text, XmlText can contain embedded formatting, so make that
available.
@davidbrochart
Copy link
Collaborator

Thanks!
I added a reference to #151 in the top comment.

@davidbrochart davidbrochart mentioned this pull request Oct 18, 2024
@davidbrochart
Copy link
Collaborator

That's really awesome.
I see that coverage fails because it drops to 97% with this PR. Do you think you could add more tests to get it back to 100%?
You would need to run:

coverage run -m pytest tests
coverage report --show-missing --fail-under=100

@ColonelThirtyTwo
Copy link
Contributor Author

Done

Copy link
Collaborator

@davidbrochart davidbrochart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I started adding minor comments on docstrings, but I'll merge this as-is and I will open a follow-up PR.
Thanks so much @ColonelThirtyTwo, great work!

python/pycrdt/_text.py Show resolved Hide resolved
python/pycrdt/_text.py Show resolved Hide resolved
python/pycrdt/_text.py Show resolved Hide resolved
python/pycrdt/_text.py Show resolved Hide resolved
python/pycrdt/_text.py Show resolved Hide resolved
@davidbrochart davidbrochart merged commit a0d91dd into jupyter-server:main Oct 19, 2024
28 checks passed
@davidbrochart
Copy link
Collaborator

Released in v0.10.4.

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.

xml support
2 participants