docs(delta, op, attr): Adds typespecs to all files; Copies README documentation for OT functions

[vherr2]

Context

I decided to read through the implementation to get a better understanding of the Delta format and OT as a concept! In doing so, I thought that typespecs might add some value for interfacing with the library, though I didn't want to pull in Dialyzer (or other tools) as library dependencies. This was primarily an exercise for myself, so feel free to close this if the changes were intentionally omitted for whatever reason!

What's Changing

• Copies docs and examples for
  • Delta.compose/2
  • Delta.transform/3
  • Delta.invert/2
• Adds typespecs for all functions for Delta, Delta.Op, and Delta.Attr

Additional Notes/Questions

There were a number of questions that this PR led me to, which I would love some clarity on, time permitting 🙂

Delta.Op String keys

I noticed that all of the Delta operators are represented as maps with specific keys indicating the type of operation, e.g. "insert". I was expecting this to be modeled with structs/records/maps with atoms as keys, but I assumed there was likely a reason for not doing so:

• Avoiding overhead costs for marshalling data between JSON formats and library structs
• Parity/interop with quilljs/delta
• Breaking interface change for direct calls to some functions
• All/some of the above?

I wasn't entirely sure I had appropriate context to do any representative benchmarking and the above reasons made it clear that I needed more understanding before diving in. My hunch is that working with atoms might add some nice structure to the codebase, and replace the need to have functions such as Op.insert?/2, Op.type?/3, and Op.info/1 and maybe (very marginally) improve some runtime performance, but I'm also suspicious that any of that is true or worth it since it's unclear what, if any, need there is to extend the core set of Delta Operators.

Public vs Private functions

Based on the README, I was expecting a smaller number of functions to be exposed publicly. In comparing with the quilljs/delta, it seems more likely that there's just missing documentation in the README; is that true?

Unreachable case in Delta.do_split/5

In pattern matching the result of Op.take(first, index, opts), it appears the first case is unreachable. After removing it, the tests continued to pass, though I was unsure of if that was representative of usage.

Delta.EmbedHandler behaviour

The usage of this in the test suite confused me -- between the documentation and the callback specs, I couldn't really tell if an Embed was more like a nested Delta, e.g. a list() (based on the Delta.Support.TestEmbed) or a special type of content value, e.g. map() based on the type and usage throughout Delta.Op

[martosaur]

Hi @vherr2! 👋

Thank you for you contribution! The changes look good to me, I left just a couple of minor comments. There's also a merge conflict due to added attribute support for delete operation, but should be fairly easy to resolve.

If you don't have time to put into this PR just let me know and I'll take it over. Also please let me know if you want this PR to count towards Hacktoberfest, we can totally make this happen!

---

Let me go through your questions and try answering them to the best of my knowledge.

Delta.Op String keys

You're correct with your list of reasons, the first one being the biggest. I'll add that from my experience working with delta in the app code it's very convenient to always have this "elixir oblivious" representation, as there is less stuff to keep in mind.

Based on the README, I was expecting a smaller number of functions to be exposed publicly. In comparing with the quilljs/delta, it seems more likely that there's just missing documentation in the README; is that true?

Yeah, as you can see documentation wasn't this library's strong side. But thanks to this PR things gonna change! 🎉

In pattern matching the result of Op.take(first, index, opts), it appears the first case is unreachable. After removing it, the tests continued to pass, though I was unsure of if that was representative of usage.

Good catch! Indeed this branch looks redundant. Don't worry about removing the clause, I'll take care of it and also will test it against the application test suite.

The usage of this in the test suite confused me -- between the documentation and the callback specs, I couldn't really tell if an Embed was more like a nested Delta, e.g. a list() (based on the Delta.Support.TestEmbed) or a special type of content value, e.g. map() based on the type and usage throughout Delta.Op

I think you correctly figured that Delta.EmbedHandler.embed type is a type of embed itself which is single key map. But it looks like specs for callbacks are off, as the value an embed stores (and therefore passed to the callback functions) can be pretty much anything. TestEmbed uses delta (i.e. list of operations) as its content. Image embed from the example uses string/link. Thanks for spotting this!

[martosaur]

I believe this is a list of deltas

Suggested change

	@spec compose_all(t) :: t
	@spec compose_all([t]) :: t

[martosaur]

Hmm, I'd vote for omitting typespecs for private functions as they don't add much clarity, but do add clutter. Especially for cases like these, where do_* function is basically a continuation of parent function implementation.

[martosaur]

Good job on documenting the most obscure operation!

[martosaur]

I gotta ask, what is a stand-in type?

[martosaur]

Shouldn't it also be String.t instead of bitstring?

[martosaur]

Heads up for when/if you'll be rebasing this: deletes can now have attributes too!