On Thursday 03 August 2006 14:29, Patrick Bernaud wrote: > Hi Peter, > > Peter TB Brett writes: > > [...] > > Yet another version of the patch: SF.net is *still* refusing to let me > > upload it! This incorporates Patrick's proposed changes, but should > > behave identically to the previous version despite the large delta. > > A few comments on this new version: > > - the properties 'command', 'filename' are strings. You can use > g_param_spec_string() instead of g_param_spec_pointer() in the > definition of the properties (print_dialog_class_init()). Fixed. > - you are defining two enums with values useful to the dialog. But > they are only in the source while you are providing a header file > for this, not the header widget. See next point. > > - these two enums could be assigned a clean type. See > g_enum_register_static(). The types of their respective properties > could be changed to an enum type instead of a quite obscure > integer type. > > I do agree that this is mostly cosmetic since the dialog is not > used from outside x_print.c. But then why a dedicated header file? > > These enums would be useful directly in TOPLEVEL and libgeda as > well. They could be moved there later. At the moment, PrintDialog is _only_ using the remaining enums (type_indices, orient_indices, and the PROP_* constants) internally. For external purposes, libgeda defines the constants used. But libgeda's constants aren't guaranteed to take values suitable for the use they're being put to in the PrintDialog class. I would fix libgeda to declare its constants 'properly' but that's rather out of the scope of this patch. > - the problem on editability of both entries for command and > filenames is due to your keypress callback. The GtkEntry is > editable by default so that there is indeed nothing to do. But it > looks like the callback shorts the keystrokes to the dialog > instead of transfering to the entry. > > No solution at the moment. > > BTW you may want to have a look at gtk_editable_set_position() to > display the end of the filename rather than its beginning. This is to do with the return value of the keypress handler. It needs to return FALSE to propagate the signal to the next eligible widget. Fixed. > - you can use gtk_dialog_add_buttons() to actually add the > buttons. Plus you can wait for the GtkResponseType right from > x_print_setup() instead of defining your own response values and > providing callbacks for the buttons (action_print() and > action_cancel()). Fixed & callbacks removed. I attach the latest version of the patch. Peter -- Fisher Society publicity officer http://tinyurl.com/o39w2 CUSBC novices, match and league secretary http://tinyurl.com/mwrc9 Quake II build tools maintainer http://tinyurl.com/fkldd v2sw6YShw7$ln5pr6ck3ma8u6/8Lw3+2m0l7Ci6e4+8t4Eb8Aen5+6g6Pa2Xs5MSr5p4 hackerkey.com
Attachment:
print-dialog-4.tar.bz2
Description: application/tbz
Attachment:
pgp0eL1Z1eIUe.pgp
Description: PGP signature
_______________________________________________ geda-dev mailing list geda-dev@xxxxxxxxxxxxxx http://www.seul.org/cgi-bin/mailman/listinfo/geda-dev