Subject: Comments on draft-ietf-lemonade-imap-sieve-02.txt

From: Alexey Melnikov <[email-masked]>
Date: Wed, 28 Mar 2007 15:43:28 +0100

Hi Barry,
The document is in a much better shape now. Thank you.

Some comments below:

> 3.1.  The Implicit Keep
>
>    For all cases that fall under IMAPSieve, the implicit keep means that
>    the message is treated as it would have been if no Sieve script were
>    run.  For APPEND, MULTIAPPEND and COPY, the message is stored into
>    the target mailbox normally.  For flag or annotation changes, the
>    message is left in the mailbox.

And would the implicit keep mean for EXPUNGEs?

> 3.6.  The Discard Action
>
>    The discard action is applicable in all cases that fall under
>    IMAPSieve.  For APPEND, MULTIAPPEND, and COPY, the message is first
>    stored into the target mailbox.  If a keep action, implicit or
>    explicit, is also in effect, the discard action now does nothing.

'Discard' cancels the implicit keep, so it can't "do nothing".

>    Otherwise, the original message is then marked with the \Deleted flag
>    (and a flag-change Sieve script is NOT invoked).

We've discussed this briefly in Lemonade: this text doesn't agree with 
the definition of "discard" in the base Sieve document - it is just an 
action that cancels the implicit keep. After the discussion I think your 
interpretation of discard in IMAP Sieve is fine with me, but you need to 
add more text to describe the motivation.

> 3.8.  The Addheader and Deleteheader Actions
>
>    Even if the [EditHeader] extension is available, since messages in
>    IMAP mailboxes are immutable these actions are NOT applicable.  Using
>    them for flag or annotation changes to existing messages would cause
>    the message to be changed.  Using them for APPEND, MULTIAPPEND, and
>    COPY would cause unexpected differences in the stored copy as
>    compared to what the client expected, and would make the client's
>    message cache invalid unexpectedly.

As far as I can see the last two sentences are describing a "what if" 
situation, but I think the text as worded is not entirely clear on that.

>    Use of these MUST result in an
>    error condition that will terminate the Sieve script.
>
> 3.9.  The Setflag, Deleteflag, and Removeflag Actions
>
>    If the [IMAP4Flags] extension is available, the actions associated
>    with it are all applicable to any case that falls under IMAPSieve.
>    It is worth noting also that the "hasflag" test that is defined in
>    the IMAP4Flags extension might be particularly useful in scripts
>    triggered by flag changes.

I think you need to clarify that "hasflag" will test the current set of 
flags
that is set for the current message, as visible in IMAP.

>    The flag changes behave as though a
>    client had made the change.

  [...]

> 4.  Open Issues With This Document

  [...]

>    2.  All of the extensions that we describe how to work with: are they
>        normative, or non-normative references?

I think most of them are normative, except for the ones which are 
explicitly disallowed (such as addheader). For example you update the 
behavior of redirect and fileinto actions.

> 5.  Examples
>
>    Example 1:
>    If a new message is added to the "ActionItems" mailbox, a copy is
>    sent to the address "[email-masked]".
>
>      require ["copy", "variables"];

Replace "variables" with "environment" here.

>    Example 2:
>    If the script is called for any message with the \Flagged flag set
>    (tested through the [IMAP4Flags] extension), a notification is sent
>    using the [Notify] extension.  No notification will be sent, though,
>    if we're called with an existing message that already had that flag
>    set.
>
>      require ["nofity", "imap4flags", "variables"];

Replace "variables" with "environment" here.

>
>      if allof (hasflag "\\Flagged",
>                not environment :contains "changedflags" "\\Flagged") {
>          notify :message "Important message in ${IMAPSieve.mailbox}";

The example needs to be fixed, because ${IMAPSieve.mailbox} is not 
defined anymore.

>      }
>
>
> 7.1.  Registration of imapsieve extension
>
>    The following template specifies the IANA registration of the Sieve
>    extension specified in this document:
>
>    To: [email-masked]
>    Subject: Registration of new Sieve extension
>    Capability name: imapsieve
>    Capability keyword: imapsieve
>    Capability arguments: N/A

The template in 3028bis has changed, so your registration needs to be 
updated.

Other comments:

I suggest you add the following requirement: 'IMAP Sieve interpreters 
SHOULD support Sieve "environment" test"'. And I would be Ok with it 
being a MUST.

---

From: "Aaron Stone" <[email-masked]>
Date: Wed, 28 Mar 2007 18:06:19 -0000

On Wed, Mar 28, 2007, Alexey Melnikov <[email-masked]> said:

[snip everything else]

> SHOULD support Sieve "environment" test"'. And I would be Ok with it 

+1. I think that IMAP Sieve should probably require a fairly good profile
of Sieve extensions, particularly those which allow the script to be smart
about the context it is running in.

Aaron