|
Mark Walters |
|
|
This is version 2 of the patch set first posted in id:"[hidden email]" This fixes the bug Jani pointed out as well as the more minor criticisms. It also has some tests and the man pages are updated. Best wishes Mark _______________________________________________ notmuch mailing list [hidden email] http://notmuchmail.org/mailman/listinfo/notmuch |
|
Mark Walters |
|
|
Add an option --from= to notmuch-reply.c to restrict guessing of the
From: header. The existing logic looks as the main headers, then at the delivery headers, and finally defaults to the config file address. This patch allows the user to restrict which of these guesses are made. Currently the supported values are: default|fallback-all current behaviour fallback-received fallback to delivery headers but not config file fallback-none only look at from/reply-to/to/cc/ headers none From: header is always left empty If the code does not find an allowed address it outputs an empty From: line and the caller can decide how to respond. --- notmuch-reply.c | 45 ++++++++++++++++++++++++++++++++++++--------- 1 files changed, 36 insertions(+), 9 deletions(-) diff --git a/notmuch-reply.c b/notmuch-reply.c index f55b1d2..8c73cb7 100644 --- a/notmuch-reply.c +++ b/notmuch-reply.c @@ -24,6 +24,15 @@ #include "gmime-filter-reply.h" #include "gmime-filter-headers.h" +/* The order here matters as we use '<' when deciding how to behave. */ +enum { + FROM_FALLBACK_ALL, + FROM_FALLBACK_RECEIVED, + FROM_FALLBACK_NONE, + FROM_NONE, + FROM_PRIMARY +}; + static void reply_headers_message_part (GMimeMessage *message); @@ -510,7 +519,8 @@ notmuch_reply_format_default(void *ctx, notmuch_config_t *config, notmuch_query_t *query, notmuch_show_params_t *params, - notmuch_bool_t reply_all) + notmuch_bool_t reply_all, + int from_select) { GMimeMessage *reply; notmuch_messages_t *messages; @@ -542,15 +552,22 @@ notmuch_reply_format_default(void *ctx, from_addr = add_recipients_from_message (reply, config, message, reply_all); - if (from_addr == NULL) + if (from_addr == NULL && from_select <= FROM_FALLBACK_RECEIVED) from_addr = guess_from_received_header (config, message); - if (from_addr == NULL) + if ((from_addr == NULL && from_select <= FROM_FALLBACK_ALL) || + from_select == FROM_PRIMARY) from_addr = notmuch_config_get_user_primary_email (config); - from_addr = talloc_asprintf (ctx, "%s <%s>", - notmuch_config_get_user_name (config), - from_addr); + /* If we have an address and we want an address print + * it. Otherwise set an empty From: header. */ + if (from_addr != NULL && from_select != FROM_NONE) { + from_addr = talloc_asprintf (ctx, "%s <%s>", + notmuch_config_get_user_name (config), + from_addr); + } else { + from_addr = talloc_strdup (ctx, ""); + } g_mime_object_set_header (GMIME_OBJECT (reply), "From", from_addr); @@ -590,7 +607,8 @@ notmuch_reply_format_headers_only(void *ctx, notmuch_config_t *config, notmuch_query_t *query, unused (notmuch_show_params_t *params), - notmuch_bool_t reply_all) + notmuch_bool_t reply_all, + unused (int from_select)) { GMimeMessage *reply; notmuch_messages_t *messages; @@ -657,10 +675,11 @@ notmuch_reply_command (void *ctx, int argc, char *argv[]) notmuch_query_t *query; char *query_string; int opt_index, ret = 0; - int (*reply_format_func)(void *ctx, notmuch_config_t *config, notmuch_query_t *query, notmuch_show_params_t *params, notmuch_bool_t reply_all); + int (*reply_format_func)(void *ctx, notmuch_config_t *config, notmuch_query_t *query, notmuch_show_params_t *params, notmuch_bool_t reply_all, int from_select); notmuch_show_params_t params = { .part = -1 }; int format = FORMAT_DEFAULT; int reply_all = TRUE; + int from_select = FROM_FALLBACK_ALL; notmuch_bool_t decrypt = FALSE; notmuch_opt_desc_t options[] = { @@ -672,6 +691,14 @@ notmuch_reply_command (void *ctx, int argc, char *argv[]) (notmuch_keyword_t []){ { "all", TRUE }, { "sender", FALSE }, { 0, 0 } } }, + { NOTMUCH_OPT_KEYWORD, &from_select, "from", 'F', + (notmuch_keyword_t []){ { "default", FROM_FALLBACK_ALL }, + { "fallback-all", FROM_FALLBACK_ALL }, + { "fallback-received", FROM_FALLBACK_RECEIVED }, + { "fallback-none", FROM_FALLBACK_NONE }, + { "none", FROM_NONE }, + { "primary", FROM_PRIMARY }, + { 0, 0 } } }, { NOTMUCH_OPT_BOOLEAN, &decrypt, "decrypt", 'd', 0 }, { 0, 0, 0, 0, 0 } }; @@ -732,7 +759,7 @@ notmuch_reply_command (void *ctx, int argc, char *argv[]) return 1; } - if (reply_format_func (ctx, config, query, ¶ms, reply_all) != 0) + if (reply_format_func (ctx, config, query, ¶ms, reply_all, from_select) != 0) return 1; notmuch_query_destroy (query); -- 1.7.2.3 _______________________________________________ notmuch mailing list [hidden email] http://notmuchmail.org/mailman/listinfo/notmuch |
|
Mark Walters |
|
|
In reply to this post by Mark Walters
---
man/man1/notmuch-reply.1 | 27 +++++++++++++++++++++++++++ 1 files changed, 27 insertions(+), 0 deletions(-) diff --git a/man/man1/notmuch-reply.1 b/man/man1/notmuch-reply.1 index 5160ece..101ed47 100644 --- a/man/man1/notmuch-reply.1 +++ b/man/man1/notmuch-reply.1 @@ -62,6 +62,33 @@ addresses), try To:, Cc:, and Bcc: headers in this order, and copy values from the first that contains something other than only the user's addresses. .RE +.TP 4 +.BR \-\-from= ( default | fallback-all | fallback-received | fallback-none | none | primary ) +.RS +.TP 4 +Controls the choice of the From: header returned. +.TP 4 +.BR default | fallback-all +Chooses the From: address by looking at the reply-to/from/to/cc +headers of the message being replied to, if no user address is found +look in the delivery headers, if still no user address is found return +the user's primary address. +.TP 4 +.BR fallback-received +As above but if no user address is found after looking in the delivery +headers returns an empty From: header. +.TP 4 +.BR fallback-none +Only look at the reply-to/from/to/cc of the original message. If no +user address is found return an empty From: header. +.TP 4 +.BR none +Always return an empty From: header. +.TP 4 +.BR primary +Always return the users primary address as the From: header. +.RE +.RE .RE See \fBnotmuch-search-terms\fR(7) -- 1.7.2.3 _______________________________________________ notmuch mailing list [hidden email] http://notmuchmail.org/mailman/listinfo/notmuch |
|
Mark Walters |
|
|
In reply to this post by Mark Walters
This patch uses the new --from option to notmuch reply to allow it to
prompt the user for the From: address in cases when the cli does not know the "correct" from address. If the cli does not it either uses the users default address or, if notmuch-always-prompt-for-sender is set, prompts the user. --- emacs/notmuch-mua.el | 47 ++++++++++++++++++++++++++++------------------- 1 files changed, 28 insertions(+), 19 deletions(-) diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el index 41f82c2..36e62f9 100644 --- a/emacs/notmuch-mua.el +++ b/emacs/notmuch-mua.el @@ -51,6 +51,24 @@ list." ;; +(defcustom notmuch-identities nil + "Identities that can be used as the From: address when composing a new message. + +If this variable is left unset, then a list will be constructed from the +name and addresses configured in the notmuch configuration file." + :type '(repeat string) + :group 'notmuch-send) + +(defcustom notmuch-always-prompt-for-sender nil + "Always prompt for the From: address when composing or forwarding a message. + +This is not taken into account when replying to a message, because in that case +the From: header is already filled in by notmuch." + :type 'boolean + :group 'notmuch-send) + +(defvar notmuch-mua-sender-history nil) + (defun notmuch-mua-user-agent-full () "Generate a `User-Agent:' string suitable for notmuch." (concat (notmuch-mua-user-agent-notmuch) @@ -75,7 +93,7 @@ list." (defun notmuch-mua-reply (query-string &optional sender reply-all) (let (headers body - (args '("reply"))) + (args '("reply" "--from=fallback-received"))) (if notmuch-show-process-crypto (setq args (append args '("--decrypt")))) (if reply-all @@ -99,6 +117,15 @@ list." ;; If sender is non-nil, set the From: header to its value. (when sender (mail-header-set 'from sender headers)) + ;; If we do not have a From: header yet it means that + ;; notmuch-reply.c was not able to make a useful guess so we fill + ;; it in ourselves. + (when (string= "" (mail-header 'from headers)) + (if notmuch-always-prompt-for-sender + (setq sender (notmuch-mua-prompt-for-sender)) + (setq sender (concat + (notmuch-user-name) " <" (notmuch-user-primary-email) ">"))) + (mail-header-set 'from sender headers)) (let ;; Overlay the composition window on that being used to read ;; the original message. @@ -153,24 +180,6 @@ OTHER-ARGS are passed through to `message-mail'." (message-goto-to)) -(defcustom notmuch-identities nil - "Identities that can be used as the From: address when composing a new message. - -If this variable is left unset, then a list will be constructed from the -name and addresses configured in the notmuch configuration file." - :type '(repeat string) - :group 'notmuch-send) - -(defcustom notmuch-always-prompt-for-sender nil - "Always prompt for the From: address when composing or forwarding a message. - -This is not taken into account when replying to a message, because in that case -the From: header is already filled in by notmuch." - :type 'boolean - :group 'notmuch-send) - -(defvar notmuch-mua-sender-history nil) - (defun notmuch-mua-prompt-for-sender () (interactive) (let (name addresses one-name-only) -- 1.7.2.3 _______________________________________________ notmuch mailing list [hidden email] http://notmuchmail.org/mailman/listinfo/notmuch |
|
Mark Walters |
|
|
In reply to this post by Mark Walters
---
test/reply | 69 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 69 insertions(+), 0 deletions(-) diff --git a/test/reply b/test/reply index 00f4bea..0bb22bb 100755 --- a/test/reply +++ b/test/reply @@ -138,4 +138,73 @@ References: <${gen_msg_id}> On Tue, 05 Jan 2010 15:43:56 -0000, Notmuch Test Suite <[hidden email]> wrote: > 200-byte header" + +test_begin_subtest "reply --from=none" +add_message '[from]="Sender <[hidden email]>"' \ + [to]=[hidden email] \ + [subject]=notmuch-reply-test \ + '[date]="Tue, 05 Jan 2010 15:43:56 -0000"' \ + '[body]="reply --from=none test"' + +output=$(notmuch reply --from=none id:${gen_msg_id}) +test_expect_equal "$output" "From: +Subject: Re: notmuch-reply-test +To: Sender <[hidden email]> +In-Reply-To: <${gen_msg_id}> +References: <${gen_msg_id}> + +On Tue, 05 Jan 2010 15:43:56 -0000, Sender <[hidden email]> wrote: +> reply --from=none test" + +test_begin_subtest "reply --from:fallback-none" +add_message '[from]="Sender <[hidden email]>"' \ + '[to]="Someone Else <[hidden email]>"' \ + [subject]=notmuch-reply-test \ + '[date]="Tue, 05 Jan 2010 15:43:56 -0000"' \ + '[body]="reply --from=fallback-none test"' + +output=$(notmuch reply --from=fallback-none id:${gen_msg_id}) +test_expect_equal "$output" "From: +Subject: Re: notmuch-reply-test +To: Sender <[hidden email]>, Someone Else <[hidden email]> +In-Reply-To: <${gen_msg_id}> +References: <${gen_msg_id}> + +On Tue, 05 Jan 2010 15:43:56 -0000, Sender <[hidden email]> wrote: +> reply --from=fallback-none test" + +test_begin_subtest "reply default from:" +add_message '[from]="Sender <[hidden email]>"' \ + '[to]="Someone Else <[hidden email]>"' \ + [subject]=notmuch-reply-test \ + '[date]="Tue, 05 Jan 2010 15:43:56 -0000"' \ + '[body]="reply default from: test"' + +output=$(notmuch reply id:${gen_msg_id}) +test_expect_equal "$output" "From: Notmuch Test Suite <[hidden email]> +Subject: Re: notmuch-reply-test +To: Sender <[hidden email]>, Someone Else <[hidden email]> +In-Reply-To: <${gen_msg_id}> +References: <${gen_msg_id}> + +On Tue, 05 Jan 2010 15:43:56 -0000, Sender <[hidden email]> wrote: +> reply default from: test" + +test_begin_subtest "Reply --from=primary" +add_message '[from]="Sender <[hidden email]>"' \ + [to]=[hidden email] \ + [subject]=notmuch-reply-test \ + '[date]="Tue, 05 Jan 2010 15:43:56 -0000"' \ + '[body]="reply --from=primary"' + +output=$(notmuch reply --from=primary id:${gen_msg_id}) +test_expect_equal "$output" "From: Notmuch Test Suite <[hidden email]> +Subject: Re: notmuch-reply-test +To: Sender <[hidden email]> +In-Reply-To: <${gen_msg_id}> +References: <${gen_msg_id}> + +On Tue, 05 Jan 2010 15:43:56 -0000, Sender <[hidden email]> wrote: +> reply --from=primary" + test_done -- 1.7.2.3 _______________________________________________ notmuch mailing list [hidden email] http://notmuchmail.org/mailman/listinfo/notmuch |
|
Jani Nikula |
|
|
In reply to this post by Mark Walters
On Sat, 4 Feb 2012 20:45:14 +0000, Mark Walters <[hidden email]> wrote:
> Add an option --from= to notmuch-reply.c to restrict guessing of the > From: header. The existing logic looks as the main headers, then at > the delivery headers, and finally defaults to the config file address. > > This patch allows the user to restrict which of these guesses are > made. Currently the supported values are: > default|fallback-all current behaviour > fallback-received fallback to delivery headers but not config file > fallback-none only look at from/reply-to/to/cc/ headers > none From: header is always left empty The patch looks good. The "primary" option added in v2 is missing from the commit message, but no need to send a new version because of that. I didn't check the other patches in the series. BR, Jani. > > If the code does not find an allowed address it outputs an empty From: > line and the caller can decide how to respond. > --- > notmuch-reply.c | 45 ++++++++++++++++++++++++++++++++++++--------- > 1 files changed, 36 insertions(+), 9 deletions(-) > > diff --git a/notmuch-reply.c b/notmuch-reply.c > index f55b1d2..8c73cb7 100644 > --- a/notmuch-reply.c > +++ b/notmuch-reply.c > @@ -24,6 +24,15 @@ > #include "gmime-filter-reply.h" > #include "gmime-filter-headers.h" > > +/* The order here matters as we use '<' when deciding how to behave. */ > +enum { > + FROM_FALLBACK_ALL, > + FROM_FALLBACK_RECEIVED, > + FROM_FALLBACK_NONE, > + FROM_NONE, > + FROM_PRIMARY > +}; > + > static void > reply_headers_message_part (GMimeMessage *message); > > @@ -510,7 +519,8 @@ notmuch_reply_format_default(void *ctx, > notmuch_config_t *config, > notmuch_query_t *query, > notmuch_show_params_t *params, > - notmuch_bool_t reply_all) > + notmuch_bool_t reply_all, > + int from_select) > { > GMimeMessage *reply; > notmuch_messages_t *messages; > @@ -542,15 +552,22 @@ notmuch_reply_format_default(void *ctx, > from_addr = add_recipients_from_message (reply, config, message, > reply_all); > > - if (from_addr == NULL) > + if (from_addr == NULL && from_select <= FROM_FALLBACK_RECEIVED) > from_addr = guess_from_received_header (config, message); > > - if (from_addr == NULL) > + if ((from_addr == NULL && from_select <= FROM_FALLBACK_ALL) || > + from_select == FROM_PRIMARY) > from_addr = notmuch_config_get_user_primary_email (config); > > - from_addr = talloc_asprintf (ctx, "%s <%s>", > - notmuch_config_get_user_name (config), > - from_addr); > + /* If we have an address and we want an address print > + * it. Otherwise set an empty From: header. */ > + if (from_addr != NULL && from_select != FROM_NONE) { > + from_addr = talloc_asprintf (ctx, "%s <%s>", > + notmuch_config_get_user_name (config), > + from_addr); > + } else { > + from_addr = talloc_strdup (ctx, ""); > + } > g_mime_object_set_header (GMIME_OBJECT (reply), > "From", from_addr); > > @@ -590,7 +607,8 @@ notmuch_reply_format_headers_only(void *ctx, > notmuch_config_t *config, > notmuch_query_t *query, > unused (notmuch_show_params_t *params), > - notmuch_bool_t reply_all) > + notmuch_bool_t reply_all, > + unused (int from_select)) > { > GMimeMessage *reply; > notmuch_messages_t *messages; > @@ -657,10 +675,11 @@ notmuch_reply_command (void *ctx, int argc, char *argv[]) > notmuch_query_t *query; > char *query_string; > int opt_index, ret = 0; > - int (*reply_format_func)(void *ctx, notmuch_config_t *config, notmuch_query_t *query, notmuch_show_params_t *params, notmuch_bool_t reply_all); > + int (*reply_format_func)(void *ctx, notmuch_config_t *config, notmuch_query_t *query, notmuch_show_params_t *params, notmuch_bool_t reply_all, int from_select); > notmuch_show_params_t params = { .part = -1 }; > int format = FORMAT_DEFAULT; > int reply_all = TRUE; > + int from_select = FROM_FALLBACK_ALL; > notmuch_bool_t decrypt = FALSE; > > notmuch_opt_desc_t options[] = { > @@ -672,6 +691,14 @@ notmuch_reply_command (void *ctx, int argc, char *argv[]) > (notmuch_keyword_t []){ { "all", TRUE }, > { "sender", FALSE }, > { 0, 0 } } }, > + { NOTMUCH_OPT_KEYWORD, &from_select, "from", 'F', > + (notmuch_keyword_t []){ { "default", FROM_FALLBACK_ALL }, > + { "fallback-all", FROM_FALLBACK_ALL }, > + { "fallback-received", FROM_FALLBACK_RECEIVED }, > + { "fallback-none", FROM_FALLBACK_NONE }, > + { "none", FROM_NONE }, > + { "primary", FROM_PRIMARY }, > + { 0, 0 } } }, > { NOTMUCH_OPT_BOOLEAN, &decrypt, "decrypt", 'd', 0 }, > { 0, 0, 0, 0, 0 } > }; > @@ -732,7 +759,7 @@ notmuch_reply_command (void *ctx, int argc, char *argv[]) > return 1; > } > > - if (reply_format_func (ctx, config, query, ¶ms, reply_all) != 0) > + if (reply_format_func (ctx, config, query, ¶ms, reply_all, from_select) != 0) > return 1; > > notmuch_query_destroy (query); > -- > 1.7.2.3 > > _______________________________________________ > notmuch mailing list > [hidden email] > http://notmuchmail.org/mailman/listinfo/notmuch notmuch mailing list [hidden email] http://notmuchmail.org/mailman/listinfo/notmuch |
|
Dmitry Kurochkin |
|
|
In reply to this post by Mark Walters
Hi Mark.
I am not sure I like this solution. My concerns are: * New option looks too complex, too specific. * There are more aspects of notmuch reply behavior which users would like to change (e.g. which part to quote). If we add an option for each, we complicate both nomtuch show UI and code. The problem is that notmuch show output format is too limiting. Instead of providing myriad of options for tweaking notmuch show text format behavior, we should add JSON format for notmuch reply similar to nomtuch show. That would allow notmuch reply to produce structured output with required additional information, which should be enough for users to construct whatever reply they want. In this particular case, notmuch reply JSON format could have "from-source" attribute that would indicate how it was guessed. Now the best part. Not so long ago, Adam (in Cc) provided a patch for improving nomtuch reply for HTML-only emails. At first he added an option for notmuch reply, like you did for from-guessing. I suggested him to implement it based on the JSON format instead and he did. AFAIK the latest version of his patches is [1]. I did not look at the code though. It seems that it is waiting for more review. So, instead of adding more nomtuch show options, I think a better solution is to work with Adam to get the notmuch reply JSON format to master and then fix the from-guessing issue by adding an attribute to notmuch reply JSON format. Regards, Dmitry [1] id:"[hidden email]" _______________________________________________ notmuch mailing list [hidden email] http://notmuchmail.org/mailman/listinfo/notmuch |
|
Mark Walters |
|
|
On Sun, 05 Feb 2012 10:58:04 +0400, Dmitry Kurochkin <[hidden email]> wrote:
> Hi Mark. > > I am not sure I like this solution. My concerns are: > > * New option looks too complex, too specific. > > * There are more aspects of notmuch reply behavior which users would > like to change (e.g. which part to quote). If we add an option for > each, we complicate both nomtuch show UI and code. > > The problem is that notmuch show output format is too limiting. Instead > of providing myriad of options for tweaking notmuch show text format > behavior, we should add JSON format for notmuch reply similar to nomtuch > show. That would allow notmuch reply to produce structured output with > required additional information, which should be enough for users to > construct whatever reply they want. > > In this particular case, notmuch reply JSON format could have > "from-source" attribute that would indicate how it was guessed. > > Now the best part. Not so long ago, Adam (in Cc) provided a patch for > improving nomtuch reply for HTML-only emails. At first he added an > option for notmuch reply, like you did for from-guessing. I suggested > him to implement it based on the JSON format instead and he did. AFAIK > the latest version of his patches is [1]. I did not look at the code > though. It seems that it is waiting for more review. > > So, instead of adding more nomtuch show options, I think a better > solution is to work with Adam to get the notmuch reply JSON format to > master and then fix the from-guessing issue by adding an attribute to > notmuch reply JSON format. Yes I think you are quite right: this would be a better solution (particularly as Adam has already done all the hard work!) Many thanks Mark _______________________________________________ notmuch mailing list [hidden email] http://notmuchmail.org/mailman/listinfo/notmuch |
|
Jani Nikula |
|
|
In reply to this post by Dmitry Kurochkin
On Sun, 05 Feb 2012 10:58:04 +0400, Dmitry Kurochkin <[hidden email]> wrote:
> Hi Mark. > > I am not sure I like this solution. My concerns are: > > * New option looks too complex, too specific. *shrug* The --from parameter is simple to implement, simple to test, and simple to use. > * There are more aspects of notmuch reply behavior which users would > like to change (e.g. which part to quote). If we add an option for > each, we complicate both nomtuch show UI and code. This I agree is more of an issue. > The problem is that notmuch show output format is too limiting. Instead > of providing myriad of options for tweaking notmuch show text format > behavior, we should add JSON format for notmuch reply similar to nomtuch > show. That would allow notmuch reply to produce structured output with > required additional information, which should be enough for users to > construct whatever reply they want. Heh, when I told Mark on IRC to just send the patches and not discuss and worry about it too much, I added "...and then someone will come up with an approach we failed to think of, and scrap the whole thing". Thanks Dmitry! ;) > In this particular case, notmuch reply JSON format could have > "from-source" attribute that would indicate how it was guessed. My first thought is that it's offloading things that are trivial in the cli to the users of the cli where it might be slightly more complicated, but... > Now the best part. Not so long ago, Adam (in Cc) provided a patch for > improving nomtuch reply for HTML-only emails. At first he added an > option for notmuch reply, like you did for from-guessing. I suggested > him to implement it based on the JSON format instead and he did. AFAIK > the latest version of his patches is [1]. I did not look at the code > though. It seems that it is waiting for more review. > > So, instead of adding more nomtuch show options, I think a better > solution is to work with Adam to get the notmuch reply JSON format to > master and then fix the from-guessing issue by adding an attribute to > notmuch reply JSON format. ...no matter what the solution will be in the end, I agree it's best to get Adam's work merged first, and see how easily this can be handled using JSON vs. the parameter. Thanks for your insights. BR, Jani. > Regards, > Dmitry > > [1] id:"[hidden email]" > _______________________________________________ > notmuch mailing list > [hidden email] > http://notmuchmail.org/mailman/listinfo/notmuch _______________________________________________ notmuch mailing list [hidden email] http://notmuchmail.org/mailman/listinfo/notmuch |
| Powered by Nabble | See how NAML generates this page |