Re: [java-gnome-hackers] LinkButton coverage
Brought to you by:
afcowie
From: Andrew C. <an...@op...> - 2009-06-30 01:14:41
|
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 |