This repository has been archived by the owner on Sep 6, 2021. It is now read-only.
Specify edit origin for color editor widgets in order to batch undos #2790
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ghost
assigned 
|
Reviewing... |
| * If origin starts with "+", then consecutive edits with the same origin will be batched for undo if | ||
| * they are close enough together in time. | ||
| * If origin starts with "*", then all consecutive edit with the same origin will be batched for | ||
| * undo. |
There was a problem hiding this comment.
Do we need to state that if you use neither prefix, the edits will never get batched? (Barring a batchOperation() that is).
|
Ready for re-review. Also added a simple unit test. |
|
Looks good -- merging. (I noticed one nit, that the type declaration on _origin is wrong, but we can fix that later esp. since it's not public API). |
peterflynn
added a commit
that referenced
this pull request
Feb 8, 2013
Specify edit origin for color editor widgets in order to batch undos
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #2721. @peterflynn -- could you look at this one?
This takes advantage of the newly-exposed "origin" parameter to
replaceRange()in CodeMirror. For each color editor, we create a unique origin string. This makes it so that all consecutive edits from a given color editor are batched into a single undo. This is slightly different from our v2 behavior, where the edits would only be batched if they were close together in time. We could implement that heuristic instead, but batching all edits from a given editor together seems to make sense as well, and is more predictable. (Of course, if there are intervening edits elsewhere in the document, they will interrupt the undo batching, as you would expect.)Note that the change to
Editor._applyChanges()isn't actually necessary for this bug fix and probably won't have any effect in the codebase today, but it seems to make sense semantically (i.e., if undos are batched in the underlying document, they should be batched in the synchronized editor, and vice versa). That issue should go away anyway when we switch to CM's own doc/view separation implementation.