Make constants publicly accessible
Opened this issue · 13 comments
On request, I am making this issue for the discussion about button constants.
Thanks ! cc @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?
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.
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
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.
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.
@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.
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? :)
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.
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.
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.
Okay, I'll make a PR to see how this scheme works out in practice.
Perfect ! Thanks for that. I'm curious to see the result.