satoren / y_ex

Yjs port for Elixir
MIT License
58 stars 2 forks source link

Support UndoManager #69

Open satoren opened 1 month ago

satoren commented 1 month ago

https://docs.yjs.dev/api/undo-manager

cortfritz commented 2 weeks ago

played around with an approach using updates. could evolve to support other types, depth, persistence, and better efficiency using SVs.

https://github.com/cortfritz/undo_manager

Let me know if you think that direction makes sense vs. something else. I don't love coding inversions of %{insert: blah...}

Possibly inversions of updates in rust?

I'm open.

cortfritz commented 2 weeks ago

I could put the question better; do you think the use of Yex doc updates to store differences can be a good path to implant the yjs spec for undo manager?

I could leverage SVs to be more efficient. And instead of a string for clients, I would implement the spec for contexts.

satoren commented 2 weeks ago

I was planning to export the UndoManager of yrs(rust) with nif.

cortfritz commented 2 weeks ago

derp. so much better. I'm happy to take a stab at it if you would like.

cortfritz commented 2 weeks ago

cannot get your main branch to pass tests. If i futz with mix.exs and compile the rust directly with cargo build i can compile. I'm guessing i'm not doing that right though.

in the meantime my WiP is here: https://github.com/cortfritz/y_ex/tree/feature/undo

just sketching for now.

satoren commented 2 weeks ago

The RUSTLER_PRECOMPILATION_YEX_BUILD environment variable must be set to 1. For those developers who are willing to participate I need to update README.

cortfritz commented 1 week ago

that works! thank you.

cortfritz commented 1 week ago

This is as far as i got.

https://github.com/cortfritz/y_ex/blob/feature/undo/native/yex/src/undo.rs

I'm able to create a new yrs::UndoManager and exercise some functions, but unable to get Undo to... undo. When I passed a Text into UndoManager::new as scope I could only get panics. Couldn't figure out how set up Undo to do it's job. I'm going to come back to this eventually but effort/progress is out of whack and i'm going to back off for now.

LOVE THIS PROJECT, thank you.

satoren commented 1 week ago

The panic is probably because the ENV is not registered with thread_local. This is used in the observer callback to send messages to the beam. Therefore, any function that will be modified must be wrapped in this. https://github.com/satoren/y_ex/blob/a23ce60c66bcfbe80b6a3accfd3aae92275cecbd/native/yex/src/awareness.rs#L96

cortfritz commented 5 days ago

followed your advice - this seems to go better:

https://github.com/cortfritz/y_ex/blob/feature/undo/native/yex/src/undo.rs

no errors, but I'm still not actually effecting an undo, with or without origin.

cortfritz commented 5 days ago

got it working!!!

I'm not able to get async working - but it looks like your implementation is synchronous?

cortfritz commented 5 days ago

getting Yex.UndoManager.new/2 to take Yex.Text, Yex.Map, Yex.Array, etc may have a better pattern. I couldn't get NifSharedType to work or NifYInput... I ended up with the less elegant:

  1. pattern match on the elixir side to have separate functions for each
  2. separate elixir go to separate rust functions in undo.rs
  3. I tried having these functions use a shared impelementation but did not get that working either. So very not DRY, each function does essentially the same thing in prepping the call to yrs undo::new

I haven't tackled XML types yet.

But tests passing for undo on Text, Map, Array, and multiple origins for Text. Will grow tests on these to be more comprehensive and add XML types, then address other functionality.

Can refactor if I find a better pattern for the interface.

cortfritz commented 4 days ago

fixed the NIF interface and implementation so we are DRY and don't have parallel implementations.

implemented redo

next will support adding scopes.

cortfritz commented 4 days ago

implemented stop_capture, expand_scope, and exclude_origin

next adding the observable behavior and the ability to add/get meta info on stack items.

cortfritz commented 4 days ago

ok, i believe i have implemented (with caveats below) all of the features demonstrated in the yjs example you link above.

rust

elixir

caveats

cortfritz commented 15 hours ago

upon creating a pull request I see some egregious mistakes, bad structure, etc. have to love coderabbit!

i'm fixing these now.