Re: [PATCH] exit function

[Christopher Roy Bratusek <zanghar freenet de>]

Am Tue, 01 Dec 2009 14:59:44 +0900 (JST)
schrieb Teika Kazura <teika lavabit com>:

> Hi. First on the code. The patch should look like:
> ------------------------------------------------------------------------
> +  (define (exit action)
> +    (when (eq action 'quit)
> +       (quit))
> +    (call-hook 'before-exit-hook)
> +    (map-windows delete-window)
> +    (case ...
> ------------------------------------------------------------------------
> Let me explain. 
> 
> 1. 
> before-exit-hook shouldn't be called for quit, or it will be called
> twice. (The call is done by rep, and not coded in Sawfish.)

Of course not.

> 
> 2.
> (window-history-save) shouldn't be called unless the user do
> (require sawfish.wm.ext.window-history). And if they do require,
> it is already hooked into before-exit-hook, so there's no need for its
> call from (exit action).

Well, I did that, as the very first version of (exit) didn't call the
before-exit-hook, but now it may safely removed.

> 3.
> It means (exit action) does not add anything for quit, so it can be
> removed? (See also my vote at the end of this message.)

Yes.

> 4.
> But I'm not sure if it's correct altogether. Look at main(argc, argv):
> ------------------------------------------------------------------------
>  ...
>  rc = rep_top_level_exit ();
> 
>   /* call all exit funcs... */
>   server_kill ();
>   functions_kill ();
>   windows_kill ();
>   frames_kill ();
>   cursors_kill ();
>   fonts_kill ();
>   colors_kill ();
>   images_kill ();
>   events_kill ();
>   session_kill ();
> ------------------------------------------------------------------------
> (exit action) skips all of them, but is it ok? My guess is yes;
> they're needed if X server remains to run, but I haven't read the
> code, so can't say anything for sure.

In theory it would be enough to call the functions they do (eg:
sm-disconnect for session_kill, fonts_kill for example is empty), or to
implement it in C, but then it won't be ready before feature freeze.

> 
> On Sat, 28 Nov 2009 14:52:26 +0100, Christopher Roy Bratusek wrote:
> >> Sorry, I wanted to say (defgroup exit "Exit Sawfish" :group misc).
> > 
> > Nope. It uses "External Applications" from
> > sawfish.wm.commands.launcher
> 
> Commands for term and browser are set in "external applications",
> since there's no other way to name them. But reboot/shutdown/suspend
> can be called 'exit' (or exit-session) , so it's natural.

On the other hand this would just be polluting SawfishConfig with just
another group. External Applications is OK, if you ask me, as those are
also external applications, of course.

> 
> >> >> * Func & command 'exit' is so confusing with 'quit'. But
> >> >> what'll be good? 
> >> [...]
> > [] exit
> > [] safe-quit
> > [] exit-session
> 
> If quit is to be dropped, then exit-session.

OK.

> Regards,
> Teika (Teika kazura)
> 

Chris