Skip to content

Add beforeHistoryChange event#1539

Closed
aslushnikov wants to merge 2 commits intocodemirror:masterfrom
aslushnikov:before-history-change
Closed

Add beforeHistoryChange event#1539
aslushnikov wants to merge 2 commits intocodemirror:masterfrom
aslushnikov:before-history-change

Conversation

@aslushnikov
Copy link
Copy Markdown
Contributor

Intent: the event is useful to get notifications about
future changes of the document so that one might be able
to update his own external document representation on an incremental
basis. This event is complimentary to the "beforeChange" event which
may be used for this purpose but which doesn't fire for the undo/redo
commands.

Intent: it is useful to get notifications about
future changes of the document so that one might be able
to update his own external document representation on an incremental
basis. This event is complimentary to the "beforeChange" event which
may be used for this purpose but which doesn't fire for the undo/redo
commands.
@marijnh
Copy link
Copy Markdown
Member

marijnh commented May 22, 2013

This is a good idea. But how about moving the signal call to makeChangeFromHistory and feeding it a regular change object similar to the one that beforeChange gets?

@aslushnikov
Copy link
Copy Markdown
Contributor Author

Do you mean smth like this?

marijnh added a commit that referenced this pull request May 24, 2013
But disable its update method in that case.

Issue #1539
@marijnh
Copy link
Copy Markdown
Member

marijnh commented May 24, 2013

Yes, though -- sorry to waste your time -- I decided that actually that's not what I want either. I've pushed a different change (see attached patch) where beforeChange is also fired on undo events, but without the update method. Since that was the only part that was problematic to support.

Does that work for you?

@aslushnikov
Copy link
Copy Markdown
Contributor Author

That works for me, thank you for your time.

@aslushnikov aslushnikov deleted the before-history-change branch October 20, 2014 14:44
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.

2 participants