Feature/undo (#103)

* draft first pass of undo * hack mix to get project to compile with rust * hack nif to try to get tests to pass * save WiP of undo manager * revert mix to original repo * get undo code to compile * undo manager compiling checkpoint * prove passable tests with incremental strategy * add undo * save WiP of undo with origin * fix NIF param context * test undo * use yrs undo correctly * extend capability beyond Yex.Text to Yex.Map and Yex.Array * make undo threadsafe * add origin awareness tests to map and array * resolved, apparently, thread safety by changing test setup * ensure thread safety * refactor to use shared implementation for new * unify NIF interface * implement stop_capture, expand_scope, and exclude_origin * clean up obviated parallel "new" functions * add observers and ability to add/get metadata from stack items * manage undo observer state in GenServer * remove debug * add clear * add undo manager with options * add options timeout test * mirror yjs examples for clarity for satoren * Update lib/server/undo_server_observer_behaviour.ex Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * move server tests to correct test folder * refactor undo_server as simplified observer_server * remove bad undo observer scheme * draft first pass of undo * hack mix to get project to compile with rust * hack nif to try to get tests to pass * save WiP of undo manager * revert mix to original repo * get undo code to compile * undo manager compiling checkpoint * prove passable tests with incremental strategy * add undo * save WiP of undo with origin * fix NIF param context * test undo * use yrs undo correctly * extend capability beyond Yex.Text to Yex.Map and Yex.Array * make undo threadsafe * add origin awareness tests to map and array * resolved, apparently, thread safety by changing test setup * ensure thread safety * refactor to use shared implementation for new * unify NIF interface * implement stop_capture, expand_scope, and exclude_origin * clean up obviated parallel "new" functions * add observers and ability to add/get metadata from stack items * manage undo observer state in GenServer * remove debug * add clear * add undo manager with options * add options timeout test * mirror yjs examples for clarity for satoren * Update lib/server/undo_server_observer_behaviour.ex Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * move server tests to correct test folder * refactor undo_server as simplified observer_server * remove bad undo observer scheme * put undo in correct fmt position * remove unused atoms previously supporting undo observers * fmt undo * format elixir code * format changes to nif * adopt byzantine structure to try to help coveralls see we are indeed covered * format undo test * worsen code structure to put up with coveralls not seeing inside pattern matches * test embedded objects * format * remove pattern matching * improve order * add xml tests * use NifUntaggedEnum * fmt properly * suffice coderabbit refactor suggestion Refactor suggestion Ensure consistent return types for new/2 and new_with_options/3. Currently, new_with_options/3 uses unwrap_manager_result/1 to directly return either the manager struct or an error, whereas new/2 calls new_with_options/3 without wrapping the result. This can lead to inconsistent error handling and unexpected behaviors. For clarity and better error handling, consider modifying both functions to return {:ok, manager} on success or {:error, reason} on failure. This aligns with Elixir conventions and makes error handling more predictable for the caller. Apply this diff to adjust the return values: def new(doc, scope) when is_struct(scope) do - new_with_options(doc, scope, %Options{}) + case new_with_options(doc, scope, %Options{}) do + {:ok, manager} -> {:ok, manager} + error -> error + end end def new_with_options(doc, scope, %Options{} = options) do doc |> Yex.Nif.undo_manager_new_with_options(scope, options) - |> unwrap_manager_result() + |> case do + {:ok, manager} -> {:ok, manager} + error -> error + end end * suffice coderabbit potential issue Handle potential errors when creating an UndoManager. In the create_undo_manager_with_options function, the use of unwrap() can cause a panic if an error occurs while getting the branch reference. It is safer to handle the error properly to prevent unexpected crashes. * suffice coderabbit unsafe transmute usage issue Avoid unsafe transmute usage and improper Env lifetime extension. Using unsafe { std::mem::transmute(env) } to extend the lifetime of Env to static is unsafe and can lead to undefined behavior. The Env should not outlive the NIF call. * remove unused functions * make purpose more clear * make purpose more clear * remove test replaced by guards * fix nits * fmt * improve coverage * simplify * satisfy coveralls demand for coverage * improve comments * replace try with case pattern match for better style * remove unsued crate * remove unsued atoms * clarify comments in tests * clarify tests * clarify purpose of origins in test --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
satoren · Dec 23, 2024 · a834e11 · a834e11
1 parent b96342d
commit a834e11
Show file tree

Hide file tree

Showing 10 changed files with 1,558 additions and 2 deletions.
diff --git a/.tool-versions b/.tool-versions
@@ -0,0 +1 @@
+elixir 1.17.3-otp-27
diff --git a/lib/nif.ex b/lib/nif.ex
@@ -167,6 +167,19 @@ defmodule Yex.Nif do
     do: :erlang.nif_error(:nif_not_loaded)
 
   def awareness_remove_states(_awareness, _clients), do: :erlang.nif_error(:nif_not_loaded)
+
+  def undo_manager_new(_doc, _scope), do: :erlang.nif_error(:nif_not_loaded)
+
+  def undo_manager_new_with_options(_doc, _scope, _options),
+    do: :erlang.nif_error(:nif_not_loaded)
+
+  def undo_manager_include_origin(_undo_manager, _origin), do: :erlang.nif_error(:nif_not_loaded)
+  def undo_manager_undo(_undo_manager), do: :erlang.nif_error(:nif_not_loaded)
+  def undo_manager_redo(_undo_manager), do: :erlang.nif_error(:nif_not_loaded)
+  def undo_manager_expand_scope(_undo_manager, _scope), do: :erlang.nif_error(:nif_not_loaded)
+  def undo_manager_exclude_origin(_undo_manager, _origin), do: :erlang.nif_error(:nif_not_loaded)
+  def undo_manager_stop_capturing(_undo_manager), do: :erlang.nif_error(:nif_not_loaded)
+  def undo_manager_clear(_undo_manager), do: :erlang.nif_error(:nif_not_loaded)
 end
 
 defmodule Yex.Nif.Util do

diff --git a/lib/undo_manager.ex b/lib/undo_manager.ex
@@ -0,0 +1,141 @@
+defmodule Yex.UndoManager.Options do
+  @moduledoc """
+  Options for creating an UndoManager.
+
+  * `:capture_timeout` - Time in milliseconds to wait before creating a new capture group
+  """
+  # Default from Yrs
+  defstruct capture_timeout: 500
+
+  @type t :: %__MODULE__{
+          capture_timeout: non_neg_integer()
+        }
+end
+
+defmodule Yex.UndoManager do
+  alias Yex.UndoManager.Options
+
+  defguard is_valid_scope(scope)
+           when is_struct(scope, Yex.Text) or
+                  is_struct(scope, Yex.Array) or
+                  is_struct(scope, Yex.Map) or
+                  is_struct(scope, Yex.XmlText) or
+                  is_struct(scope, Yex.XmlElement) or
+                  is_struct(scope, Yex.XmlFragment)
+
+  @moduledoc """
+  Represents a Y.UndoManager instance.
+  """
+  defstruct [:reference]
+
+  @type t :: %__MODULE__{
+          reference: reference()
+        }
+
+  @doc """
+  Creates a new UndoManager for the given document and scope with default options.
+  The scope can be a Text, Array, Map, XmlText, XmlElement, or XmlFragment type.
+
+  ## Errors
+  - Returns `{:error, "Invalid scope: expected a struct"}` if scope is not a struct
+  - Returns `{:error, "Failed to get branch reference"}` if there's an error accessing the scope
+  """
+  @spec new(Yex.Doc.t(), struct()) ::
+          {:ok, Yex.UndoManager.t()} | {:error, term()}
+  def new(doc, scope)
+      when is_valid_scope(scope) do
+    new_with_options(doc, scope, %Options{})
+  end
+
+  @doc """
+  Creates a new UndoManager with the given options.
+
+  ## Options
+
+  See `Yex.UndoManager.Options` for available options.
+
+  ## Errors
+  - Returns `{:error, "NIF error: <message>"}` if underlying NIF returns an error
+  """
+  @spec new_with_options(Yex.Doc.t(), struct(), Options.t()) ::
+          {:ok, Yex.UndoManager.t()} | {:error, term()}
+  def new_with_options(doc, scope, options)
+      when is_valid_scope(scope) and
+             is_struct(options, Options) do
+    case Yex.Nif.undo_manager_new_with_options(doc, scope, options) do
+      {:ok, manager} -> {:ok, manager}
+      {:error, message} -> {:error, "NIF error: #{message}"}
+    end
+  end
+
+  @doc """
+  Includes an origin to be tracked by the UndoManager.
+  """
+  def include_origin(undo_manager, origin) do
+    Yex.Nif.undo_manager_include_origin(undo_manager, origin)
+  end
+
+  @doc """
+  Excludes an origin from being tracked by the UndoManager.
+  """
+  def exclude_origin(undo_manager, origin) do
+    Yex.Nif.undo_manager_exclude_origin(undo_manager, origin)
+  end
+
+  @doc """
+  Undoes the last tracked change.
+  """
+  def undo(undo_manager) do
+    Yex.Nif.undo_manager_undo(undo_manager)
+  end
+
+  @doc """
+  Redoes the last undone change.
+  """
+  def redo(undo_manager) do
+    Yex.Nif.undo_manager_redo(undo_manager)
+  end
+
+  @doc """
+  Expands the scope of the UndoManager to include additional shared types.
+  The scope can be a Text, Array, or Map type.
+  """
+  def expand_scope(undo_manager, scope) do
+    Yex.Nif.undo_manager_expand_scope(undo_manager, scope)
+  end
+
+  @doc """
+  Stops capturing changes for the current stack item.
+  This ensures that the next change will create a new stack item instead of
+  being merged with the previous one, even if it occurs within the normal timeout window.
+
+  ## Example:
+      text = Doc.get_text(doc, "text")
+      undo_manager = UndoManager.new(doc, text)
+
+      Text.insert(text, 0, "a")
+      UndoManager.stop_capturing(undo_manager)
+      Text.insert(text, 1, "b")
+      UndoManager.undo(undo_manager)
+      # Text.to_string(text) will be "a" (only "b" was removed)
+  """
+  def stop_capturing(undo_manager) do
+    Yex.Nif.undo_manager_stop_capturing(undo_manager)
+  end
+
+  @doc """
+  Clears all StackItems stored within current UndoManager, effectively resetting its state.
+
+  ## Example:
+      text = Doc.get_text(doc, "text")
+      undo_manager = UndoManager.new(doc, text)
+
+      Text.insert(text, 0, "Hello")
+      Text.insert(text, 5, " World")
+      UndoManager.clear(undo_manager)
+      # All undo/redo history is now cleared
+  """
+  def clear(undo_manager) do
+    Yex.Nif.undo_manager_clear(undo_manager)
+  end
+end
diff --git a/mix.exs b/mix.exs
@@ -64,7 +64,8 @@ defmodule Yex.MixProject do
       {:credo, "~> 1.7", only: [:dev, :test], runtime: false},
       {:dialyxir, "~> 1.4", only: [:dev, :test], runtime: false},
       {:excoveralls, "~> 0.18", only: :test},
-      {:benchee, "~> 1.0", only: :dev}
+      {:benchee, "~> 1.0", only: :dev},
+      {:mock, "~> 0.3.0", only: :test}
     ]
   end
 end
