Re: Session management patch, please comment.

[Richard Stallman]

It is an amazingly large amount of code, but I suppose there is no
easier way.  What a pity.

Did you copy any substantial part of it from the X distribution, or
anywhere else?  Or did you write all of it afresh?

    + DEFUN ("x-sm-interact-done", Fx_sm_interact_done, Sx_sm_interact_done, 1, 
1, 0,
    +        doc: /* End interaction as a response to save_yourself.
    + If the argument CANCEL is non-nil, Emacs will tell the session manager
    + to cancel the shutdown */)

This isn't clear enough--it doesn't tell me when to use this function
or how.  Perhaps it would be clear to a person who knows this relates
to the session manager *and* knows all about the session manager, but
that is not good enough; this doc string ought to make sense to an
ordinary Emacs Lisp programmer who knows nothing about the session
manager.

    +     doc: /* The previous session id Emacs got from session manager.
    + This is nil if Emacs was not started by the session manager.
    + The value of this variable and `x-sm-id' may be the same, depending on how
    + the session manager works.
    + See also `x-sm-id'.*/);

This doc string should explain what the previous session id is good
for.  What does it mean?  Documentation for data should explain its
overall meaning, not just what it looks like and what values are found
when.

    +   DEFVAR_LISP ("x-sm-cancel-shutdown", &Vx_sm_cancel_shutdown,
    +     doc: /* If non-nil, Emacs will cancel the session manager shutdown.
    + Setting this variable enables hooks in `save-yourself-hook' to cancel
    + a shutdown.  This should only be done at an explicit user request to do
    + so.*/);

When should the Lisp programmer set or use this?  When is it tested?
I don't see anything in the code that ever tests this variable, so I
am at a loss to understand its purpose.

    + (defvar save-yourself-hook nil
    +   "Hooks run when a save-yourself event occurs.
    + The hook gets one argument, the current session id.  This is a string and
    + can for example, be used to create a unique file name.")

Since this is not a normal hook, it should have a different name.
Perhaps save-yourself-functions.  But I think save-session-functions
is a clearer name; it gives the user more information and is more
natural.

    + (defvar restart-yourself-hook nil
    +   "Hooks run when Emacs is started by a session manager..
    + The hook gets two arguments, PREV-SESSID and SESSID.

Likewise this.  Perhaps restart-session-functions.

In general, replacing "yourself" with "session" in all the function and 
variable names would be an improvement.