Hey Serkan. I was hoping to catch you last night.
On Mon, 2009-06-29 at 22:07 +0300, Serkan Kaba wrote:
> Can you examine this branch quickly,
I worked on it yesterday. It's great, except for a few concerns I have
about the URI handler part.
If you could grab [pull a copy / merge into yours / whatever]
'hackers/andrew/linkbutton',
http://research.operationaldynamics.com/bzr/java-gnome/hackers/andrew/linkbutton/
you'll see that I added another small unit test in addition to my usual
editorial.
Along the way, I hit some puzzling behaviour.
1)
According to devhelp, in C there is only be one URI hook. Of course,
since we happen to use a signal to get us across the JNI boundary, we
can have several.
I'm not sure if we should try and emulate the underlying API, or if it
matters that we can have numerous hook call backs. I don't think it
matters, but right now we have a (self-inflicted) test failure as a
result.
2)
However, if we have several, there is then no real difference between
LinkButton.UriHook and Button.Clicked; if you want to do something
special, just connect to Button.Clicked right?
... except that setting the URI hook (at least once) has the beneficial
side effect of removing that LinkButton's default 'uri-hook' [which is
to call gtk_show_uri()]. Which really seems the whole point.
So maybe all we really need is a boolean property which is
'default-handler' or 'show' or 'use-default' [ie setUseDefault()],
default true, or deactiveateDefaultUriHandler() or
dontShowInBrowswerAndInsteadDoItYourself() :) with a tiny bit of JNI
code to pass NULL to gtk_link_button_set_uri_hook()
Then one can easily just use Button.Clicked for whatever behaviour you
plan to have, and we could more or less scrap exposing the
LinkButton.UriHook callback.
3)
I added a custom URI handler in the Snapshot. After I did a call to
setUriHook() the LinkButton that I had _not_ changed stopped working.
That's gotta be bad :)
finally
4)
As a final stylistic question, I'm wondering if we shouldn't make
setUriHook() be a connect()
[and while we're at it, maybe we should have done that instead of
TreeModelFilter's setVisibleCallback() originally? Sometimes I wonder
about that]
For what it's worth, we should try and be [self-]consistent; since it is
setVisibleCallback() there, maybe it should be setUriHookCallback()
here.
or it should just be connect(). :(
Hm.
++
Anyway, (3) is most likely a bug, so that'll need addressing. But also
have a think about the broader design question.
You've done some great work, and like Guillaume this week dug into the
same horrid override case. But if we don't "need" it, then, well
{shrug}.
I look forward to your thoughts.
AfC
Sydney
|