diff --git a/mix.lock b/mix.lock
@@ -16,8 +16,10 @@
   "makeup": {:hex, :makeup, "1.1.2", "9ba8837913bdf757787e71c1581c21f9d2455f4dd04cfca785c70bbfff1a76a3", [:mix], [{:nimble_parsec, "~> 1.2.2 or ~> 1.3", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm", "cce1566b81fbcbd21eca8ffe808f33b221f9eee2cbc7a1706fc3da9ff18e6cac"},
   "makeup_elixir": {:hex, :makeup_elixir, "0.16.2", "627e84b8e8bf22e60a2579dad15067c755531fea049ae26ef1020cad58fe9578", [:mix], [{:makeup, "~> 1.0", [hex: :makeup, repo: "hexpm", optional: false]}, {:nimble_parsec, "~> 1.2.3 or ~> 1.3", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm", "41193978704763f6bbe6cc2758b84909e62984c7752b3784bd3c218bb341706b"},
   "makeup_erlang": {:hex, :makeup_erlang, "1.0.1", "c7f58c120b2b5aa5fd80d540a89fdf866ed42f1f3994e4fe189abebeab610839", [:mix], [{:makeup, "~> 1.0", [hex: :makeup, repo: "hexpm", optional: false]}], "hexpm", "8a89a1eeccc2d798d6ea15496a6e4870b75e014d1af514b1b71fa33134f57814"},
+  "meck": {:hex, :meck, "0.9.2", "85ccbab053f1db86c7ca240e9fc718170ee5bda03810a6292b5306bf31bae5f5", [:rebar3], [], "hexpm", "81344f561357dc40a8344afa53767c32669153355b626ea9fcbc8da6b3045826"},
   "mime": {:hex, :mime, "2.0.6", "8f18486773d9b15f95f4f4f1e39b710045fa1de891fada4516559967276e4dc2", [:mix], [], "hexpm", "c9945363a6b26d747389aac3643f8e0e09d30499a138ad64fe8fd1d13d9b153e"},
   "mint": {:hex, :mint, "1.6.2", "af6d97a4051eee4f05b5500671d47c3a67dac7386045d87a904126fd4bbcea2e", [:mix], [{:castore, "~> 0.1.0 or ~> 1.0", [hex: :castore, repo: "hexpm", optional: true]}, {:hpax, "~> 0.1.1 or ~> 0.2.0 or ~> 1.0", [hex: :hpax, repo: "hexpm", optional: false]}], "hexpm", "5ee441dffc1892f1ae59127f74afe8fd82fda6587794278d924e4d90ea3d63f9"},
+  "mock": {:hex, :mock, "0.3.9", "10e44ad1f5962480c5c9b9fa779c6c63de9bd31997c8e04a853ec990a9d841af", [:mix], [{:meck, "~> 0.9.2", [hex: :meck, repo: "hexpm", optional: false]}], "hexpm", "9e1b244c4ca2551bb17bb8415eed89e40ee1308e0fbaed0a4fdfe3ec8a4adbd3"},
   "nimble_options": {:hex, :nimble_options, "1.1.1", "e3a492d54d85fc3fd7c5baf411d9d2852922f66e69476317787a7b2bb000a61b", [:mix], [], "hexpm", "821b2470ca9442c4b6984882fe9bb0389371b8ddec4d45a9504f00a66f650b44"},
   "nimble_parsec": {:hex, :nimble_parsec, "1.4.0", "51f9b613ea62cfa97b25ccc2c1b4216e81df970acd8e16e8d1bdc58fef21370d", [:mix], [], "hexpm", "9c565862810fb383e9838c1dd2d7d2c437b3d13b267414ba6af33e50d2d1cf28"},
   "nimble_pool": {:hex, :nimble_pool, "1.1.0", "bf9c29fbdcba3564a8b800d1eeb5a3c58f36e1e11d7b7fb2e084a643f645f06b", [:mix], [], "hexpm", "af2e4e6b34197db81f7aad230c1118eac993acc0dae6bc83bac0126d4ae0813a"},

diff --git a/native/yex/src/atoms.rs b/native/yex/src/atoms.rs
@@ -34,4 +34,5 @@ rustler::atoms! {
   delete,
   retain,
   attributes,
+
 }
diff --git a/native/yex/src/lib.rs b/native/yex/src/lib.rs
@@ -12,6 +12,7 @@ mod subscription;
 mod sync;
 mod term_box;
 mod text;
+mod undo;
 mod utils;
 mod wrap;
 mod xml;

diff --git a/native/yex/src/shared_type.rs b/native/yex/src/shared_type.rs
@@ -4,7 +4,6 @@ use std::ops::Deref;
 use yrs::{Hook, ReadTxn, SharedRef, TransactionMut};
 
 use crate::{
-    atoms,
     doc::{DocResource, ReadTransaction, TransactionResource},
     wrap::SliceIntoBinary,
 };
-Original file line number
+Diff line change
@@ Expand Up / @@ -34,4 +34,5 @@ rustler::atoms! { @@
       delete,
       retain,
       attributes,
     }