Hi Anton, I have a few comments inline below. With those changes, I'm fine with the patch.
On Tue, Jun 26, 2018 at 05:24:39PM +0900, Anton Lindqvist wrote:
> diff --git a/buffy.c b/buffy.c
> index eab17e6e..8f164f7f 100644
> --- a/buffy.c
> +++ b/buffy.c
> @@ -505,7 +505,9 @@ static int buffy_mbox_check (BUFFY* mailbox, struct stat
> *sb, int check_stats)
> }
>
> /* Check all Incoming for new mail and total/new/flagged messages
> - * force: if true, ignore BuffyTimeout and check for new mail anyway
> + * The force argument may be any combination of the following values:
> + * MUTT_BUFFY_CHECK_FORCE ignore BuffyTimeout and check for new mail
> + * MUTT_BUFFY_CHECK_FORCE_STATS ignore BuffyTimeout and calculate
> statistics
> */
> int mutt_buffy_check (int force)
> {
> @@ -525,7 +527,7 @@ int mutt_buffy_check (int force)
>
> #ifdef USE_IMAP
> /* update postponed count as well, on force */
> - if (force)
> + if ((force & MUTT_BUFFY_CHECK_FORCE))
> mutt_update_num_postponed ();
> #endif
>
> @@ -536,8 +538,9 @@ int mutt_buffy_check (int force)
> if (!force && (t - BuffyTime < BuffyTimeout))
> return BuffyCount;
>
> - if (option (OPTMAILCHECKSTATS) &&
> - (t - BuffyStatsTime >= BuffyCheckStatsInterval))
> + if ((force & MUTT_BUFFY_CHECK_FORCE_STATS) ||
> + ((option (OPTMAILCHECKSTATS) &&
^^
Nit: I don't think you need the double parenthesis just above here.
> + (t - BuffyStatsTime >= BuffyCheckStatsInterval))))
^^
> diff --git a/buffy.h b/buffy.h
> index c0cfddf4..81f0cc7f 100644
> --- a/buffy.h
> +++ b/buffy.h
> @@ -63,4 +63,8 @@ void mutt_buffy_setnotified (const char *path);
>
> int mh_buffy (BUFFY *, int);
>
> +/* force flags passed to mutt_buffy_check() */
> +#define MUTT_BUFFY_CHECK_FORCE 0x1
> +#define MUTT_BUFFY_CHECK_FORCE_STATS 0x2
I'd prefer if this was like the other bit-flag definitions, like in
mutt.h:
#define MUTT_BUFFY_CHECK_FORCE 1
#define MUTT_BUFFY_CHECK_FORCE_STATS (1<<1)
> diff --git a/commands.c b/commands.c
> index 3654fc0d..f5f22cd2 100644
> --- a/commands.c
> +++ b/commands.c
> @@ -1028,4 +1028,10 @@ int mutt_check_traditional_pgp (HEADER *h, int *redraw)
> return rv;
> }
>
> +void mutt_check_stats (void)
> +{
> + if (!option (OPTMAILCHECKSTATS))
> + return;
Above, you allow the MUTT_BUFFY_CHECK_FORCE_STATS flag to override the
option(OPTMAILCHECKSTATS) config variable. I think this makes sense -
some people may want to *only* manually refresh the stats. So I would
suggest removing the option check here.
>
> + mutt_buffy_check (MUTT_BUFFY_CHECK_FORCE | MUTT_BUFFY_CHECK_FORCE_STATS);
> +}
> diff --git a/doc/manual.xml.head b/doc/manual.xml.head
> index 8ac8fd9d..e78d9ec5 100644
> --- a/doc/manual.xml.head
> +++ b/doc/manual.xml.head
> @@ -953,6 +953,20 @@ In addition, the <emphasis>index</emphasis> and
>
> <variablelist>
>
> +<varlistentry>
> +<term>
> +<literal><check-stats></literal><anchor id="check-stats"/>
> +</term>
> +<listitem>
> +<para>
> +Calculate statistics for all monitored mailboxes declared using the
> +<command>mailboxes</command> command.
> +This function requires
> +<link linkend="mail-check-stats">$mail_check_stats</link> to be set.
So, if we agree on that, we can remove the above sentence saying
$mail_check_stats is required. What do you think?
--
Kevin J. McCarthy
GPG Fingerprint: 8975 A9B3 3AA3 7910 385C 5308 ADEF 7684 8031 6BDA
signature.asc
Description: PGP signature
