[whatwg] Fixing undo on the Web - UndoManager and Transaction

On 11-08-09 6:36 PM, Jonas Sicking wrote:
> On Tue, Aug 9, 2011 at 3:11 PM, Ryosuke Niwa<rniwa at webkit.org>  wrote:
>> On Tue, Aug 9, 2011 at 2:55 PM, Jonas Sicking<jonas at sicking.cc>  wrote:
>>> I don't think it's a matter of which use cases can or can't be solved with
>>> either solution. It's pretty clear to me that all scenarios can be solved
>>> with either API.
>>
>> Right, they're isomorphic.
>>>
>>> It's just a matter of which pattern is more common and so which one we
>>> should make more convenient. If almost everyone puts the same code in apply
>>> and reapply then we're just creating more work for people.
>>>
>>> Here's how you'd implement the apply/reapply/unapply syntax using simply
>>> apply/unapply
>>>
>>> apply: function() { if (!this.applied) { action1(); this.applied = true; }
>>> else { action2(); }
>>> unapply: unapply
>>
>> One thing I don't like about this approach is that it requires a flag.  With
>> my proposal, all you need to do is to call a function. Also, we can make it
>> so that when you don't supply a value to reapply (i.e. reapply is null),
>> then undoManager automatically calls apply instead (I always intended this
>> behavior from the beginning but I've apparently left this details out).
>
> Sure, your API is more convenient in certain situations. But it also
> encourages code duplication (I'll note that in the examples you
> originally provided in this thread you always ended up duplicating
> code between apply/reapply), which easily leads to bugs.

I think this is a very important point, and this downside makes me think 
that we shouldn't expose a reapply API.  Since almost every case that I 
can think of where having a reapply function would make more sense 
requires authors to maintain other state information in their 
transaction objects, I don't think that maintaining one more boolean 
flag is going to make things noticeably harder for them.

Ehsan

Received on Friday, 12 August 2011 15:07:43 UTC