Add Alt + left/right arrow to move between workspaces.METACITY_2_3_2

2001-08-03  Havoc Pennington  <hp@pobox.com>

	* src/keybindings.c: Add Alt + left/right arrow to
	move between workspaces.

	* src/screen.c (set_wm_check_hint): put property pointing back to
	itself on the _WIN_SUPPORTING_WM_CHECK window.

5 files changed, 142 insertions, 44 deletions

diff --git a/ChangeLog b/ChangeLog
index 709156e9..ccd06c5d 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,13 @@
 2001-08-03  Havoc Pennington  <hp@pobox.com>
 
+	* src/keybindings.c: Add Alt + left/right arrow to 
+	move between workspaces.
+
+	* src/screen.c (set_wm_check_hint): put property pointing back to
+	itself on the _WIN_SUPPORTING_WM_CHECK window.
+
+2001-08-03  Havoc Pennington  <hp@pobox.com>
+	
 	* src/display.c (event_callback): push error trap around configure 
 	of withdrawn window, fixes a crash caused by rapidly
 	creating/destroying a window.
diff --git a/README b/README
index 004e48c5..cdac58b3 100644
--- a/README
+++ b/README
@@ -42,6 +42,8 @@ METACITY FEATURES
     Alt-Tab           forward cycle window focus
     Alt-Shift-Tab     backward cycle focus
     Alt-Escape        focus previous window
+    Alt-Left Arrow    previous workspace
+    Alt-Right Arrow   next workspace
 
  - Window keybindings:
     Alt-space         window menu
@@ -195,15 +197,6 @@ METACITY BUGS, NON-FEATURES, AND CAVEATS
    window cycling that I would like to copy. (The little 
    popup window thing.)
 
- - People seem to like shortcuts using arrow keys for moving
-   between workspaces. My only question here is that I'm not sure 
-   what the spatial relationship between workspaces is. 
-   (Are they in a grid, or a horizontal row, or what.)
-   I think we may need a window manager spec extension to 
-   share this information between the WM and the pager.
-   We could add left/right arrow and pretend they're in
-   a row for the short term though.
-
  - Should Metacity support flipping in right-to-left locales?
    I don't know what window managers look like in a right-to-left
    locale. I assume the window titles should be right-justified;
