Re: Random image patch

[Felix Riemann <friemann gnome org>]

Am Mittwoch, den 16.12.2009, 17:55 +0100 schrieb knight666:
> Hello everyone,

Hi!

> Eye of GNOME is a great program. But it had one little annoyance for
> me: no random image button! I've decided to write my very first patch
> ever. :)
> 
> I'm not sure how to post it, so I'm just going to paste it here.
> 

Posting on this list is okay. Alternatively you can open a ticket on
https://bugzilla.gnome.org .

Normally we'd like to get patches as produced by diff or git. These are
easy to apply on our side as we don't have to hand apply your changes.

When you work on extracted tarballs you can compare your changed copy to
a clean copy by running: "diff -Nupdr --exclude="*.o"  eog-clean-dir
eog-changed-dir > mypath.diff"

The best way is using git. Although that has a learning curve:
http://live.gnome.org/Git/Developers

> I changed the following things:
> 
> typedef enum EogThumbViewSelectionChange (eog-thumb-view.h)
> eog_thumb_view_select_single (eog-thumb-view.c)
> eog_window_cmd_go_random (eog-window.c) (new function)
> static const GtkActionEntry action_entries_collection (eog-window.c)
> set_action_properties (eog-window.c)
> 
> Here are my changes:
> 
> ...
> 
> eog-window.c:
> 
> static void
> set_action_properties (GtkActionGroup *window_group,
>                GtkActionGroup *image_group,
>                GtkActionGroup *collection_group)
> {
>         ...
> 
>         action = gtk_action_group_get_action (collection_group,
> "GoRandom");
>         g_object_set (action, "short_label", _("Random image"), NULL);

Watch out for GNOME's capitalization rules here. It should be "Random
Image" or just "Random" (this is a label for the toolbar), just like
above. Not sure if the label needs a translation context. But I guess
translators will tell us if necessary.

On second thought as it is used for the toolbar it might be unneeded
anyway (see below).

>         ...
> }
> 
> I hope you don't mind me posting in this manner and that my patch can
> be integrated into Eye of GNOME. :)

Well, normally we'd like to get patches as produced by diff or git. :)
These are easy to apply on our side as we don't have to hand apply your
changes.

When you work on extracted tarballs you can compare your changed copy to
a clean copy by running: "diff -Nupdr --exclude="*.o"  eog-clean
eog-changed > mypath.diff"

The best way is using git. Although that has a learning curve:
http://live.gnome.org/Git/Developers

Regarding your changes. Found nothing really bad from looking at it.
Well, as you already heard, we prefer to keep the main application as
clean as possible and append functionality through plugins. There's also
already a plugin that randomizes the image order in the slideshow mode.
Though, it has a limitation therein that it is the same order everytime
if you choose the same starting image (which your code I think hasn't).
So, I am kinda tempted to put at least the changes for EogListStore in
eog and just make the UI elements a plugin (assuming it works). The
existing slideshow plugin could then also take advantage of this change.
If possible the plugins could be merged into one then. The disadvantage
(at the moment) would be that you cannot have a toolbar button then.

Regards

Felix