time.el and display-time-mail-function

time.el and display-time-mail-function

[Jason Bastek]

Hello,

Recently I tried to write a custom display-time-mail-function, as
provided for in time.el (my mail system was being converted from mbox
to maildir++ format, so I had to check for the existence of files in a
directory, rather than for the existence of a single file).

However I think there's a problem in the logic in time.el.  In
function display-time-update, it has the following:

	 (mail (or (and display-time-mail-function
			(funcall display-time-mail-function))
		   (and (stringp mail-spool-file)

where display-time-mail-function is a user-definable function, which
seems like the perfect thing for my situation.  The problem is the
"or" -- if my display-time-mail-function returns nil (to indicate that
there is no new mail), instead of stopping there, it will continue
with the standard mail-spool-file check.

I believe this logic is wrong, and it could be fixed by changing the
"or" to a "cond"/"if", e.g.:

         (mail (if display-time-mail-function
                   (funcall display-time-mail-function)
                 (and (stringp mail-spool-file)

Does this seem like a reasonable change to make?  It seems that,
without this change, display-time-mail-function is useless.  But maybe
I'm missing some aspect.

Thanks,

-Jason

Re: time.el and display-time-mail-function

[Richard M. Stallman]

    where display-time-mail-function is a user-definable function, which
    seems like the perfect thing for my situation.  The problem is the
    "or" -- if my display-time-mail-function returns nil (to indicate that
    there is no new mail), instead of stopping there, it will continue
    with the standard mail-spool-file check.

Why do you think that is undesirable?

    Does this seem like a reasonable change to make?  It seems that,
    without this change, display-time-mail-function is useless.

It is not useless.  It gives you a way to check for mail
in some other place.

If there's no mail in the other place, but there is mail
in the standard place, the current code will say you have mail.
Why is that a problem?

Re: time.el and display-time-mail-function

[Jason Bastek]

>>> "RMS" == Richard M Stallman <rms@gnu.org> writes:

  RMS>     where display-time-mail-function is a user-definable
  RMS>     function, which seems like the perfect thing for my
  RMS>     situation.  The problem is the "or" -- if my
  RMS>     display-time-mail-function returns nil (to indicate that
  RMS>     there is no new mail), instead of stopping there, it will
  RMS>     continue with the standard mail-spool-file check.

  RMS> Why do you think that is undesirable?

My reasoning is as follows:

The purpose of this function display-time-update is (essentially) to
determine whether there is new mail.  It provides the user with a
customizable function, display-time-mail-function.  So I would think
that, if I go to the trouble of defining this function, I want the
semantics to be:

   my function returns t    => there is new mail
   my function returns nil  => there is no new mail

But because of the "or", the semantics are:

   my function returns t    => there is new mail
   my function returns nil  => try other ways of checking for new mail

And in my case, the other way of checking for new mail returns an
incorrect answer.  This is because my environment variable MAIL now
points to a directory, not a file, and the code ends up calling
file-attributes on this directory, and looking at the size in bytes to
see if it's >0.  For a directory, the size in bytes will always be >0,
therefore the code will always report that there is new mail.

I notice that there is a newer version of time.el with some code to do
the directory-based check, which is nice, and will probably fix my
problem.  But the code still uses "or", so it will run into the same
problem if MAIL is pointing to a directory rather than a file (and the
other mail-checking methods return nil).

  RMS> If there's no mail in the other place, but there is mail
  RMS> in the standard place, the current code will say you have mail.
  RMS> Why is that a problem?

I think this is explained above -- the current code assumes that
mail-spool-file points to a mail file to be checked.  But in my case
(and the case of anyone else whose system is using maildir rather than
mbox), there is no such file, there's only a directory.  So because of
the "or", failures to find new mail using user-defined methods will
always fall back on the checking of mail-spool-file for non-zeroness.
This will result in a false positive.

This can be worked around in several ways (undefining the env var
MAIL, setting display-time-mail-file to t, etc), but I see those as
hacks around the problem of the "or".  Changing it to a "cond" seems
to do the more correct thing.

If people are relying on the "or" behavior because they have several
mail drops, perhaps time.el should be redesigned to allow for this
(have a user-definable list of mail files to check).  But as it stands
it's definitely broken for me.

-Jason

Re: time.el and display-time-mail-function

[Richard M. Stallman]

    This can be worked around in several ways (undefining the env var
    MAIL, setting display-time-mail-file to t, etc), but I see those as
    hacks around the problem of the "or".  Changing it to a "cond" seems
    to do the more correct thing.

In principle, the change you are asking for will make things more
flexible, since the user-defined function could check the standard
places too.

However, it would be an incompatible change, and I am not sure who
many users take advantage of the current code and would have to change
their init files.  WIthout pollinmg the users to find out how many, I
think it is better to keep the code unchanged, and you can use one of
the various workarounds.  For instance, you could set
display-time-mail-file to t.