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 :)
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
