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

Allow non-string children in Text objects #238

Merged
merged 17 commits into from
Mar 23, 2020

Conversation

nornagon
Copy link
Contributor

@nornagon nornagon commented Mar 17, 2020

This builds on work from @pvh and @ept to allow non-string objects to be present in Automerge.Text. While the experiments with representing rich-text formatting by "format markers" proved unsuccessful, I think this is still useful for representing "embedded objects" within text.

Closes #194

@nornagon
Copy link
Contributor Author

I think this closes #194?

@ept ept merged commit 8c8c73f into automerge:master Mar 23, 2020
ept added a commit that referenced this pull request Mar 23, 2020
Allow non-string children in Text objects

Fixes #194
ept added a commit that referenced this pull request Mar 23, 2020
@ept
Copy link
Member

ept commented Mar 23, 2020

Merged, thank you!

@ept
Copy link
Member

ept commented Mar 23, 2020

Note for posterity: this PR also includes @pvh's initial implementation of the OAFS model (see #193) in test/text_test.js. For now that seems as good a place as any to keep it, but if we want to make it available as part of the Automerge API we'll have to move it into the main library code.

@nornagon
Copy link
Contributor Author

@ept I don't think this includes any OAFS stuff, though it does contain a pre-OAFS attempt to model rich text using inline markers in the Text object.

@nornagon nornagon deleted the text-control-objects branch March 23, 2020 15:10
philxia added a commit to philxia/automerge that referenced this pull request Jun 27, 2020
* test/watchable_doc_test: move callback registration to beforeEach()

* copy automerge.d.ts to dist on build/publish

* use strict mode everywhere

The original code here suggests that in a browser,
modifying a frozen object will fail silently, whereas
in node it will throw an error. This test was failing
for me in browser testing, so I dug into it and it
turns out the difference isn't node vs browser
but strict mode vs not. (In mocha.opts we had
"use strict" so node tests were always running
in strict mode. TypeScript was also compiling in
strict mode.)

Since we have control over whether we're in strict 
mode or not, it makes more sense to just turn it on
everywhere and only test for that scenario.

screw it, just use strict mode everywhere

* test/types: add CounterList

* @types/automerge/index.d.ts: `message` can be undefined

* restore uuid factory, add typings

* @types/automerge/index.d: add uuid()

* @types/automerge/index.d: getElemId

* put all types in automerge namespace

* test/test: restore message type test

* make `Change` generic: `Change<T>`

* test/proxies_test: restore tests with string indices

* Action parameter is different for Ops vs Diffs

* properly type Table.add so it enforces property order when passing array

* npm: fix name of types file

* Automerge.Text no longer considered experimental

* Undo unnecessary formatting changes

* Revert back to plain JS for tests

* Nicer JSON serialization of Counter, Table, and Text

* Improve JSON serialization of Table

Put the rows in a separate object from the columns, so that applications
can iterate over the rows by calling `Object.values(doc.table.rows)`.

* Changelog for automerge#163 and 0.10.1

* 0.10.1

* actorId and elemId are not necessarily UUIDs

* Remove unused type definitions for tests

* setActorId is frontend-only

* Some corrections of the TypeScript types

* Initial set of tests for the TypeScript API

* Fix undo/redo when using separate frontend and backend

Bug: when using Automerge in a configuration with separate frontend and
backend, attempting to perform `Frontend.undo()` would result in the
following exception:

    TypeError: Cannot read property 'updated' of null
      at makeChange (frontend/index.js:113:43)

Fixed the bug and added tests that exercise undo/redo in the case where
frontend and backend are separate.

* Fix some bugs in type definitions

* Exception on modifying frozen object is optional

In Node v12.3.1 and v10.16.0, assigning or deleting a property of a
frozen object no longer throws an exception like it did in previous
versions. Rather than trying to split the test by version number, I
figured it was easier to just remove the assertion that an exception is
thrown (and just check that the object remains unmodified).

* Tests still to do

* Update caveats in readme

* Exception on modifying frozen object is optional

In Node v12.3.1 and v10.16.0, assigning or deleting a property of a
frozen object no longer throws an exception like it did in previous
versions. Rather than trying to split the test by version number, I
figured it was easier to just remove the assertion that an exception is
thrown (and just check that the object remains unmodified).

* Changelog for automerge#165