diff --git a/src/display.c b/src/display.c
index bf3cef5e..ccc82e85 100644
--- a/src/display.c
+++ b/src/display.c
@@ -1287,6 +1287,10 @@ meta_display_get_workspace_by_index (MetaDisplay *display,
                                      int          index)
 {
   GList *tmp;
+
+  /* should be robust, index is maybe from an app */
+  if (index < 0)
+    return NULL;
   
   tmp = g_list_nth (display->workspaces, index);
 
@@ -1304,6 +1308,10 @@ meta_display_get_workspace_by_screen_index (MetaDisplay *display,
   GList *tmp;
   int i;
 
+  /* should be robust, index is maybe from an app */
+  if (index < 0)
+    return NULL;
+  
   i = 0;
   tmp = display->workspaces;
   while (tmp != NULL)
diff --git a/src/keybindings.c b/src/keybindings.c
index a717a8b3..8c0f9d57 100644
--- a/src/keybindings.c
+++ b/src/keybindings.c
@@ -57,6 +57,14 @@ static void handle_focus_previous     (MetaDisplay *display,
                                        MetaWindow  *window,
                                        XEvent      *event,
                                        gpointer     data);
+static void handle_workspace_left     (MetaDisplay *display,
+                                       MetaWindow  *window,
+                                       XEvent      *event,
+                                       gpointer     data);
+static void handle_workspace_right     (MetaDisplay *display,
+                                       MetaWindow  *window,
+                                       XEvent      *event,
+                                       gpointer     data);
 
 typedef struct _MetaKeyBinding MetaKeyBinding;
 
@@ -89,6 +97,8 @@ static MetaKeyBinding screen_bindings[] = {
   { XK_ISO_Left_Tab, ShiftMask | Mod1Mask, KeyPress, handle_tab_backward, NULL, 0 },
   { XK_Tab, ShiftMask | Mod1Mask, KeyPress, handle_tab_backward, NULL, 0 },
   { XK_Escape, Mod1Mask, KeyPress, handle_focus_previous, NULL, 0 },
+  { XK_Left, Mod1Mask, KeyPress, handle_workspace_left, NULL, 0 },
+  { XK_Right, Mod1Mask, KeyPress, handle_workspace_right, NULL, 0 },
   { None, 0, 0, NULL, NULL, 0 }
 };
 
@@ -500,6 +510,47 @@ meta_display_process_key_event (MetaDisplay *display,
 }
 
 static void
+switch_to_workspace (MetaDisplay *display,
+                     MetaWorkspace *workspace)
+{
+  MetaWindow *move_window;
+
+  move_window = NULL;
+  if (display->grab_op == META_GRAB_OP_MOVING)
+    move_window = display->grab_window;
+      
+  if (move_window != NULL)
+    {
+      if (move_window->on_all_workspaces)
+        move_window = NULL; /* don't move it after all */
+
+      /* We put the window on the new workspace, flip spaces,
+       * then remove from old workspace, so the window
+       * never gets unmapped and we maintain the button grab
+       * on it.
+       */
+      if (move_window)
+        {
+          if (!meta_workspace_contains_window (workspace,
+                                               move_window))
+            meta_workspace_add_window (workspace, move_window);
+        }
+    }
+      
+  meta_workspace_activate (workspace);
+
+  if (move_window)
+    {
+      /* Lamely rely on prepend */
+      g_assert (move_window->workspaces->data == workspace);
+                
+      while (move_window->workspaces->next) /* while list size > 1 */
+        meta_workspace_remove_window (move_window->workspaces->next->data,
+                                      move_window);
+    }
+}
+
+static void
 handle_activate_workspace (MetaDisplay *display,
                            MetaWindow  *event_window,
                            XEvent      *event,
@@ -514,41 +565,69 @@ handle_activate_workspace (MetaDisplay *display,
 
   if (workspace)
     {
-      MetaWindow *move_window;
+      switch_to_workspace (display, workspace);
+    }
+  else
+    {
+      /* We could offer to create it I suppose */
+    }
+}
 
-      move_window = NULL;
-      if (display->grab_op == META_GRAB_OP_MOVING)
-        move_window = display->grab_window;
-      
-      if (move_window != NULL)
-        {
-          if (move_window->on_all_workspaces)
-            move_window = NULL; /* don't move it after all */
-
-          /* We put the window on the new workspace, flip spaces,
-           * then remove from old workspace, so the window
-           * never gets unmapped and we maintain the button grab
-           * on it.
-           */
-          if (move_window)
-            {
-              if (!meta_workspace_contains_window (workspace,
-                                                   move_window))
-                meta_workspace_add_window (workspace, move_window);
-            }
-        }
-      
-      meta_workspace_activate (workspace);
+static void
+handle_workspace_left (MetaDisplay *display,
+                       MetaWindow  *window,
+                       XEvent      *event,
+                       gpointer     data)
+{
+  MetaWorkspace *workspace;
+  MetaScreen *screen;
+  int i;
+  
+  screen = meta_display_screen_for_root (display,
+                                         event->xkey.root);
 
-      if (move_window)
-        {
-          /* Lamely rely on prepend */
-          g_assert (move_window->workspaces->data == workspace);
-                
-          while (move_window->workspaces->next) /* while list size > 1 */
-            meta_workspace_remove_window (move_window->workspaces->next->data,
-                                          move_window);
-        }
+  if (screen == NULL)
+    return;  
+  
+  i = meta_workspace_index (screen->active_workspace);
+  --i;
+
+  workspace = meta_display_get_workspace_by_index (display, i);
+  
+  if (workspace)
+    {
+      switch_to_workspace (display, workspace);
+    }
+  else
+    {
+      /* We could offer to create it I suppose */
+    }
+}
+
+static void
+handle_workspace_right (MetaDisplay *display,
+                        MetaWindow  *window,
+                        XEvent      *event,
+                        gpointer     data)
+{
+  MetaWorkspace *workspace;
+  MetaScreen *screen;
+  int i;
+  
+  screen = meta_display_screen_for_root (display,
+                                         event->xkey.root);
+
+  if (screen == NULL)
+    return;  
+  
+  i = meta_workspace_index (screen->active_workspace);
+  ++i;
+
+  workspace = meta_display_get_workspace_by_index (display, i);
+  
+  if (workspace)
+    {
+      switch_to_workspace (display, workspace);
     }
   else
     {
diff --git a/src/screen.c b/src/screen.c
index 7f704ed4..e40304b1 100644
--- a/src/screen.c
+++ b/src/screen.c
@@ -50,12 +50,22 @@ set_wm_check_hint (MetaScreen *screen)
                    XA_WINDOW,
                    32, PropModeReplace, (guchar*) data, 1);
 
-  /* Legacy GNOME hint */
-  XChangeProperty (screen->display->xdisplay, screen->xroot,
+  /* Legacy GNOME hint (uses cardinal, dunno why) */
+
+  /* legacy hint window should have property containing self */
+  XChangeProperty (screen->display->xdisplay, screen->display->leader_window,
                    screen->display->atom_win_supporting_wm_check,
                    XA_CARDINAL,
                    32, PropModeReplace, (guchar*) data, 1);
 
+  /* do this after setting up window fully, to avoid races
+   * with clients listening to property notify on root.
+   */
+  XChangeProperty (screen->display->xdisplay, screen->xroot,
+                   screen->display->atom_win_supporting_wm_check,
+                   XA_CARDINAL,
+                   32, PropModeReplace, (guchar*) data, 1);
+  
   return Success;
 }