Hi Costantino,
I've looked at your proposed changes at
http://cr.openjdk.java.net/~anthony/7-43-GTKFileDialog-6913179.1
(BTW i wonder what 7-43 is?)
Some comments follow:
- We're going to have two functions that check version of GTK. Does it
make sense to refactor UNIXToolkit to use the new one? And get rid of
the old one completely?
- Why would you bother with returning a String from
native_gtk_check_version()? It is effectively ignored later, so you
could as well return a boolean value from the native function.
- Most gtk2_interface functions are named gtk2_XXX. However silly this
is, i believe gtk_file_chooser_load() is better renamed, just for
consistency =)
- Note that g_thread_get_initialized() is available since glib-2.20
only. Is there a way to enable the new filechooser on older systems?
- Load/unload calls in GtkFileDialogPeer.c seems unbalanced to me. Consider:
setVisible(true)
run()
init_GtkFileDialogPeer(env) {
if (jvm == NULL) {
gtk2_load();
}
}
setVisible(false)
quit()
gtk2_unload();
setVisible(true)
run()
init_GtkFileDialogPeer(env) {
if (jvm == NULL) // this is false, gtk2_load() isn't called
}
Do you really want to load GTK each time a file dialog is shown? This is
pretty expensive. Unloading will also break GTK LAF.
Thanks!
--
Peter
