Simplify batches/deltas and change event

[pjasiun]

I do not believe anymore that users will create their own Deltas. It will not gonna happen. So we can simplify this piece of the API:

• move the code from the register to the delta constructor,
• introduce ModelDocument.applyDelta https://github.com/ckeditor/ckeditor5-engine/issues/500 (@scofalik will be happy).

I'm not sure about two thinks:

• how relations between Batch, Delta and applyDelta should look like: do I need to pass Batch to the Delta constructor? Do I need to add Delta to the Batch? In fact, I need to add delta to the batch and apply it at the same time so, maybe I should pass both Delta and Batch to the applyDelta and it will add delta to the batch? Or adding Delta to the Batch should apply it since Batch knows about the document?
• if there will be a reach constructor in each Delta class won't it be a problem for the OT where we need to create a delta in a custom way? The simplest solution would be to keep the constructor empty and move the register logic to the init method, which can not be called in the OT, but then we make the API more complicated again.

[Reinmar]

https://github.com/ckeditor/ckeditor5-engine/issues/678 describes the bigger picture of this change.

[Reinmar]

Quick thought, without much of an analysis: Each delta module should expose a function called like today's Batch methods. So instead of:

model.batch().insert( what, where );

you'd do:

const batch = model.createBatch();
insert( batch, what, where );

That insert() would do all the steps:

1. create delta,
2. add it to the batch,
3. apply the delta.

We have hundreds of batch.xxx() usage and this made me think that we must not complicate this too much. So the 3 steps above shouldn't be required to be done manually.

[pjasiun]

Reviewing ckeditor/ckeditor5-engine#793 I realized that we should also update the change event. Now it's very messy. It could be fired not per operation, but per delta and contains operations as internal. Fortunately, because big part of the code is listening on the dispatcher, not on the change event, it might be not that hard to change as it looks like.

[Reinmar]

That's a part of a whole topic – https://github.com/ckeditor/ckeditor5-engine/issues/678.

[pjasiun]

Fixing https://github.com/ckeditor/ckeditor5-engine/issues/1099 we decided to fire change event also when there is no real change (for instance rename to the same value). This is because there is no other event which let you notice that operation was applied and some plugins need such information. We should consider having a more abstract change event for regular plugins which is fired only when there is real change (this is what most plugins except) and some internal event (operationApplied) for deeper integration.

[Reinmar]

A case that we need to consider here is that people will try to use the change event to handle... changes. And the problem with this event is that after each atomic change is applied is that the temporary state may be incorrect.

E.g. see #599 (comment). This code is not only unnecessarily slow but also dangerous because after some changes getData() might not work.

If we'll introduce API for postfixer there's a chance that we should hide the atomic change event and only expose event like changesDone (perhaps with more info) which is going to be the one that people should use. In other words – I don't see use cases for atomic change event.

[pjasiun]

It's partially fixed by https://github.com/ckeditor/ckeditor5-engine/issues/858, https://github.com/ckeditor/ckeditor5-engine/issues/1172 and https://github.com/ckeditor/ckeditor5-engine/issues/1186. The rest will be fixed by https://github.com/ckeditor/ckeditor5-engine/issues/1207 and https://github.com/ckeditor/ckeditor5-engine/issues/1162. We can close this ticket.