* Run Travis tests on Node 11 and 12 as well

* README: note correct immutable behavior

* index.d.ts: remove stray question mark

* typescript tests: Automerge.Text

* typescript tests: Automerge.Table

* no need to specify type twice

* expand on usage of types in initialization

* typescript tests: Automerge.Counter

* ignore .vscode

* add `Automerge.from`

Returns a new document object with the given initial state. See automerge#127

* Document created with `from` needs to have a backend

* Update the documentation for Automerge.from

* Changelog for automerge#127

Closes automerge#175

* Fix indentation and whitespace

* Remove unused code

* Add types for Automerge.from (automerge#127)

* WIP: experiment with readonly types returned outside of Automerge

* @types/automerge/index.d: move comment

* aha! Doc<T> = DeepFreezeObject<T>, not DeepFreeze<T>

* tweak `any` tests to work with DeepFreeze

* group types functionally

* upgrade typescript

* rename to Freeze, Proxy

* more rearranging

* simplify Table definition by inheriting from Array<T>

* getConflicts key is `keyof T`

* add documentation for Doc<T> and Proxy<D>

* formatting

* fix handler signature
automerge/automerge#155 (review)

* add tests for DocSet and WatchableDoc

* add DocSet tests

* ignore .vscode

* Revert "Update the documentation for Automerge.from"

This reverts commit c0ba4d2.

* Make object freezing optional

It turns out that in V8, copying frozen objects (especially arrays) is a
lot more expensive than copying non-frozen objects.
https://bugs.chromium.org/p/chromium/issues/detail?id=980227
https://jsperf.com/cloning-frozen-array/1
https://jsperf.com/cloning-frozen-object/1

Removing freezing entirely from skip_list.js since these objects are not
part of the user-facing API, and so we don't need to worry as much about
accidental mutation.

Retaining freezing in a few places in counter.js, table.js,
apply_patch.js and frontend/index.js where it has negligible performance
impact.

* Allow freeze option to be passed in to init() and load()

* Update README: we no longer freeze objects by default

* Slight README refresh

* Our own object-copying function is faster

Surprisingly, Object.assign() is not the fastest way of copying an
object: https://jsperf.com/cloning-large-objects/1

* Remote type parameter from Change interface

It is only needed for the "before" property, and I think that property
should not be considered part of the public API. (It is only used within
the frontend for internal bookkeeping; the Change objects produced and
consumed by the backend do not have this property.)

* Use opaque BackendState type in the backend

* Remove unused type parameter from Message interface

* Update the documentation for Automerge.from

* Changelog and README updates for automerge#155

* TypeScript support for freeze option

* Changelog for automerge#177 and automerge#179

Fixes: automerge#177

* Changelog for v0.11.0

* 0.11.0

* Allow Text instatiation with preset value

* docs: Add introduction to "Applying operations to the local state"

* `DocSet<T>` was missing `docIds` property

* `DocSet.getDoc` return value is `Doc<T>,` not `T`

# Conflicts:
#	@types/automerge/index.d.ts

* `Clock` is a `Map`, not an object

* allow passing `init` options to `from`

* index.d.ts: more precise InitOptions definition; add InitOptions to .from

* add `freeze` to `InitOptions`

* add tests for `from` with options

* refactor: Remove unused `local` list

* docs: Add local state overview

* Types for `Frontend.init()` and `Frontend.from()`

Addendum to automerge#183

* Changelog for automerge#183

* Some refactoring/tidying

Name `instantiateText` is for consistency with `instantiateTable`

* Allow modification of Text without assignment to document

Methods `.set()`, `.insertAt()` and `.deleteAt()` are now defined
directly on Automerge.Text, rather than going through the listProxy.
Therefore, these methods can now be called directly on a Text object
without having to first assign the Text object to the document.

Fixes automerge#180, automerge#166

* Changelog for automerge#180 and automerge#181

* List methods such as .filter() should use proxy objects

Fixes automerge#174

* Upgrade npm dependencies

- Remove `@types/sinon` and `@types/uuid` dependencies, which are unused.
- Update all version numbers in package.json to their latest version,
  except for `babel-loader`, which I'm keeping unchanged for now
  (upgrading it to 8.0.6 breaks `yarn build`, and I can't be bothered to
  figure that out now).
- Run `yarn upgrade` to bring packages up-to-date. The new version of
  `get-caller-file` dropped support for Node 9, and since that version
  of Node is now unsupported anyway, I dropped it from the Travis CI
  test matrix.
- Version 4.0.0-alpha.4 of `selenium-webdriver` requires Node 10 or
  higher. As Node 8 is still supported, I want to keep it in the CI text
  matrix. Thus, I had to add a resolution rule fixing `selenium-webdriver`
  to 4.0.0-alpha.1, which is compatible with Node 8.

* docs: Add description for determining causal readiness

* docs: Add missing operations

* .toString() of Text should return a string content

* test.js: add tests for concurrent deletion

* Fix stack overflow error on large changes

Fixes automerge#202. The stack overflow is not due to unbounded recursion, but
rather because of a method's varargs argument list being too long
(presumably each argument takes up some space on the stack). In
particular, there are several places where we are calling Array.push
with an argument list generated using the spread operator in order to
concatenate arrays. This commit replaces them with for loops, which also
happen to be faster: https://jsperf.com/pushing-large-arrays

* Changelog for 0.12.0

* 0.12.0

* test/typescript_test.ts: docSet.docIds

* Revert "`Clock` is a `Map`, not an object"

This reverts commit d8fd73f.

* Changelog for automerge#174, automerge#184, and automerge#199

* Readme updates

* Fix trap error by setting writable property descriptor to true

* add initialization tests for array, primitives

* fix typo in test name; make describe labels consistent

* readme: automatic formatting

* readme: initializing a document

* readme: updating a document

* readme: rearrange outline

* readme: actorId note

* readme: undo/redo edits

* readme: sending/receiving edits

* readme: conflicting changes edits

* readme: crdt datatypes edits

* readme: emoji

* readme: cross-references

* Tidy up description of network layers

* Text.toString()

* Add DocSet.removeDoc

* Changelog for automerge#210 and 0.12.1

* 0.12.1

* Remove unused parameter in SkipList node

Fixes automerge#213

* a test for round-tripping a control object

* split out tests a bit

* Allow objects to appear as elements in Automerge.Text

Fixes automerge#194

* toSpans() and a test set for it

* add to types

* okay, maybe not any

* quill delta experiment

* show that overlapping spans work

* basic apply-delta function

* account for control characters occupying space during delete & retain

* clean up the op application functions a little bit

* parse embeds from deltadocs

* make it possible to distinguish between attributes and embed control characters

* finish support for embeds

* explicitly account for control characters vs embeds

* Fix freezing of opaque string types

* Include link to repo

* docs(readme): add perge library to list under sending/receiving changes

* Update README.md

* Allow options object to be passed in to Automerge.change

Similarly in emptyChange(), undo(), and redo()

* canUndo should return false after Automerge.from()

* Bump handlebars from 4.1.2 to 4.5.3

Bumps [handlebars](https://github.com/wycats/handlebars.js) from 4.1.2 to 4.5.3.
- [Release notes](https://github.com/wycats/handlebars.js/releases)
- [Changelog](https://github.com/wycats/handlebars.js/blob/master/release-notes.md)
- [Commits](handlebars-lang/handlebars.js@v4.1.2...v4.5.3)

Signed-off-by: dependabot[bot] <[email protected]>

* fix deleteAt bug with input 0

* New `Automerge.getAllChanges()` API

* Bring internals documentation up-to-date

* Document frontend-backend protocol

* Bring the rest of the internals doc up-to-date

* Reword intro

* Fix indentation

* Changelog for v0.13.0

* 0.13.0

* Update copyright

* Add link to automerge-client-server

* Rephrase caveats

* remove tests for adding a Table row as an array of values

* Remove KeyOrder array from Table type

* remove KeyOrder array from types in tests

* clean up documentation

* remove undocumented ability to add table row from array of values

* remove test for API being removed

* Bump acorn from 6.2.1 to 6.4.1

Bumps [acorn](https://github.com/acornjs/acorn) from 6.2.1 to 6.4.1.
- [Release notes](https://github.com/acornjs/acorn/releases)
- [Commits](acornjs/acorn@6.2.1...6.4.1)

Signed-off-by: dependabot[bot] <[email protected]>

* support more nesting & fix retain logic

* use string concat instead of join()

* Update dependencies (yarn upgrade)

* Updating dependencies

* Throw exception if people try to use the old API

* Remove obsolete Node 8 and 11 from testing matrix

* Changelog for automerge#236

* Get Sauce Labs tests working again

* Update Sauce Labs build badge

* Remove the list of columns from Automerge.Table

Since automerge#236 the Automerge.Table type no longer allows rows to be added as
an array of values, which are then mapped to column names. Now that this
feature is removed, nothing in Automerge is actually using the list of
columns that is currently required by the Automerge.Table constructor.
The columns are still saved, but since they do not enforce any schema,
they are purely advisory.

Hopefully one day we can have proper schema support in Automerge, but
the current half-hearted implementation is not really helping us get
there. Therefore I think it is best to just remove this list of columns
feature.

* Update Mocha configuration file format

mocha.opts is deprecated. This stops the annoying deprecation warning
when running the tests.

* Mocha should load test files

* Remove trailing whitespace

* Changelog for automerge#238

* Make table row's primary key available as `id` property

Currently, if you have a row object from an Automerge.Table, you can get
its primary key with `Automerge.getObjectId(row)`. This API is not very
discoverable; users who don't know about this API might be tempted to
generate their own IDs for table rows, which would miss the entire point
of Automerge.Table.

This patch makes the primary key more visible by making it available as
`row.id` instead. When a new row is added, we check that the row object
doesn't already have an `id` property. We also ensure that the `id`
property cannot be modified.

(Besides API usability, there is also a deeper reason for making this
change: on the `performance` branch, objectIds are backend-generated
Lamport timestamps rather than UUIDs to enable better compression; since
`Table.add` should synchronously return the primary key of the new row,
it must use a different ID from the objectId. Putting the row's primary
key in a separate property reinforces that distinction.)

* Fix description of Table in README

* Changelog for automerge#241 and automerge#242

* Remove set() method from Automerge.Table API

It is not needed, since add() and remove() handle changes to the set of
rows, and properties of row objects can be updated directly without
requiring any set() operation at the table level.

Looking at the code, I now realise that set() was not intended to be
part of the public API at all: it is called while applying a patch, and
if a user calls it, it does not generate any operations in the change
context. Hence I renamed it to start with an underscore (like `_clone()`
and `_freeze()`), and added a warning to the comment.

* Changelog for automerge#243

* Changelog for 0.14.0

* 0.14.0

* Link to new CRDT website

* Update table class type to reflect property getter

* Changelog and test for automerge#249

* Fix console inspection of proxy objects in Node

* Fix type signatures for WatchableDoc#get and WatchableDoc#set

* Use Slack's own invitation link instead of communityinviter

* Queued changes (whose dependencies are missing) should also be saved

Previously, load(save()) would discard any queued changes.

Bug reported by @KarenSarmiento, fixes automerge#258

* Make clearer that fine-grained updates are preferred

Add a README section as suggested by @johannesjo in automerge#260, and make a
more descriptive exception when users try to assign an object that is
already in the document.

Fixes automerge#260

* Changelog for 0.14.1

* 0.14.1

* Link to automerge#253 from README

* support calling indexOf with an object

* logo assets

* replace h1 with logo

* README.md: smaller logo

* images with wider spacing

* revert unintended reformatting

* Update name of main branch

fixes automerge#264

Co-authored-by: Herb Caudill <[email protected]>
Co-authored-by: Martin Kleppmann <[email protected]>
Co-authored-by: Herb Caudill <[email protected]>
Co-authored-by: Harry Brundage <[email protected]>
Co-authored-by: Irakli Gozalishvili <[email protected]>
Co-authored-by: Eric Dahlseng <[email protected]>
Co-authored-by: Peter van Hardenberg <[email protected]>
Co-authored-by: Mikey Stengel <[email protected]>
Co-authored-by: Brent Keller <[email protected]>
Co-authored-by: Jeff Peterson <[email protected]>
Co-authored-by: Max Gfeller <[email protected]>
Co-authored-by: Sam McCord <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Speros Kokenes <[email protected]>
Co-authored-by: Jeremy Apthorp <[email protected]>
Co-authored-by: Lauritz Hilsøe <[email protected]>
Co-authored-by: vincent.capicotto <[email protected]>
Co-authored-by: Phil Schatz <[email protected]>
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.

Inserting non text characters into Automerge.Text
3 participants