[PATCH v2 3/6] cli: add support for replying just to the sender in "notmuch reply"
> > The reply-to-thread is a rare case anyway, regardless of reply-to-all or > reply-to-sender, and even the current implementation does not gather all > the recipients from all the messages. Try it out, it seems to me it does > not quite do what you think it does. > > IMHO it should use oldest-first rather than newest-first sort order, and > I guess it should really go through the messages being replied to and > get the recipients from all of them. It's just out of scope for this > patch set to fix these. But if that gets fixed later, I would like the > reply-to-sender for multiple messages follow the logic of single-reply, > just with all the addresses put together. I completely agree it's a rare case. Having tried the code (yours and the original) it seems to be very strange: In its current form it seems to be reply to people on newest message in thread and include all the bodies with some spurious headers mixed in. So your code is reply-to-sender applied to people on newest message in thread, and include all the bodies with some slightly different spurious headers mixed in. I like your suggested solution. Anyway it's a rare case; the current code is odd and yours is no odder so I agree with leaving it for a later patch (if anyone cares enough!). Best wishes Mark
[PATCH v2 3/6] cli: add support for replying just to the sender in "notmuch reply"
On Sun, 08 Jan 2012 23:23:15 +, Mark Walters
wrote:
>
> I like this version (of the whole series) but have two queries. (Note I
> haven't actually tried it out yet: I have just been reading the code.)
>
> > + /* Force recipient type in reply-to-sender mode just in case replying to
> > +* user's own message finds recipients in Cc/Bcc fields only.
> > +*/
> > + if (reply_all)
> > + recipient_type = reply_to_map[i].recipient_type;
> > + else
> > + recipient_type = GMIME_RECIPIENT_TYPE_TO;
> > +
> > + addr = add_recipients_for_string (reply, config, recipient_type,
> > + recipients, add_recipients);
> > +
>
> Why force the recipient type? I do not think notmuch should ever move
> bcc people onto a different header. I think that will bite someone when
> it leaks information.
>
> I would be happy with either empty headers or with the people staying
> as bcc.
>
> In the cc: case I have no preference (as those addresses are already
> public) but I would suggest being consistent with bcc so either empty
> headers or keep them in the cc line.
I think you're right. I was trying to clean up a corner case in
reply-to-self, but probably over did it. I'll remove the recipient type
forcing altogether. Then it'll keep addresses in the same header as they
were.
> > for (messages = notmuch_query_search_messages (query);
> > notmuch_messages_valid (messages);
> > notmuch_messages_move_to_next (messages))
> > {
> [...]
> > (void)add_recipients_from_message (reply, config, message, reply_all);
>
> Since the above logic is applied to each email individually I think
> working out the recipients when replying to multiple emails (e.g.,
> reply-to-sender on a thread) could be very confusing. Some of the people
> being replied to will have been senders, some recipients (it is very
> likely that the thread contains messages we sent), some could even be
> cc, bcc people. Personally, I would have no idea what to expect from
> reply-to-sender in this case.
>
> (My personal choice would be not to allow notmuch-reply-to-sender if
> multiple messages are specified. But I can obtain that by unbinding "r"
> in the notmuch-search-mode-map keymap.)
The reply-to-thread is a rare case anyway, regardless of reply-to-all or
reply-to-sender, and even the current implementation does not gather all
the recipients from all the messages. Try it out, it seems to me it does
not quite do what you think it does.
IMHO it should use oldest-first rather than newest-first sort order, and
I guess it should really go through the messages being replied to and
get the recipients from all of them. It's just out of scope for this
patch set to fix these. But if that gets fixed later, I would like the
reply-to-sender for multiple messages follow the logic of single-reply,
just with all the addresses put together.
BR,
Jani.
Re: [PATCH v2 3/6] cli: add support for replying just to the sender in "notmuch reply"
> > The reply-to-thread is a rare case anyway, regardless of reply-to-all or > reply-to-sender, and even the current implementation does not gather all > the recipients from all the messages. Try it out, it seems to me it does > not quite do what you think it does. > > IMHO it should use oldest-first rather than newest-first sort order, and > I guess it should really go through the messages being replied to and > get the recipients from all of them. It's just out of scope for this > patch set to fix these. But if that gets fixed later, I would like the > reply-to-sender for multiple messages follow the logic of single-reply, > just with all the addresses put together. I completely agree it's a rare case. Having tried the code (yours and the original) it seems to be very strange: In its current form it seems to be reply to people on newest message in thread and include all the bodies with some spurious headers mixed in. So your code is reply-to-sender applied to people on newest message in thread, and include all the bodies with some slightly different spurious headers mixed in. I like your suggested solution. Anyway it's a rare case; the current code is odd and yours is no odder so I agree with leaving it for a later patch (if anyone cares enough!). Best wishes Mark ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH v2 3/6] cli: add support for replying just to the sender in "notmuch reply"
On Sun, 08 Jan 2012 23:23:15 +, Mark Walters
wrote:
>
> I like this version (of the whole series) but have two queries. (Note I
> haven't actually tried it out yet: I have just been reading the code.)
>
> > + /* Force recipient type in reply-to-sender mode just in case replying to
> > +* user's own message finds recipients in Cc/Bcc fields only.
> > +*/
> > + if (reply_all)
> > + recipient_type = reply_to_map[i].recipient_type;
> > + else
> > + recipient_type = GMIME_RECIPIENT_TYPE_TO;
> > +
> > + addr = add_recipients_for_string (reply, config, recipient_type,
> > + recipients, add_recipients);
> > +
>
> Why force the recipient type? I do not think notmuch should ever move
> bcc people onto a different header. I think that will bite someone when
> it leaks information.
>
> I would be happy with either empty headers or with the people staying
> as bcc.
>
> In the cc: case I have no preference (as those addresses are already
> public) but I would suggest being consistent with bcc so either empty
> headers or keep them in the cc line.
I think you're right. I was trying to clean up a corner case in
reply-to-self, but probably over did it. I'll remove the recipient type
forcing altogether. Then it'll keep addresses in the same header as they
were.
> > for (messages = notmuch_query_search_messages (query);
> > notmuch_messages_valid (messages);
> > notmuch_messages_move_to_next (messages))
> > {
> [...]
> > (void)add_recipients_from_message (reply, config, message, reply_all);
>
> Since the above logic is applied to each email individually I think
> working out the recipients when replying to multiple emails (e.g.,
> reply-to-sender on a thread) could be very confusing. Some of the people
> being replied to will have been senders, some recipients (it is very
> likely that the thread contains messages we sent), some could even be
> cc, bcc people. Personally, I would have no idea what to expect from
> reply-to-sender in this case.
>
> (My personal choice would be not to allow notmuch-reply-to-sender if
> multiple messages are specified. But I can obtain that by unbinding "r"
> in the notmuch-search-mode-map keymap.)
The reply-to-thread is a rare case anyway, regardless of reply-to-all or
reply-to-sender, and even the current implementation does not gather all
the recipients from all the messages. Try it out, it seems to me it does
not quite do what you think it does.
IMHO it should use oldest-first rather than newest-first sort order, and
I guess it should really go through the messages being replied to and
get the recipients from all of them. It's just out of scope for this
patch set to fix these. But if that gets fixed later, I would like the
reply-to-sender for multiple messages follow the logic of single-reply,
just with all the addresses put together.
BR,
Jani.
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v2 3/6] cli: add support for replying just to the sender in "notmuch reply"
Add new option --reply-to=(all|sender) to "notmuch reply" to select whether
to reply to all (sender and all recipients), or just sender. Reply to all
remains the default.
Credits to Mark Walters for his similar earlier
work where I picked up the basic idea of handling reply-to-sender in
add_recipients_from_message(). All bugs are mine, though.
Signed-off-by: Jani Nikula
---
Settled on --reply-to=(all|sender) per Carl's earlier suggestion
(id:87pqn5cg4g.fsf at yoom.home.cworth.org) and David's approval on IRC.
---
man/man1/notmuch-reply.1 | 28 +---
notmuch-reply.c | 78 --
2 files changed, 84 insertions(+), 22 deletions(-)
diff --git a/man/man1/notmuch-reply.1 b/man/man1/notmuch-reply.1
index 099d808..21afa35 100644
--- a/man/man1/notmuch-reply.1
+++ b/man/man1/notmuch-reply.1
@@ -14,11 +14,13 @@ Constructs a reply template for a set of messages.
To make replying to email easier,
.B notmuch reply
takes an existing set of messages and constructs a suitable mail
-template. The Reply-to header (if any, otherwise From:) is used for
-the To: address. Vales from the To: and Cc: headers are copied, but
-not including any of the current user's email addresses (as configured
-in primary_mail or other_email in the .notmuch\-config file) in the
-recipient list
+template. The Reply-to: header (if any, otherwise From:) is used for
+the To: address. Unless
+.BR \-\-reply-to=sender
+is specified, values from the To: and Cc: headers are copied, but not
+including any of the current user's email addresses (as configured in
+primary_mail or other_email in the .notmuch\-config file) in the
+recipient list.
It also builds a suitable new subject, including Re: at the front (if
not already present), and adding the message IDs of the messages being
@@ -45,6 +47,22 @@ Includes subject and quoted message body.
Only produces In\-Reply\-To, References, To, Cc, and Bcc headers.
.RE
.RE
+.RS
+.TP 4
+.BR \-\-reply\-to= ( all | sender )
+.RS
+.TP 4
+.BR all " (default)"
+Replies to all addresses.
+.TP 4
+.BR sender
+Replies only to the sender. If Reply-to: header (if any, otherwise
+From:) is any of the current user's configured email addresses
+(replying to user's own message), try To:, Cc:, and Bcc: headers in
+this order, and use the addresses in the first that contains something
+other than only the user's addresses for the To: address.
+.RE
+.RE
See \fBnotmuch-search-terms\fR(7)
for details of the supported syntax for .
diff --git a/notmuch-reply.c b/notmuch-reply.c
index 000f6da..71946b3 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -170,7 +170,7 @@ address_is_users (const char *address, notmuch_config_t
*config)
/* For each address in 'list' that is not configured as one of the
* user's addresses in 'config', add that address to 'message' as an
- * address of 'type'.
+ * address of 'type', if 'add' is true.
*
* The first address encountered that *is* the user's address will be
* returned, (otherwise NULL is returned).
@@ -179,7 +179,8 @@ static const char *
add_recipients_for_address_list (GMimeMessage *message,
notmuch_config_t *config,
GMimeRecipientType type,
-InternetAddressList *list)
+InternetAddressList *list,
+notmuch_bool_t add)
{
InternetAddress *address;
int i;
@@ -197,7 +198,7 @@ add_recipients_for_address_list (GMimeMessage *message,
continue;
add_recipients_for_address_list (message, config,
-type, group_list);
+type, group_list, add);
} else {
InternetAddressMailbox *mailbox;
const char *name;
@@ -211,7 +212,7 @@ add_recipients_for_address_list (GMimeMessage *message,
if (address_is_users (addr, config)) {
if (ret == NULL)
ret = addr;
- } else {
+ } else if (add) {
g_mime_message_add_recipient (message, type, name, addr);
}
}
@@ -222,7 +223,7 @@ add_recipients_for_address_list (GMimeMessage *message,
/* For each address in 'recipients' that is not configured as one of
* the user's addresses in 'config', add that address to 'message' as
- * an address of 'type'.
+ * an address of 'type', if 'add' is true.
*
* The first address encountered that *is* the user's address will be
* returned, (otherwise NULL is returned).
@@ -231,7 +232,8 @@ static const char *
add_recipients_for_string (GMimeMessage *message,
notmuch_config_t *config,
GMimeRecipientType type,
- const char *recipients)
+ const char *recipients,
+ notmuch_bool_t add)
{
InternetAddressList
[PATCH v2 3/6] cli: add support for replying just to the sender in "notmuch reply"
I like this version (of the whole series) but have two queries. (Note I
haven't actually tried it out yet: I have just been reading the code.)
> + /* Force recipient type in reply-to-sender mode just in case replying to
> + * user's own message finds recipients in Cc/Bcc fields only.
> + */
> + if (reply_all)
> + recipient_type = reply_to_map[i].recipient_type;
> + else
> + recipient_type = GMIME_RECIPIENT_TYPE_TO;
> +
> + addr = add_recipients_for_string (reply, config, recipient_type,
> + recipients, add_recipients);
> +
Why force the recipient type? I do not think notmuch should ever move
bcc people onto a different header. I think that will bite someone when
it leaks information.
I would be happy with either empty headers or with the people staying
as bcc.
In the cc: case I have no preference (as those addresses are already
public) but I would suggest being consistent with bcc so either empty
headers or keep them in the cc line.
> for (messages = notmuch_query_search_messages (query);
> notmuch_messages_valid (messages);
> notmuch_messages_move_to_next (messages))
> {
[...]
> (void)add_recipients_from_message (reply, config, message, reply_all);
Since the above logic is applied to each email individually I think
working out the recipients when replying to multiple emails (e.g.,
reply-to-sender on a thread) could be very confusing. Some of the people
being replied to will have been senders, some recipients (it is very
likely that the thread contains messages we sent), some could even be
cc, bcc people. Personally, I would have no idea what to expect from
reply-to-sender in this case.
(My personal choice would be not to allow notmuch-reply-to-sender if
multiple messages are specified. But I can obtain that by unbinding "r"
in the notmuch-search-mode-map keymap.)
Best wishes
Mark
Re: [PATCH v2 3/6] cli: add support for replying just to the sender in "notmuch reply"
I like this version (of the whole series) but have two queries. (Note I
haven't actually tried it out yet: I have just been reading the code.)
> + /* Force recipient type in reply-to-sender mode just in case replying to
> + * user's own message finds recipients in Cc/Bcc fields only.
> + */
> + if (reply_all)
> + recipient_type = reply_to_map[i].recipient_type;
> + else
> + recipient_type = GMIME_RECIPIENT_TYPE_TO;
> +
> + addr = add_recipients_for_string (reply, config, recipient_type,
> + recipients, add_recipients);
> +
Why force the recipient type? I do not think notmuch should ever move
bcc people onto a different header. I think that will bite someone when
it leaks information.
I would be happy with either empty headers or with the people staying
as bcc.
In the cc: case I have no preference (as those addresses are already
public) but I would suggest being consistent with bcc so either empty
headers or keep them in the cc line.
> for (messages = notmuch_query_search_messages (query);
> notmuch_messages_valid (messages);
> notmuch_messages_move_to_next (messages))
> {
[...]
> (void)add_recipients_from_message (reply, config, message, reply_all);
Since the above logic is applied to each email individually I think
working out the recipients when replying to multiple emails (e.g.,
reply-to-sender on a thread) could be very confusing. Some of the people
being replied to will have been senders, some recipients (it is very
likely that the thread contains messages we sent), some could even be
cc, bcc people. Personally, I would have no idea what to expect from
reply-to-sender in this case.
(My personal choice would be not to allow notmuch-reply-to-sender if
multiple messages are specified. But I can obtain that by unbinding "r"
in the notmuch-search-mode-map keymap.)
Best wishes
Mark
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v2 3/6] cli: add support for replying just to the sender in "notmuch reply"
Add new option --reply-to=(all|sender) to "notmuch reply" to select whether
to reply to all (sender and all recipients), or just sender. Reply to all
remains the default.
Credits to Mark Walters for his similar earlier
work where I picked up the basic idea of handling reply-to-sender in
add_recipients_from_message(). All bugs are mine, though.
Signed-off-by: Jani Nikula
---
Settled on --reply-to=(all|sender) per Carl's earlier suggestion
(id:87pqn5cg4g@yoom.home.cworth.org) and David's approval on IRC.
---
man/man1/notmuch-reply.1 | 28 +---
notmuch-reply.c | 78 --
2 files changed, 84 insertions(+), 22 deletions(-)
diff --git a/man/man1/notmuch-reply.1 b/man/man1/notmuch-reply.1
index 099d808..21afa35 100644
--- a/man/man1/notmuch-reply.1
+++ b/man/man1/notmuch-reply.1
@@ -14,11 +14,13 @@ Constructs a reply template for a set of messages.
To make replying to email easier,
.B notmuch reply
takes an existing set of messages and constructs a suitable mail
-template. The Reply-to header (if any, otherwise From:) is used for
-the To: address. Vales from the To: and Cc: headers are copied, but
-not including any of the current user's email addresses (as configured
-in primary_mail or other_email in the .notmuch\-config file) in the
-recipient list
+template. The Reply-to: header (if any, otherwise From:) is used for
+the To: address. Unless
+.BR \-\-reply-to=sender
+is specified, values from the To: and Cc: headers are copied, but not
+including any of the current user's email addresses (as configured in
+primary_mail or other_email in the .notmuch\-config file) in the
+recipient list.
It also builds a suitable new subject, including Re: at the front (if
not already present), and adding the message IDs of the messages being
@@ -45,6 +47,22 @@ Includes subject and quoted message body.
Only produces In\-Reply\-To, References, To, Cc, and Bcc headers.
.RE
.RE
+.RS
+.TP 4
+.BR \-\-reply\-to= ( all | sender )
+.RS
+.TP 4
+.BR all " (default)"
+Replies to all addresses.
+.TP 4
+.BR sender
+Replies only to the sender. If Reply-to: header (if any, otherwise
+From:) is any of the current user's configured email addresses
+(replying to user's own message), try To:, Cc:, and Bcc: headers in
+this order, and use the addresses in the first that contains something
+other than only the user's addresses for the To: address.
+.RE
+.RE
See \fBnotmuch-search-terms\fR(7)
for details of the supported syntax for .
diff --git a/notmuch-reply.c b/notmuch-reply.c
index 000f6da..71946b3 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -170,7 +170,7 @@ address_is_users (const char *address, notmuch_config_t
*config)
/* For each address in 'list' that is not configured as one of the
* user's addresses in 'config', add that address to 'message' as an
- * address of 'type'.
+ * address of 'type', if 'add' is true.
*
* The first address encountered that *is* the user's address will be
* returned, (otherwise NULL is returned).
@@ -179,7 +179,8 @@ static const char *
add_recipients_for_address_list (GMimeMessage *message,
notmuch_config_t *config,
GMimeRecipientType type,
-InternetAddressList *list)
+InternetAddressList *list,
+notmuch_bool_t add)
{
InternetAddress *address;
int i;
@@ -197,7 +198,7 @@ add_recipients_for_address_list (GMimeMessage *message,
continue;
add_recipients_for_address_list (message, config,
-type, group_list);
+type, group_list, add);
} else {
InternetAddressMailbox *mailbox;
const char *name;
@@ -211,7 +212,7 @@ add_recipients_for_address_list (GMimeMessage *message,
if (address_is_users (addr, config)) {
if (ret == NULL)
ret = addr;
- } else {
+ } else if (add) {
g_mime_message_add_recipient (message, type, name, addr);
}
}
@@ -222,7 +223,7 @@ add_recipients_for_address_list (GMimeMessage *message,
/* For each address in 'recipients' that is not configured as one of
* the user's addresses in 'config', add that address to 'message' as
- * an address of 'type'.
+ * an address of 'type', if 'add' is true.
*
* The first address encountered that *is* the user's address will be
* returned, (otherwise NULL is returned).
@@ -231,7 +232,8 @@ static const char *
add_recipients_for_string (GMimeMessage *message,
notmuch_config_t *config,
GMimeRecipientType type,
- const char *recipients)
+ const char *recipients,
+ notmuch_bool_t add)
{
InternetAddressLi
