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

Batch type #3729

Closed
pjasiun opened this issue May 25, 2016 · 9 comments
Closed

Batch type #3729

pjasiun opened this issue May 25, 2016 · 9 comments
Assignees
Labels
package:engine type:feature This issue reports a feature request (an idea for a new functionality or a missing option).
Milestone

Comments

@pjasiun
Copy link

pjasiun commented May 25, 2016

We need the way to initialise data in the editor. The problem is that on the one hand we need a change event, so the data will be converted to the view, but we do not want to have an undo step for the initial data.

@pjasiun
Copy link
Author

pjasiun commented May 25, 2016

More I think about it more I feel that it might be useful to mark batches which should be ignored in the undo stack.

It would work for both initialisation and collaborative editing.

For now, we agreed that deltas from other users can be applied without any batch. Such batch-less deltas would be ignored then by the undo feature. But what if we want to show group of changes introduced by certain user, for instance in suggestion mode? My point is, that these changes, introduced by other, which we want to ignore in the undo stack, may need to be grouped too, batches works for such grouping very well.

Also note that creating deltas, without a batch is not an easy task, right now. So for initialisation it would not be the simplest solution.

We can not also not put these changes to the history, because then the document version will be different then the number of operations in the history, what can be a reason of issues, especially when you think about collaboration and sending this initial data for other users.

@pjasiun
Copy link
Author

pjasiun commented May 25, 2016

Also note that re-initialization is a different case. When a use call setData and we decide it should reset the history we can do it. We can reset the model by clearing the history, removing all roots and reseting base version. Then we can create a root and set data the way we do it at the first place. Note that this way history will be empty but also match document version.

@pjasiun pjasiun changed the title Init data Batch type May 27, 2016
@pjasiun
Copy link
Author

pjasiun commented May 27, 2016

In https://github.com/ckeditor/ckeditor5-typing/issues/7 we realised that changes caused by undo should break typing batch, but changes causes by collaboration should not. This is why be can not put all of them as deltas without a batch. We need to group these changes in batches and created batch types.

For now we need special batches for:

  • collaboration,
  • undo/redo,
  • initialization.

@scofalik
Copy link
Contributor

scofalik commented Jun 1, 2016

When we made this change we can consider whether it is a good practice to use ModelDocument#applyOperation without using a batch. I.e. in undo there is a code:

for ( let delta of updatedDeltas ) {
    for ( let operation of delta.operations ) {
        this.editor.document.applyOperation( operation );
    }
}

This could be change to "transparent" batch.

I am writing this because in some places we already made kind-of-an-assumption that we check if applied Operation has a batch. If not, we treat it as "transparent" operation, that should not be considered by, i.e. undo mechanisms or typing mechanisms. Now we would be able to just check batch type instead of checking if batch is equal to null.

@Reinmar
Copy link
Member

Reinmar commented Jun 1, 2016

The idea about batch types was that you'll be checking them instead of whether the batch exists or not... so I guess that this means that you'll always need to generate batches. Does it sound feasible?

@scofalik
Copy link
Contributor

scofalik commented Jun 1, 2016

Yes, you wrote basically what I wrote. I just wanted to point out that there already might be places that will need to be fixed.

@pjasiun pjasiun assigned pjasiun and scofalik and unassigned pjasiun Jun 14, 2016
@scofalik
Copy link
Contributor

@scofalik
Copy link
Contributor

For now we will stick only with two types:

  • default - for almost all batches
  • transparent - for batches that should be ignored by features, for now this is initial batch and OT batches (OT batches should be ignored by typing, undo and possible other features).

@pjasiun
Copy link
Author

pjasiun commented Jun 24, 2016

Closed via: ckeditor/ckeditor5-engine#499.

@pjasiun pjasiun closed this as completed Jun 24, 2016
@mlewand mlewand transferred this issue from ckeditor/ckeditor5-engine Oct 9, 2019
@mlewand mlewand added this to the v0.1.0 milestone Oct 9, 2019
@mlewand mlewand added type:feature This issue reports a feature request (an idea for a new functionality or a missing option). package:engine labels Oct 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:engine type:feature This issue reports a feature request (an idea for a new functionality or a missing option).
Projects
None yet
Development

No branches or pull requests

4 participants