Wikia

Sawfish

Clean-up for Zimmerman's iconify patch

773pages on
this wiki
Talk0
Browse all patches

Author Edit

Teika kazura

Synopsis Edit

This patch improves Ian Zimmerman's iconify patch (IZIP hereafter). It was commited as rev4373.

This patch restores the code in 'events.c' which was removed by IZIP, and put it in windows.c -> add_window. It replaces the addition by IZIP.

Backgrounds Edit

1. Setting 'iconify->no' in Sawfish configurator does nothing, contrary to the common sense. So sawfish can iconify non-icon initial-state windows as a user setting, but it can't un-iconify iconified-init-states as a user setting. That's because sawfish's property had not provided distinction between 'unset' and 'set nil' until summar of 2008. Desirably it be fixed, but it is a bit tedious task, taking consistency check into account. I just postpone this issue, being satisfied to have recorded it at wiki.

2. There had been a subtlety, but in my opinion not a bug, which was pointed out by Ian Zimmerman. Ian's patch solves it.

It's that even after the prob 1 is solved by hacking lisp codes, event.c -> map_request hinders the effort. That's because it calls 'iconify' after window matching, which is done in windows.c -> add_window. It was solved by IZIP.

3. Ian's patch works, but in ugly way. It uses XIconifyWindow which is for clients' use. As a result, a window which should be iconified appears once un-iconified, and it soon gets iconified. You can see something flash on your screen for a moment. It also takes away the focus. Such implementation can be a seed of bug.

Also the wrong comment is added to the source: "There's a minor downside: rep doesn't know about this, so any hooks that might be operative for iconification don't get run..." No. Eventually sawfish iconifies the window, so hooks is called.

4. This is solved by this patch. It now iconifies the window in windows.c -> add_window, before 'before_add_window_hook' where matcher is called. Because add_window is called only from map_request, this is permissible.

questions Edit

Safety Edit

In this patch, call to 'iconify-window' is done quite early, just before before-add-window-hook. There, window set up is not complete.

The doc on before-add-window-hook says:

This occurs before the window is created, installed or placed. At this early stage, the only safe action is to set properties of the window (with `window-set').

As far as the author tried, it seems to work, but is it really safe?

Exact hook spec Edit

For this issue, read comments [1] and [2].

In this work, I found that in 'lisp/sawfish/wm/state/iconify.jl', 'iconify-window-hook' and 'window-state-change-hook' are called, even when a window appears as iconified from the first time. I think it is permissible for 'iconify-window-hook' if it is documented (not so written now), but not for 'window-state-change-hook'. But I'm not sure, so please tell me the way it should be.

To be complete, I'll add the doc patch.

Patch testing Edit

  1. Copy/paste the patch listed below into some file, eg. TEST.diff.
  2. If you don't have sawfish sources yet, have one, as described get it from GIT repo.
  3. Go into the directory where sawfish sources reside, eg. cd sawfish
  4. Test if the patch applies cleanly with this command:
    patch -p1 --ignore-whitespace --dry-run < TEST.diff
    in case of problems try also: -p0 or -p2
  5. If it applies cleanly, then remove the --dry-run from above command and run it again, otherwise ask on the mailing list.
  6. Compile sawfish: ./autogen.sh && make
  7. Install it for testing, but it depends on your linux distribution.
    1. It is always better to install sawfish as your distribution package, but it is different for each distribution.
    2. So you may try make install, which will install sawifish in /usr/local/share/sawfish/ (if you have write access). But then make sure that you run the correct version and delete it from that directory afterwards, to avoid any conflicts.
  8. Se also

PS: edit this template if you feel that those instructions can be improved.

Patch Edit

diff -ur sawfish-svn.orig/src/events.c sawfish-svn/src/events.c
--- sawfish-svn.orig/src/events.c	2009-01-20 15:17:59.783454000 +0900
+++ sawfish-svn/src/events.c	2009-02-06 16:14:07.648554675 +0900
@@ -747,22 +747,6 @@
     Lisp_Window *w = find_window_by_id (id);
     if (w == 0)
     {
-        /* The reason for doing this before the add is to give matchers
-         * a chance to do their thing and reverse the iconification,
-         * if the user desires.  There's a minor downside: rep doesn't
-         * know about this, so any hooks that might be operative for
-         * iconification don't get run, and there's no iconify sound.
-         * But it can be argued that this is the Right Thing anyway,
-         * since the hint was set outside of rep's watch too. */
-
-        XWMHints* wmhints = XGetWMHints(dpy, id);
-
-	if (wmhints && (wmhints->flags & StateHint) && wmhints->initial_state == IconicState)
-            XIconifyWindow(dpy, id, screen_num);
-        
-	if (wmhints)
-            XFree(wmhints);
-        
 	/* Also adds the frame. */
 	w = add_window (id);
 	if (w == 0)
diff -ur sawfish-svn.orig/src/windows.c sawfish-svn/src/windows.c
--- sawfish-svn.orig/src/windows.c	2009-01-08 15:10:02.432303000 +0900
+++ sawfish-svn/src/windows.c	2009-02-06 16:21:13.704546444 +0900
@@ -424,7 +424,8 @@
 }
 
 
-/* Add the top-level window ID to the manager's data structures */
+/* Add the top-level window ID to the manager's data structures.
+ * This is called only from events.c -> map_request */
 Lisp_Window *
 add_window (Window id)
 {
@@ -510,6 +511,21 @@
 	if (initialising)
 	    Fwindow_put (rep_VAL (w), Qplaced, Qt);
 
+	/* If the window requires to start as icon, then iconify it.
+	 * It is better to be done before 'before_add_window_hook', where
+	 * matching takes place, because in future, matcher can have an
+	 * option to un-iconify, overrinding the application's request
+	 * to iconify. 
+	 */
+	if (w->wmhints && w->wmhints->flags & StateHint
+	    && w->wmhints->initial_state == IconicState)
+	  {
+	    DEFSTRING (iconify_mod, "sawfish.wm.state.iconify");
+	    rep_call_lisp1 (module_symbol_value
+			    (rep_VAL (&iconify_mod), Qiconify_window),
+			    rep_VAL(w));
+	  }
+	
 	/* ..then call the add-window-hook's.. */
 	rep_PUSHGC(gc_win, win);
 	Fcall_window_hook (Qbefore_add_window_hook, rep_VAL(w), Qnil, Qnil);

Community's reasons for inclusion or rejection Edit

When voting anonymously please write your name, so that it can be associated with your posts on the mailing list. If you are logged in you can sign yourself by typing four tilda characters: ~~~~.

  • Please vote with: Yes vote: yes., No vote: no., Try vote: let's try in experimental., Wtf vote: pondering. or Suspend wait for next release.
  • Wtf vote: pondering. Wait until the #questions get solved.
    • Author has not yet compiled svn sawfish, so please someone apply this patch, and confirm $xclock -iconic works, or wait for a while until the author himself does it. - Teika kazura 09:28, 7 February 2009 (UTC)
  • It is commited as rev 4468. - Teika kazura 07:27, 18 April 2009 (UTC)

Around Wikia's network

Random Wiki