El jue, 01-10-2009 a las 21:27 +0200, Henrik Sperre Johansen escribió: > I'm saying that in my eyes, it seems like the best option because: > - It keeps the current behaviour of handling empty input and cancel the > same way. At least in my experience, this is the common case. > - It corrects the deficiency that there's currently no way to handle the > two differently (which in my eyes is the real bug). > - Using a default where you're likely to have to handle nil and empty in > the same way seem (to me at least) in some ways just as bad as > implementing do: on Object. > - The new method request:onCancel: will show up right below request:, so > it's easy to spot if you find yourself in the exceptional situation > where you DO need to handle these cases differently. (Especially with a > method comment explaining that's exactly what it's intended for) > - As an added benefit, it also keeps compatability with Squeak :)
Very well spoke but I'm not convinced. If there are a correct method (request:onCancel:) the class comments will suggest to use it for everything, just passing an empty block to cancel if you don't want to do anything. Besides, the request method will exist and will be not removed. So in the image we have code that is not to be used and new code that is suggested to be used. With time (if the laziness isn't big) some packages will be converted to use the new suggested code. Some of them will never do. The problem is, we have the opportunity to fix it, one and for all, accepting the work that that means (but not very hard because isn't something like using continuations, threads or using the thisContext methods, so everyone could do it with just a couple minutes involved). Pharo has already walk a long way in improving and unloading not so good/unwanted code. The same will happen in next milestones when the FileDirectory code is removed. *That* is really a hard breakage. This is in comparison, just a simple find senders, rewrite method on first package load on Pharo. So, again, I vote for the complete fixing and not give a second thought on the Squeak compatibility. Of course, I will contribute coding time to help this task finish as soon as posible. Just MHO. > > Cheers, > Henry > > On 01.10.2009 19:07, Stéphane Ducasse wrote: > > Thanks henry > > Now we will to make a synthesis of all the options and pros and cons. > > I'm too tired to get a clear idea.n but what you are saying that this > > is better > > to keep resquest funky behavior and introduce request:onCancel: > > > > Stef > > > > > >>>> I see two options: > >>>> > >>>> - yours is to complicate the API in order to keep backward > >>>> compatibility with the code base and thus avoiding bugs. > >>>> - mine is to fix the problem once and for all. Yes, it also means > >>>> bugs > >>>> will appear during the period of fixing which can take some time > >>>> > >>> the point andreas is to deprecate request:* so that when you see > >>> where > >>> to change instead > >>> of getting bitten. > >>> > >>> Stef > >>> > >> Deprecation sounds like a bad solution in this case to me. > >> It's useful when functionality has either been > >> - Moved > >> - Removed > >> - Changed in a way that results in a silent failure > >> > >> In this case, either: > >> -It's changed in both Squeak and Pharo, and you get MNU's everywhere > >> for the old ifEmpty. None of the 3 cases above is satisfied, so I > >> don't really see the benefit of deprecation here. You still have to > >> change your app, replacing all calls to the now deprecated to the > >> new one, plus make sure the code at each spot handles the nil case. > >> - It's deprecated just one place, leading to extra porting overhead > >> if you have to change both the method name and the answer handling. > >> (plus the other dialect would have to add a deprecated ask: method > >> telling you how to conform to semantics). > >> > >> > >>> > >>>> Please let me comment on your reasons: > >>>> > >>>> On Tue, Sep 29, 2009 at 6:31 PM,<[email protected]> > >>>> wrote: > >>>> > >>>>> 1) It is subtle breakage that creates a major and completely > >>>>> unnecessary > >>>>> PITA for anyone who needs to write UIManager code that works across > >>>>> Pharo, Squeak, > >>>>> Croquet etc. > >>>>> > >>>> I do not agree. Squeak doesn't have to change its implementation of > >>>> #request:. Clients can just test for the nil value without wondering > >>>> if it is going to be returned (in Pharo) or not (in Squeak). > >>>> > >>>> This has an immediate consequence : you can apply the two changesets > >>>> which replace #isEmpty by #isEmptyOrNil after each call to > >>>> #request:. > >>>> You don't have to change #request: for this. > >>>> > >> That's false. > >> I suspect you only considered the case of porting Squeak code to > >> Pharo (which doesn't fail silently anyways, which is the major PITA, > >> you get an UndefinedObject>> MNU). > >> Porting code written in Pharo, f.ex. using ifNotNil: to handle an > >> action where empty input is allowed, WILL fail silently in Squeak if > >> request:* is not modified, resulting in a cancel action replacing > >> the previous value with an empty string. > >> > >> > >>>> > >>>> > >>>>> 2) It weakens the framework. Toolbuilder is based on having the > >>>>> same > >>>>> semantics across > >>>>> platforms and UIs and this change means that the entire family of > >>>>> input > >>>>> requests is > >>>>> no longer reliable as it will behave differently between Pharo and > >>>>> anything > >>>>> else in > >>>>> existence. > >>>>> > >>>> > >>>> To me, UIManager is broken. > >>>> > >> I agree, the issue is whether the default behaviour of handling > >> empty input and cancel the same way is what's broken, or the fact > >> that no way of handling such a case at all is provided. > >> > >> Considering the amount of times I've had to add ifEmpty: clauses > >> doing the same as ifNil: to request: call return values in VW (which > >> returns nil), I'd say the latter. > >> > >> > >>>> I agree that #onCancel: can be interesting on its own. However, I > >>>> see > >>>> it more as a new interesting feature whereas I'm talking about bug > >>>> fixing. > >>>> > >> Andreas has a good point, changing the current behaviour will either: > >> a) Lead to a lot of changes (albeit simple ones) for everyone if > >> adopted everywhere. > >> b) Lead to subtle errors when porting between dialects if adopted > >> just in one place. > >> > >> The fact it's been the way it is without getting changed so far, and > >> my anecdotal experiences in VW implies to me that the case where you > >> want to distinguish between an empty response and a cancel is the > >> exception rather than the rule. > >> Therefore, I support the option of adding explicit onCancel: aBlock > >> versions, and use those in the cases it's needed, rather than > >> changing the underlying behaviour, leading to either situation a) or > >> b) above. > >> > >> Cheers, > >> Henry > >> > > _______________________________________________ > > Pharo-project mailing list > > [email protected] > > http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project > > > > > > > > > _______________________________________________ > Pharo-project mailing list > [email protected] > http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project -- Miguel Cobá http://miguel.leugim.com.mx _______________________________________________ Pharo-project mailing list [email protected] http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
