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,<codesite-noreply@google.com> 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 Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
-- Miguel Cobá http://miguel.leugim.com.mx