Comment 1 Stephan Arts 2012-04-18 22:11:38 CEST

Simon, can you review this patch?

Comment 2 Simon Steinbeiss 2012-04-18 22:46:58 CEST

ad 1) I agree with the changes proposed in Masato's patch. E.g. Thunar follows the distinction of adding "..." to menuitems that open another window, not just execute some action directly.

ad 2) With respect to the mnemonic problems, I suggest the following:
* View menu
  - "_Show Toolbar"
  - "Show Status _Bar"
  - "Thumb_nail Size"
  - "Set as _Wallpaper"

* Go menu
  - "_Forward"
  - "F_irst"
(Because "i" is also the latin correspondent of the arabic "1".)

Comment 3 Stephan Arts 2012-04-19 18:40:11 CEST

I committed some of your suggestions in the code-cleanup I performed.

Can you please check it out?

Comment 4 Stephan Arts 2012-04-21 13:23:17 CEST

Thank you, fixed the accelerators in master now.

Hi,
One string is forgotten ;)

diff --git a/src/main_window.c b/src/main_window.c
index e415376..cc54f7b 100644
--- a/src/main_window.c
+++ b/src/main_window.c
@@ -1319,7 +1319,7 @@ rstto_main_window_image_list_iter_changed (RsttoMainWindow *window)
             gtk_menu_shell_append (GTK_MENU_SHELL (open_with_menu), menu_item);
             g_signal_connect(G_OBJECT(menu_item), "activate", G_CALLBACK(cb_rstto_main_window_open_with_other_app), window);
 
-            menu_item = gtk_menu_item_new_with_mnemonic (_("Open With Other _Application"));
+            menu_item = gtk_menu_item_new_with_mnemonic (_("Open With Other _Application..."));
             gtk_menu_shell_append (GTK_MENU_SHELL (open_with_window_menu), menu_item);
             g_signal_connect(G_OBJECT(menu_item), "activate", G_CALLBACK(cb_rstto_main_window_open_with_other_app), window);
 
Regards

Comment 6 Stephan Arts 2012-04-22 09:38:19 CEST

Thank you, I've committed the patch in commit c3fba28116a87ab86210aee4b61e42286d7c717c