Re: [PATCH] Re: gEDA-dev: libgeda, gschem: New print dialog

[Patrick Bernaud <b-patrick-at-NOSPAMwanadoo_dot_fr>]

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()).

  - 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.

  - 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.

  - 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()).

Regards,


Patrick


_______________________________________________
geda-dev mailing list
geda-dev@moria.seul.org
http://www.seul.org/cgi-bin/mailman/listinfo/geda-dev