On Sep 30, 2009, at 1:27 47PM, Stéphane Ducasse wrote:
On Sep 30, 2009, at 12:55 PM, Damien Cassou wrote:
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
