WIP: feat: ignore transactions which were not added to history

phikes commented

2020-08-04 18:37:18 +02:00

(Migrated from github.com)

prosemirror-history reads a metadata field addToHistory and doesn't
allow to undo changes if a transactions sets it to false. I think its
reasonable that this plugin ignores these transactions as well as they
are not undoable by the undo command anyways.

`prosemirror-history` reads a metadata field `addToHistory` and doesn't allow to undo changes if a transactions sets it to `false`. I think its reasonable that this plugin ignores these transactions as well as they are not undoable by the undo command anyways.

👍 3

marijnh commented

2020-08-04 18:43:18 +02:00

(Migrated from github.com)

I think its reasonable that this plugin ignores these transactions as well as they are not undoable by the undo command anyways.

Why?

> I think its reasonable that this plugin ignores these transactions as well as they are not undoable by the undo command anyways. Why?

phikes commented

2020-08-04 18:53:24 +02:00

(Migrated from github.com)

If your questions targets the reasoning why the plugin should ignore transactions with addToHistory set to false the reasoning is in the second part of the sentence. My scenario is as follows:

an input rule transaction happens
our state is configured to dispatch other transactions which are "behind the scenes" transactions (in that case they adjust UUIDs) and as such are set to be ignored in the history
these transactions overwrite the input rules plugin's state so that the original input rule transactions cannot be undone anymore by the undoInputRule command

If your questions targets the reasoning why the plugin should ignore transactions with `addToHistory` set to `false` the reasoning is in the second part of the sentence. My scenario is as follows: * an input rule transaction happens * our state is configured to dispatch other transactions which are "behind the scenes" transactions (in that case they adjust UUIDs) and as such are set to be ignored in the history * these transactions overwrite the input rules plugin's state so that the original input rule transactions cannot be undone anymore by the `undoInputRule` command

phikes commented

2020-08-04 19:06:10 +02:00

(Migrated from github.com)

Btw since this is probably a breaking change I could also add a configuration option to inputRules so it's opt-in behaviour:

export function inputRules({rules, onlyTransactionsInHistory = false}) {

Btw since this is probably a breaking change I could also add a configuration option to `inputRules` so it's opt-in behaviour: ``` export function inputRules({rules, onlyTransactionsInHistory = false}) { ```

marijnh commented

2020-08-04 20:13:52 +02:00

(Migrated from github.com)

Ah, right, you're trying to avoid clearing the undo logic when something like a collaborative change comes in. But as it is, this patch isn't correct—if, for example, a non-history change inserts or deletes content before the last inputrules change, the positions in that stored change no longer align with the current document.

phikes commented

2020-08-05 11:09:31 +02:00

(Migrated from github.com)

Thanks for the response! Makes total sense. I am fairly new to Prosemirror and am trying to figure out how to solve the problem you mentioned. As soon as there is a change that affects not only the position of an original input rule change, but also maybe it's contents and that change is not added to history I think we could not recover. As such I'd think of two options:

Make the ignoring bit opt-in (see above; in our case we can safely ignore the non-history changes e.g.)
Adjust the transaction in the state in the input rules plugin as long as we are only shifting positions and not changing the content it describes (in addition to it having addToHistory !== false)

Thanks for the response! Makes total sense. I am fairly new to Prosemirror and am trying to figure out how to solve the problem you mentioned. As soon as there is a change that affects not only the position of an original input rule change, but also maybe it's contents and that change is not added to history I think we could not recover. As such I'd think of two options: * Make the ignoring bit opt-in (see above; in our case we can safely ignore the non-history changes e.g.) * Adjust the transaction in the state in the input rules plugin as long as we are only shifting positions and not changing the content it describes (in addition to it having addToHistory !== false)

marijnh commented

2020-08-05 11:18:47 +02:00

(Migrated from github.com)

Look into position mapping—it is probably possible to update the undoable data in a safe way.

Look into [position mapping](https://prosemirror.net/docs/guide/#transform.mapping)—it is probably possible to update the undoable data in a safe way.

👍 1

phikes commented

2020-08-05 13:02:31 +02:00

(Migrated from github.com)

This is what I thought about, however we would still need to discard the transactions as soon as another transaction comes along which modifies inside of the (mapped) range of the original transaction, no?

So in pseudo-code something like this?

stored = tr.getMeta(this)
if (stored) return stored

if (tr didNotChangeAnything in prev's content) { return prevWithMappedPositions }

return null // content changed inside of the original input rule tr or new input rule tr

This is what I thought about, however we would still need to discard the transactions as soon as another transaction comes along which modifies inside of the (mapped) range of the original transaction, no? So in pseudo-code something like this? ``` stored = tr.getMeta(this) if (stored) return stored if (tr didNotChangeAnything in prev's content) { return prevWithMappedPositions } return null // content changed inside of the original input rule tr or new input rule tr ```

marijnh commented

2020-08-05 13:21:13 +02:00

(Migrated from github.com)

we would still need to discard the transactions as soon as another transaction comes along which modifies inside of the (mapped) range of the original transaction, no?

This is the same case as step rebasing for history or collab editing. You can map steps through anything. Sometimes the result will be null (the changed content was deleted) and sometimes the step can't be applied anymore (maybeStep will return failure), so this becomes a bit more tricky to manage (probably don't want to store the plain transaction anymore, but rather an array of steps), but should be doable.

> we would still need to discard the transactions as soon as another transaction comes along which modifies inside of the (mapped) range of the original transaction, no? This is the same case as step rebasing for history or collab editing. You can map steps through anything. Sometimes the result will be null (the changed content was deleted) and sometimes the step can't be applied anymore (`maybeStep` will return failure), so this becomes a bit more tricky to manage (probably don't want to store the plain transaction anymore, but rather an array of steps), but should be doable.

phikes commented

2020-08-05 13:22:06 +02:00

(Migrated from github.com)

Thanks for the response, really appreciate it. I will see if I can make sense out of it and build it into my patch.

marijn closed this pull request

2026-04-01 20:55:19 +02:00

Pull request closed

This pull request cannot be reopened because the branch was deleted.

Rows
Columns

WIP: feat: ignore transactions which were not added to history #4

Pull request closed