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
