[whatwg] Feedback on UndoManager spec

[Ehsan Akhgari]

As I mentioned on #whatwg today, I'm fine with this proposal.

Cheers,
Ehsan

----- Original Message -----
> From: "Ryosuke Niwa" <rniwa at webkit.org>
> To: "Jonas Sicking" <jonas at sicking.cc>, "Aryeh Gregor" <ayg at aryeh.name>, "Ojan Vafai" <ojan at chromium.org>, "whatwg"
> <whatwg at lists.whatwg.org>, "Ehsan Akhgari" <ehsan at mozilla.com>
> Sent: Wednesday, November 30, 2011 3:05:39 PM
> Subject: Re: [whatwg] Feedback on UndoManager spec
> I've talked about this with Jonas and Ehsan on #whatwg. Jonas made a
> proposal that we can void this issue by renaming automatic
> transaction's apply to "implement" (name TBD) without the argument so
> that we can differentiate automatic transactions from manual
> transactions. So we can do:
> 
> 
> 
> editor.undoManager.transact({implement: function() { ... }});
> editor.undoManager.transact({implement: function() { ... }, reapply:
> function() {...}});
> 
> 
> while still letting apply take an argument for manual transactions:
> 
> 
> 
> editor.undoManager.transact({apply: function(type) { if (type) ...
> else ... }});
> editor.undoManager.transact({apply: function() { ... }, reapply:
> function() {...}});
> 
> 
> - Ryosuke
> 
> 
> On Sat, Nov 26, 2011 at 4:41 PM, Ryosuke Niwa < rniwa at webkit.org >
> wrote:
> 
> 
> Calling apply, unapply, and reapply methods for automatic DOM
> transaction poses a problem if we merge apply and reapply because now
> whenever you define an automatic transaction, you'd have to check the
> argument and bail out as needed:
> editor.undoManager.transact({apply: function (isReapply) {
> if (isReapply)
> return;
> ...
> }, isAutomatic: true});
> 
> as supposed to:
> 
> 
> 
> editor.undoManager.transact({apply: function (isReapply) {
> ...
> }, isAutomatic: true});
> 
> 
> This is strictly worse than having apply, unapply and reapply methods.
> UA now makes a call to apply on reapply even though the author isn't
> going to do anything, and the author has to manually check the
> argument in very simple case where he/she wants the UA to take care of
> everything.
> 
> 
> - Ryosuke
> 
> 
> 
> 
> 
> On Fri, Oct 28, 2011 at 1:31 PM, Jonas Sicking <jonas at sicking.cc>
> wrote:
> 
> 
> 
> On Fri, Oct 28, 2011 at 12:20 PM, Aryeh Gregor < ayg at aryeh.name >
> wrote:
> > On Fri, Oct 28, 2011 at 2:47 PM, Ojan Vafai < ojan at chromium.org >
> > wrote:
> >> On Fri, Oct 28, 2011 at 11:36 AM, Aryeh Gregor < ayg at aryeh.name >
> >> wrote:
> >>> It's a few extra characters. I really think the increase in
> >>> clarity
> >>> is worth it. Boolean parameters are much more confusing, and
> >>> should
> >>> be avoided wherever possible.
> >>
> >> +1. I'm also OK with the argument if it's a string.
> >
> > Jonas?
> 
> I'm ok with the string argument too. But I'm also fine with keeping it
> a boolean. I don't really see the risk that people will use the wrong
> interpretation of the boolean and that that wrong impression would
> spread through copy-past as being a very real risk.
> 
> I agree boolean arguments can be a pain. But they are more of a pain
> on the caller side than on the callee side.
> 
> / Jonas

-- 
--
Ehsan Akhgari
ehsan at mozilla.com
http://ehsanakhgari.org/