Make constants publicly accessible

[oakes]

On request, I am making this issue for the discussion about button constants.

[GuillaumeGomez]

Thanks ! cc @gkoz

[gkoz]

It seems there's no precedent for exporting such constants in rgtk. Maybe an enum would be better. What if we renamed the trait to AsDialogButtons and introduced

enum DialogButtons {
    Ok,
    OkCancel,
    YesNo,
    // etc
}

impl AsDialogButtons for DialogButtons {
    // ...
}

then exported this enum?

[GuillaumeGomez]

I like what you did with your constants. @jeremyletang and I tried as much as possible to keep enums when possible but there are places where it causes problems (in big enums but I don't which ones). So don't worry about that.

[gkoz]

The enum would be used pretty much the same way as the consts but avoid the ugly uppercased names :)

SomeDialog::new("title", DialogButtons::YesNo)

As an unexpected benefit it would let us replace the GtkButtonsType buttons parameter of MessageDialog::new and improve consistency between different dialogs while pretending it works just like the underlying API. https://developer.gnome.org/gtk3/stable/GtkMessageDialog.html#gtk-message-dialog-new

An interesting detail: the Gnome HIG recommends against using such generic constants (I guess that's why the dialogs take variable button arrays now) and calls for placing the Cancel button first. https://developer.gnome.org/hig/stable/dialogs.html.en#primary-buttons

[GuillaumeGomez]

Then it's perfect ! Let's do it (or rather please do it !) this way !

I didn't know that. We could add a little doc above the function with this link for users to give gnome recommendations.

[gkoz]

Ugh, I'm starting to feel that the variadic hack is just not worth it. It won't matter if add_button is called a couple times. So I'm gonna scrap the abomination. The Dialog APIs can get much better then.

[GuillaumeGomez]

@jeremyletang and I were thinking about doing it with a macro (but I think I already told you that, no ?). But it wasn't a priority so we didn't do it.

[gkoz]

I couldn't figure out a way to use a macro without exposing it to the users which is totally not worth it. Maximizing efficiency of adding dialog buttons shouldn't be a priority anyway right? :)

[gkoz]

Actually all gtk_dialog_add_buttons does is call gtk_dialog_add_buttons_va_list which calls gtk_dialog_add_button in a loop. We'll actually improve efficiency by doing the loop ourselves. Using those variadic functions officially Doesn't Make Sense. So yeah.

[gkoz]

So after pondering Dialog and its associated types more, @GuillaumeGomez would you be open to trying out a different convention whereas the dialog module would be moved out of traits to live directly under gtk and contain all the dialog related enums (currently named ResponseType, MessageType, ButtonsType) as well as DialogTrait and Dialog, AboutDialog and MessageDialog (other dialogs are a bit more ambiguous because they come in pairs of e.g. ColorChooserDialog/ColorChooserWidget). The Response enum would incorporate the GtkResponseType variants and User(u16) variant, the Buttons enum more or less as described above.

Motivation: Rust seems to favour utilizing hierarchical modules organization against dumping all types in a flat namespace. This more of a self-documenting approach would benefit new GTK users although might be confusing to those, who've used other GTK bindings. The benefit of physical separation of the widgets/traits modules on the other hand is still lost on me, it hurts maintainability and requires having more modules and files than necessary.

[GuillaumeGomez]

I'm not very open to this but I can't find any good argument against it so I think we should give it a try. If this seems fine, then we'll have to extend it to other similar cases.

[gkoz]

Okay, I'll make a PR to see how this scheme works out in practice.

[GuillaumeGomez]

Perfect ! Thanks for that. I'm curious to see the result.