[whatwg] Feedback on UndoManager spec

On Wed, Oct 26, 2011 at 10:57 AM, Aryeh Gregor <ayg at aryeh.name> wrote:
>>> 15) Is the isReapply parameter to apply() needed? ?The only place I
>>> see where it's used is if the author specifies a manual transaction
>>> but leaves off a reapply() method. ?In that case, why not just call
>>> apply() with no extra parameter? ?If the author wanted to have apply()
>>> and reapply() behave differently, they could have specified a separate
>>> reapply() method.
>>
>> There are good arguments made by Jonas on this topic.
>> Please look at whatwg archives?on this topic.
>
> I looked at the archives and didn't see any good arguments. ?As far as
> I can tell, if authors wanted behavior like with the isReapply
> parameter, they could easily emulate it by changing
>
> ?{ apply: f }
>
> to
>
> ?{ apply: function() { f(true) }, reapply: function() { f(false) } }
>
> so the extra isReapply parameter doesn't give any extra control to
> authors. ?It just adds a second way to do the same thing, and
> complicates the API. ?It would be simpler and easier to understand if
> authors just had to write the extra line or two of code and specify
> separate functions for apply/reapply always, instead of being able to
> specify either two functions or one that takes a boolean parameter to
> achieve the same effect.

I think that in most implementations of the apply/reapply functions,
the differences between the functions will be minimal. I.e. most of
the time you'll want to made the same modifications to the document
the second time a action is applied as you did the first time.

By splitting it out into two callbacks we encourage people to
duplicate that code.

I'd much rather see code like:

transact = { apply: function(reapply) {
  <do lots of DOM modifications>;
  if (reapply) {
    <do reapply specific mutations>;
  }
  unapply: ...
} }

/ Jonas

Received on Wednesday, 26 October 2011 20:30:24 UTC