unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
* [PATCH v2 0/3] Improve the display of matching/non-matching authors.
@ 2014-10-24 17:44 David Edmondson
  2014-10-24 17:44 ` [PATCH v2 1/3] search: Separately report matching and non-matching authors David Edmondson
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: David Edmondson @ 2014-10-24 17:44 UTC (permalink / raw)
  To: notmuch


Improve the display of matching/non-matching authors.

Distinguishing between matching and non-matching authors in the emacs
interface is currently done by parsing the :authors attribute of a
search result. If one of the authors uses the pipe symbol (|) in their
'From' address this parsing incorrectly determines the matching and
non-matching authors.

Address this by adding explicit matching and non-matching authors
attributes to the structured output formats.

v2:
- Return the matching/non-matching authors as a list.
- More improvements to the code that renders the authors are possible
  (to improve the chosen break between visible and invisible), but a
  planned re-write of the `notmuch-search-result-format' code would
  render that irrelevant.


David Edmondson (3):
  search: Separately report matching and non-matching authors.
  emacs: Improved display of matching/non-matching authors.
  test: Update tests for 'authors_matched' and authors_non_matched'.

 emacs/notmuch.el             |  64 ++++++++++++++------------
 notmuch-search.c             | 105 +++++++++++++++++++++++++++++++++++++++++++
 test/T160-json.sh            |   9 ++++
 test/T170-sexp.sh            |   4 +-
 test/T470-missing-headers.sh |   8 ++++
 5 files changed, 159 insertions(+), 31 deletions(-)

-- 
2.1.1

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2 1/3] search: Separately report matching and non-matching authors.
  2014-10-24 17:44 [PATCH v2 0/3] Improve the display of matching/non-matching authors David Edmondson
@ 2014-10-24 17:44 ` David Edmondson
  2015-01-18 14:19   ` David Bremner
  2015-01-18 17:59   ` Mark Walters
  2014-10-24 17:44 ` [PATCH v2 2/3] emacs: Improved display of matching/non-matching authors David Edmondson
  2014-10-24 17:44 ` [PATCH v2 3/3] test: Update tests for 'authors_matched' and authors_non_matched' David Edmondson
  2 siblings, 2 replies; 11+ messages in thread
From: David Edmondson @ 2014-10-24 17:44 UTC (permalink / raw)
  To: notmuch

In addition to the 'authors' attribute of each search result, include
'authors_matched' and 'authors_non_matched' attributes. Both
attributes are always included and are formatted as a list of
authors. If there are no matching authors, the 'authors_non_matched'
attribute is set to the empty list.
---
 notmuch-search.c | 105 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 105 insertions(+)

diff --git a/notmuch-search.c b/notmuch-search.c
index bc9be45..18c3b20 100644
--- a/notmuch-search.c
+++ b/notmuch-search.c
@@ -22,6 +22,8 @@
 #include "sprinter.h"
 #include "string-util.h"
 
+#include <glib.h>
+
 typedef enum {
     OUTPUT_SUMMARY,
     OUTPUT_THREADS,
@@ -69,6 +71,105 @@ get_thread_query (notmuch_thread_t *thread,
     return 0;
 }
 
+/* Return a more pleasent rendering of the mail address
+ * `nasty_author'. */
+static const char *
+_nice_author (void *ctx, const char *nasty_author)
+{
+    const char *nice_author = NULL;
+
+    InternetAddressList *list = internet_address_list_parse_string (nasty_author);
+    if (list) {
+	InternetAddress *address = internet_address_list_get_address (list, 0);
+	if (address) {
+	    nice_author = internet_address_get_name (address);
+	    if (nice_author == NULL) {
+		InternetAddressMailbox *mailbox = INTERNET_ADDRESS_MAILBOX (address);
+		nice_author = internet_address_mailbox_get_addr (mailbox);
+	    }
+	}
+	/* Duplicate the string before `g_object_unref' destroys
+	 * it. */
+	if (nice_author)
+	    nice_author = talloc_strdup (ctx, nice_author);
+
+	g_object_unref (G_OBJECT (list));
+    }
+
+    if (nice_author)
+	return nice_author;
+    else
+	return nasty_author;
+}
+
+static int
+_enumerate_authors (sprinter_t *format,
+		 notmuch_thread_t *thread)
+{
+    notmuch_messages_t *messages;
+    GHashTable *matched_hash = g_hash_table_new_full (g_str_hash, g_str_equal, NULL, NULL);
+    GHashTable *unmatched_hash = g_hash_table_new_full (g_str_hash, g_str_equal, NULL, NULL);
+    GPtrArray *matched_array = g_ptr_array_new ();
+    GPtrArray *unmatched_array = g_ptr_array_new ();
+
+    /* Iterate over the messages in the thread collecting matching and
+     * non-matching authors. */
+    for (messages = notmuch_thread_get_messages (thread);
+	 notmuch_messages_valid (messages);
+	 notmuch_messages_move_to_next (messages))
+    {
+	notmuch_message_t *message = notmuch_messages_get (messages);
+	const char *author = _nice_author (thread, notmuch_message_get_header (message, "from"));
+
+	if (author) {
+	    GHashTable *hash;
+	    GPtrArray *array;
+
+	    if (notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_MATCH)) {
+		hash = matched_hash;
+		array = matched_array;
+	    } else {
+		hash = unmatched_hash;
+		array = unmatched_array;
+	    }
+
+	    if (!g_hash_table_lookup_extended (hash, author, NULL, NULL)) {
+		char *copy = talloc_strdup (thread, author);
+		g_hash_table_insert (hash, copy, NULL);
+		g_ptr_array_add (array, (char *) copy);
+	    }
+	}
+    }
+
+    /* Output the matched authors. */
+    unsigned int i;
+    format->map_key (format, "authors_matched");
+    format->begin_list (format);
+    for (i = 0; i < matched_array->len; i++)
+	format->string (format, (char *) g_ptr_array_index( matched_array, i));
+    format->end (format);
+
+    /* Output the non-matched authors, but not if they were seen
+     * already in the matched authors list. */
+    format->map_key (format, "authors_non_matched");
+    format->begin_list (format);
+    for (i = 0; i < unmatched_array->len; i++) {
+	char *author = (char *) g_ptr_array_index( unmatched_array, i);
+
+	if (!g_hash_table_lookup_extended (matched_hash, author, NULL, NULL))
+	    format->string (format, author);
+    }
+    format->end (format);
+
+    g_hash_table_unref (matched_hash);
+    g_hash_table_unref (unmatched_hash);
+
+    g_ptr_array_free (matched_array, TRUE);
+    g_ptr_array_free (unmatched_array, TRUE);
+
+    return 0;
+}
+
 static int
 do_search_threads (sprinter_t *format,
 		   notmuch_query_t *query,
@@ -152,6 +253,10 @@ do_search_threads (sprinter_t *format,
 		format->integer (format, total);
 		format->map_key (format, "authors");
 		format->string (format, authors);
+		if (_enumerate_authors (format, thread) < 0) {
+		    fprintf (stderr, "Out of memory\n");
+		    return 1;
+		}
 		format->map_key (format, "subject");
 		format->string (format, subject);
 		if (notmuch_format_version >= 2) {
-- 
2.1.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 2/3] emacs: Improved display of matching/non-matching authors.
  2014-10-24 17:44 [PATCH v2 0/3] Improve the display of matching/non-matching authors David Edmondson
  2014-10-24 17:44 ` [PATCH v2 1/3] search: Separately report matching and non-matching authors David Edmondson
@ 2014-10-24 17:44 ` David Edmondson
  2015-01-18 14:27   ` David Bremner
  2015-01-18 18:10   ` Mark Walters
  2014-10-24 17:44 ` [PATCH v2 3/3] test: Update tests for 'authors_matched' and authors_non_matched' David Edmondson
  2 siblings, 2 replies; 11+ messages in thread
From: David Edmondson @ 2014-10-24 17:44 UTC (permalink / raw)
  To: notmuch

Rather than splitting the :authors attribute, which is error prone,
use the separate :authors_matched and :authors_non_matched attributes.

This improves the display of authors should one of them include a pipe
symbol (|) in their 'from' address.
---
 emacs/notmuch.el | 64 +++++++++++++++++++++++++++++++-------------------------
 1 file changed, 35 insertions(+), 29 deletions(-)

diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index b44a907..688b37c 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -672,22 +672,24 @@ foreground and blue background."
 	;; Reverse the list so earlier entries take precedence
 	(reverse notmuch-search-line-faces)))
 
-(defun notmuch-search-author-propertize (authors)
+(defun notmuch-search-author-propertize (authors matching-length)
   "Split `authors' into matching and non-matching authors and
 propertize appropriately. If no boundary between authors and
 non-authors is found, assume that all of the authors match."
-  (if (string-match "\\(.*\\)|\\(.*\\)" authors)
-      (concat (propertize (concat (match-string 1 authors) ",")
-			  'face 'notmuch-search-matching-authors)
-	      (propertize (match-string 2 authors)
-			  'face 'notmuch-search-non-matching-authors))
-    (propertize authors 'face 'notmuch-search-matching-authors)))
-
-(defun notmuch-search-insert-authors (format-string authors)
+  (let ((match-part (substring authors 0 matching-length))
+	(non-match-part (substring authors matching-length)))
+
+      (concat (propertize match-part 'face 'notmuch-search-matching-authors)
+	      (propertize non-match-part 'face 'notmuch-search-non-matching-authors))))
+
+(defun notmuch-search-insert-authors (format-string matching-authors non-matching-authors)
   ;; Save the match data to avoid interfering with
   ;; `notmuch-search-process-filter'.
   (save-match-data
-    (let* ((formatted-authors (format format-string authors))
+    (let* ((authors (if (string= "" non-matching-authors)
+			matching-authors
+		      (concat matching-authors ", " non-matching-authors)))
+	   (formatted-authors (format format-string authors))
 	   (formatted-sample (format format-string ""))
 	   (visible-string formatted-authors)
 	   (invisible-string "")
@@ -703,9 +705,9 @@ non-authors is found, assume that all of the authors match."
 	    (setq visible-string (substring formatted-authors 0 visible-length)
 		  invisible-string (substring formatted-authors visible-length))
 	    ;; If possible, truncate the visible string at a natural
-	    ;; break (comma or pipe), as incremental search doesn't
-	    ;; match across the visible/invisible border.
-	    (when (string-match "\\(.*\\)\\([,|] \\)\\([^,|]*\\)" visible-string)
+	    ;; break (comma), as incremental search doesn't match
+	    ;; across the visible/invisible border.
+	    (when (string-match "\\(.*\\)\\(, \\)\\([^,]*\\)" visible-string)
 	      ;; Second clause is destructive on `visible-string', so
 	      ;; order is important.
 	      (setq invisible-string (concat (match-string 3 visible-string)
@@ -721,20 +723,23 @@ non-authors is found, assume that all of the authors match."
 				       ? ))))
 
       ;; Use different faces to show matching and non-matching authors.
-      (if (string-match "\\(.*\\)|\\(.*\\)" visible-string)
-	  ;; The visible string contains both matching and
-	  ;; non-matching authors.
-	  (setq visible-string (notmuch-search-author-propertize visible-string)
-		;; The invisible string must contain only non-matching
-		;; authors, as the visible-string contains both.
-		invisible-string (propertize invisible-string
-					     'face 'notmuch-search-non-matching-authors))
-	;; The visible string contains only matching authors.
-	(setq visible-string (propertize visible-string
-					 'face 'notmuch-search-matching-authors)
-	      ;; The invisible string may contain both matching and
-	      ;; non-matching authors.
-	      invisible-string (notmuch-search-author-propertize invisible-string)))
+      (let ((visible-length (length visible-string))
+	    (matching-length (length matching-authors)))
+
+	(if (> visible-length matching-length)
+	    ;; The visible string contains both matching and
+	    ;; non-matching authors.
+	    (setq visible-string (notmuch-search-author-propertize visible-string matching-length)
+		  ;; The invisible string must contain only non-matching
+		  ;; authors, as the visible-string contains both.
+		  invisible-string (propertize invisible-string
+					       'face 'notmuch-search-non-matching-authors))
+	  ;; The visible string contains only matching authors.
+	  (setq visible-string (propertize visible-string
+					   'face 'notmuch-search-matching-authors)
+		;; The invisible string may contain both matching and
+		;; non-matching authors.
+		invisible-string (notmuch-search-author-propertize invisible-string (- visible-length matching-length)))))
 
       ;; If there is any invisible text, add it as a tooltip to the
       ;; visible text.
@@ -768,8 +773,9 @@ non-authors is found, assume that all of the authors match."
 			'face 'notmuch-search-subject)))
 
    ((string-equal field "authors")
-    (notmuch-search-insert-authors
-     format-string (notmuch-sanitize (plist-get result :authors))))
+    (notmuch-search-insert-authors format-string
+				   (notmuch-sanitize (mapconcat 'identity (plist-get result :authors_matched) ", "))
+				   (notmuch-sanitize (mapconcat 'identity (plist-get result :authors_non_matched) ", "))))
 
    ((string-equal field "tags")
     (let ((tags (plist-get result :tags))
-- 
2.1.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 3/3] test: Update tests for 'authors_matched' and authors_non_matched'.
  2014-10-24 17:44 [PATCH v2 0/3] Improve the display of matching/non-matching authors David Edmondson
  2014-10-24 17:44 ` [PATCH v2 1/3] search: Separately report matching and non-matching authors David Edmondson
  2014-10-24 17:44 ` [PATCH v2 2/3] emacs: Improved display of matching/non-matching authors David Edmondson
@ 2014-10-24 17:44 ` David Edmondson
  2015-01-18 18:12   ` Mark Walters
  2 siblings, 1 reply; 11+ messages in thread
From: David Edmondson @ 2014-10-24 17:44 UTC (permalink / raw)
  To: notmuch

---
 test/T160-json.sh            | 9 +++++++++
 test/T170-sexp.sh            | 4 ++--
 test/T470-missing-headers.sh | 8 ++++++++
 3 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/test/T160-json.sh b/test/T160-json.sh
index c1cf649..0a8df18 100755
--- a/test/T160-json.sh
+++ b/test/T160-json.sh
@@ -25,6 +25,10 @@ test_expect_equal_json "$output" "[{\"thread\": \"XXX\",
  \"matched\": 1,
  \"total\": 1,
  \"authors\": \"Notmuch Test Suite\",
+ \"authors_matched\": [
+     \"Notmuch Test Suite\"
+ ],
+ \"authors_non_matched\": [],
  \"subject\": \"json-search-subject\",
  \"query\": [\"id:$gen_msg_id\", null],
  \"tags\": [\"inbox\",
@@ -59,6 +63,11 @@ test_expect_equal_json "$output" "[{\"thread\": \"XXX\",
  \"matched\": 1,
  \"total\": 1,
  \"authors\": \"Notmuch Test Suite\",
+ \"authors\": \"Notmuch Test Suite\",
+ \"authors_matched\": [
+     \"Notmuch Test Suite\"
+ ],
+ \"authors_non_matched\": [],
  \"subject\": \"json-search-utf8-body-sübjéct\",
  \"query\": [\"id:$gen_msg_id\", null],
  \"tags\": [\"inbox\",
diff --git a/test/T170-sexp.sh b/test/T170-sexp.sh
index 667e319..f2a08bf 100755
--- a/test/T170-sexp.sh
+++ b/test/T170-sexp.sh
@@ -19,7 +19,7 @@ test_expect_equal "$output" "((((:id \"${gen_msg_id}\" :match t :excluded nil :f
 test_begin_subtest "Search message: sexp"
 add_message "[subject]=\"sexp-search-subject\"" "[date]=\"Sat, 01 Jan 2000 12:00:00 -0000\"" "[body]=\"sexp-search-message\""
 output=$(notmuch search --format=sexp "sexp-search-message" | notmuch_search_sanitize)
-test_expect_equal "$output" "((:thread \"0000000000000002\" :timestamp 946728000 :date_relative \"2000-01-01\" :matched 1 :total 1 :authors \"Notmuch Test Suite\" :subject \"sexp-search-subject\" :query (\"id:$gen_msg_id\" nil) :tags (\"inbox\" \"unread\")))"
+test_expect_equal "$output" "((:thread \"0000000000000002\" :timestamp 946728000 :date_relative \"2000-01-01\" :matched 1 :total 1 :authors \"Notmuch Test Suite\" :authors_matched (\"Notmuch Test Suite\") :authors_non_matched () :subject \"sexp-search-subject\" :query (\"id:$gen_msg_id\" nil) :tags (\"inbox\" \"unread\")))"
 
 test_begin_subtest "Show message: sexp, utf-8"
 add_message "[subject]=\"sexp-show-utf8-body-sübjéct\"" "[date]=\"Sat, 01 Jan 2000 12:00:00 -0000\"" "[body]=\"jsön-show-méssage\""
@@ -44,7 +44,7 @@ test_expect_equal "$output" "((((:id \"$id\" :match t :excluded nil :filename \"
 test_begin_subtest "Search message: sexp, utf-8"
 add_message "[subject]=\"sexp-search-utf8-body-sübjéct\"" "[date]=\"Sat, 01 Jan 2000 12:00:00 -0000\"" "[body]=\"jsön-search-méssage\""
 output=$(notmuch search --format=sexp "jsön-search-méssage" | notmuch_search_sanitize)
-test_expect_equal "$output" "((:thread \"0000000000000005\" :timestamp 946728000 :date_relative \"2000-01-01\" :matched 1 :total 1 :authors \"Notmuch Test Suite\" :subject \"sexp-search-utf8-body-sübjéct\" :query (\"id:$gen_msg_id\" nil) :tags (\"inbox\" \"unread\")))"
+test_expect_equal "$output" "((:thread \"0000000000000005\" :timestamp 946728000 :date_relative \"2000-01-01\" :matched 1 :total 1 :authors \"Notmuch Test Suite\" :authors_matched (\"Notmuch Test Suite\") :authors_non_matched () :subject \"sexp-search-utf8-body-sübjéct\" :query (\"id:$gen_msg_id\" nil) :tags (\"inbox\" \"unread\")))"
 
 
 test_done
diff --git a/test/T470-missing-headers.sh b/test/T470-missing-headers.sh
index cb38301..250a370 100755
--- a/test/T470-missing-headers.sh
+++ b/test/T470-missing-headers.sh
@@ -34,6 +34,10 @@ test_expect_equal_json "$output" '
 [
     {
         "authors": "",
+        "authors_matched": [
+            ""
+        ],
+        "authors_non_matched": [],
         "date_relative": "2001-01-05",
         "matched": 1,
         "subject": "",
@@ -48,6 +52,10 @@ test_expect_equal_json "$output" '
     },
     {
         "authors": "Notmuch Test Suite",
+        "authors_matched": [
+            "Notmuch Test Suite"
+        ],
+        "authors_non_matched": [],
         "date_relative": "1970-01-01",
         "matched": 1,
         "subject": "",
-- 
2.1.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/3] search: Separately report matching and non-matching authors.
  2014-10-24 17:44 ` [PATCH v2 1/3] search: Separately report matching and non-matching authors David Edmondson
@ 2015-01-18 14:19   ` David Bremner
  2015-01-18 17:59   ` Mark Walters
  1 sibling, 0 replies; 11+ messages in thread
From: David Bremner @ 2015-01-18 14:19 UTC (permalink / raw)
  To: David Edmondson, notmuch

David Edmondson <dme@dme.org> writes:

> In addition to the 'authors' attribute of each search result, include
> 'authors_matched' and 'authors_non_matched' attributes. Both
> attributes are always included and are formatted as a list of
> authors. If there are no matching authors, the 'authors_non_matched'
> attribute is set to the empty list.
> ---

It would be nicer if the tests were repaired in the same commit, or
minimally marked broken.

>  static int
>  do_search_threads (sprinter_t *format,
>  		   notmuch_query_t *query,
> @@ -152,6 +253,10 @@ do_search_threads (sprinter_t *format,
>  		format->integer (format, total);
>  		format->map_key (format, "authors");
>  		format->string (format, authors);
> +		if (_enumerate_authors (format, thread) < 0) {
> +		    fprintf (stderr, "Out of memory\n");
> +		    return 1;
> +		}

Maybe I'm just blind, but I don't see how enumerate authors ever returns
anything other than 0. 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 2/3] emacs: Improved display of matching/non-matching authors.
  2014-10-24 17:44 ` [PATCH v2 2/3] emacs: Improved display of matching/non-matching authors David Edmondson
@ 2015-01-18 14:27   ` David Bremner
  2015-01-18 18:10   ` Mark Walters
  1 sibling, 0 replies; 11+ messages in thread
From: David Bremner @ 2015-01-18 14:27 UTC (permalink / raw)
  To: David Edmondson, notmuch

David Edmondson <dme@dme.org> writes:

> +(defun notmuch-search-author-propertize (authors matching-length)
>    "Split `authors' into matching and non-matching authors and
>  propertize appropriately. If no boundary between authors and
>  non-authors is found, assume that all of the authors match."

The docstring could maybe be updated.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/3] search: Separately report matching and non-matching authors.
  2014-10-24 17:44 ` [PATCH v2 1/3] search: Separately report matching and non-matching authors David Edmondson
  2015-01-18 14:19   ` David Bremner
@ 2015-01-18 17:59   ` Mark Walters
  2015-01-19  9:14     ` David Edmondson
  1 sibling, 1 reply; 11+ messages in thread
From: Mark Walters @ 2015-01-18 17:59 UTC (permalink / raw)
  To: David Edmondson, notmuch

On Fri, 24 Oct 2014, David Edmondson <dme@dme.org> wrote:
> In addition to the 'authors' attribute of each search result, include
> 'authors_matched' and 'authors_non_matched' attributes. Both
> attributes are always included and are formatted as a list of
> authors. If there are no matching authors, the 'authors_non_matched'
> attribute is set to the empty list.

Hi

Sorry to be so slow reviewing this. Would it be possible to do the
matching/non-matching stuff in lib/thread.cc and just call that from
notmuch-search.c? I guess you would need to add a matched_authors, and
unmatched_authors string to the notmuch_thread struct.

Doing this in search.c seems to redo things that the thread code is
already doing but maybe I don't really know this code.

Best wishes

Mark

> ---
>  notmuch-search.c | 105 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 105 insertions(+)
>
> diff --git a/notmuch-search.c b/notmuch-search.c
> index bc9be45..18c3b20 100644
> --- a/notmuch-search.c
> +++ b/notmuch-search.c
> @@ -22,6 +22,8 @@
>  #include "sprinter.h"
>  #include "string-util.h"
>  
> +#include <glib.h>
> +
>  typedef enum {
>      OUTPUT_SUMMARY,
>      OUTPUT_THREADS,
> @@ -69,6 +71,105 @@ get_thread_query (notmuch_thread_t *thread,
>      return 0;
>  }
>  
> +/* Return a more pleasent rendering of the mail address
> + * `nasty_author'. */
> +static const char *
> +_nice_author (void *ctx, const char *nasty_author)
> +{
> +    const char *nice_author = NULL;
> +
> +    InternetAddressList *list = internet_address_list_parse_string (nasty_author);
> +    if (list) {
> +	InternetAddress *address = internet_address_list_get_address (list, 0);
> +	if (address) {
> +	    nice_author = internet_address_get_name (address);
> +	    if (nice_author == NULL) {
> +		InternetAddressMailbox *mailbox = INTERNET_ADDRESS_MAILBOX (address);
> +		nice_author = internet_address_mailbox_get_addr (mailbox);
> +	    }
> +	}
> +	/* Duplicate the string before `g_object_unref' destroys
> +	 * it. */
> +	if (nice_author)
> +	    nice_author = talloc_strdup (ctx, nice_author);
> +
> +	g_object_unref (G_OBJECT (list));
> +    }
> +
> +    if (nice_author)
> +	return nice_author;
> +    else
> +	return nasty_author;
> +}
> +
> +static int
> +_enumerate_authors (sprinter_t *format,
> +		 notmuch_thread_t *thread)
> +{
> +    notmuch_messages_t *messages;
> +    GHashTable *matched_hash = g_hash_table_new_full (g_str_hash, g_str_equal, NULL, NULL);
> +    GHashTable *unmatched_hash = g_hash_table_new_full (g_str_hash, g_str_equal, NULL, NULL);
> +    GPtrArray *matched_array = g_ptr_array_new ();
> +    GPtrArray *unmatched_array = g_ptr_array_new ();
> +
> +    /* Iterate over the messages in the thread collecting matching and
> +     * non-matching authors. */
> +    for (messages = notmuch_thread_get_messages (thread);
> +	 notmuch_messages_valid (messages);
> +	 notmuch_messages_move_to_next (messages))
> +    {
> +	notmuch_message_t *message = notmuch_messages_get (messages);
> +	const char *author = _nice_author (thread, notmuch_message_get_header (message, "from"));
> +
> +	if (author) {
> +	    GHashTable *hash;
> +	    GPtrArray *array;
> +
> +	    if (notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_MATCH)) {
> +		hash = matched_hash;
> +		array = matched_array;
> +	    } else {
> +		hash = unmatched_hash;
> +		array = unmatched_array;
> +	    }
> +
> +	    if (!g_hash_table_lookup_extended (hash, author, NULL, NULL)) {
> +		char *copy = talloc_strdup (thread, author);
> +		g_hash_table_insert (hash, copy, NULL);
> +		g_ptr_array_add (array, (char *) copy);
> +	    }
> +	}
> +    }
> +
> +    /* Output the matched authors. */
> +    unsigned int i;
> +    format->map_key (format, "authors_matched");
> +    format->begin_list (format);
> +    for (i = 0; i < matched_array->len; i++)
> +	format->string (format, (char *) g_ptr_array_index( matched_array, i));
> +    format->end (format);
> +
> +    /* Output the non-matched authors, but not if they were seen
> +     * already in the matched authors list. */
> +    format->map_key (format, "authors_non_matched");
> +    format->begin_list (format);
> +    for (i = 0; i < unmatched_array->len; i++) {
> +	char *author = (char *) g_ptr_array_index( unmatched_array, i);
> +
> +	if (!g_hash_table_lookup_extended (matched_hash, author, NULL, NULL))
> +	    format->string (format, author);
> +    }
> +    format->end (format);
> +
> +    g_hash_table_unref (matched_hash);
> +    g_hash_table_unref (unmatched_hash);
> +
> +    g_ptr_array_free (matched_array, TRUE);
> +    g_ptr_array_free (unmatched_array, TRUE);
> +
> +    return 0;
> +}
> +
>  static int
>  do_search_threads (sprinter_t *format,
>  		   notmuch_query_t *query,
> @@ -152,6 +253,10 @@ do_search_threads (sprinter_t *format,
>  		format->integer (format, total);
>  		format->map_key (format, "authors");
>  		format->string (format, authors);
> +		if (_enumerate_authors (format, thread) < 0) {
> +		    fprintf (stderr, "Out of memory\n");
> +		    return 1;
> +		}
>  		format->map_key (format, "subject");
>  		format->string (format, subject);
>  		if (notmuch_format_version >= 2) {
> -- 
> 2.1.1
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 2/3] emacs: Improved display of matching/non-matching authors.
  2014-10-24 17:44 ` [PATCH v2 2/3] emacs: Improved display of matching/non-matching authors David Edmondson
  2015-01-18 14:27   ` David Bremner
@ 2015-01-18 18:10   ` Mark Walters
  2015-01-19  9:18     ` David Edmondson
  1 sibling, 1 reply; 11+ messages in thread
From: Mark Walters @ 2015-01-18 18:10 UTC (permalink / raw)
  To: David Edmondson, notmuch

On Fri, 24 Oct 2014, David Edmondson <dme@dme.org> wrote:
> Rather than splitting the :authors attribute, which is error prone,
> use the separate :authors_matched and :authors_non_matched attributes.
>
> This improves the display of authors should one of them include a pipe
> symbol (|) in their 'from' address.

Hi

I haven't fully understood this code yet: do you need to join the
matching and non-matching authors and then split again for propertizing?
It feels like it would be nicer to keep them separate and only combine
at the very end.

Does this all get nicer with your formatting search results series?

Best wishes

Mark


> ---
>  emacs/notmuch.el | 64 +++++++++++++++++++++++++++++++-------------------------
>  1 file changed, 35 insertions(+), 29 deletions(-)
>
> diff --git a/emacs/notmuch.el b/emacs/notmuch.el
> index b44a907..688b37c 100644
> --- a/emacs/notmuch.el
> +++ b/emacs/notmuch.el
> @@ -672,22 +672,24 @@ foreground and blue background."
>  	;; Reverse the list so earlier entries take precedence
>  	(reverse notmuch-search-line-faces)))
>  
> -(defun notmuch-search-author-propertize (authors)
> +(defun notmuch-search-author-propertize (authors matching-length)
>    "Split `authors' into matching and non-matching authors and
>  propertize appropriately. If no boundary between authors and
>  non-authors is found, assume that all of the authors match."
> -  (if (string-match "\\(.*\\)|\\(.*\\)" authors)
> -      (concat (propertize (concat (match-string 1 authors) ",")
> -			  'face 'notmuch-search-matching-authors)
> -	      (propertize (match-string 2 authors)
> -			  'face 'notmuch-search-non-matching-authors))
> -    (propertize authors 'face 'notmuch-search-matching-authors)))
> -
> -(defun notmuch-search-insert-authors (format-string authors)
> +  (let ((match-part (substring authors 0 matching-length))
> +	(non-match-part (substring authors matching-length)))
> +
> +      (concat (propertize match-part 'face 'notmuch-search-matching-authors)
> +	      (propertize non-match-part 'face 'notmuch-search-non-matching-authors))))
> +
> +(defun notmuch-search-insert-authors (format-string matching-authors non-matching-authors)
>    ;; Save the match data to avoid interfering with
>    ;; `notmuch-search-process-filter'.
>    (save-match-data
> -    (let* ((formatted-authors (format format-string authors))
> +    (let* ((authors (if (string= "" non-matching-authors)
> +			matching-authors
> +		      (concat matching-authors ", " non-matching-authors)))
> +	   (formatted-authors (format format-string authors))
>  	   (formatted-sample (format format-string ""))
>  	   (visible-string formatted-authors)
>  	   (invisible-string "")
> @@ -703,9 +705,9 @@ non-authors is found, assume that all of the authors match."
>  	    (setq visible-string (substring formatted-authors 0 visible-length)
>  		  invisible-string (substring formatted-authors visible-length))
>  	    ;; If possible, truncate the visible string at a natural
> -	    ;; break (comma or pipe), as incremental search doesn't
> -	    ;; match across the visible/invisible border.
> -	    (when (string-match "\\(.*\\)\\([,|] \\)\\([^,|]*\\)" visible-string)
> +	    ;; break (comma), as incremental search doesn't match
> +	    ;; across the visible/invisible border.
> +	    (when (string-match "\\(.*\\)\\(, \\)\\([^,]*\\)" visible-string)
>  	      ;; Second clause is destructive on `visible-string', so
>  	      ;; order is important.
>  	      (setq invisible-string (concat (match-string 3 visible-string)
> @@ -721,20 +723,23 @@ non-authors is found, assume that all of the authors match."
>  				       ? ))))
>  
>        ;; Use different faces to show matching and non-matching authors.
> -      (if (string-match "\\(.*\\)|\\(.*\\)" visible-string)
> -	  ;; The visible string contains both matching and
> -	  ;; non-matching authors.
> -	  (setq visible-string (notmuch-search-author-propertize visible-string)
> -		;; The invisible string must contain only non-matching
> -		;; authors, as the visible-string contains both.
> -		invisible-string (propertize invisible-string
> -					     'face 'notmuch-search-non-matching-authors))
> -	;; The visible string contains only matching authors.
> -	(setq visible-string (propertize visible-string
> -					 'face 'notmuch-search-matching-authors)
> -	      ;; The invisible string may contain both matching and
> -	      ;; non-matching authors.
> -	      invisible-string (notmuch-search-author-propertize invisible-string)))
> +      (let ((visible-length (length visible-string))
> +	    (matching-length (length matching-authors)))
> +
> +	(if (> visible-length matching-length)
> +	    ;; The visible string contains both matching and
> +	    ;; non-matching authors.
> +	    (setq visible-string (notmuch-search-author-propertize visible-string matching-length)
> +		  ;; The invisible string must contain only non-matching
> +		  ;; authors, as the visible-string contains both.
> +		  invisible-string (propertize invisible-string
> +					       'face 'notmuch-search-non-matching-authors))
> +	  ;; The visible string contains only matching authors.
> +	  (setq visible-string (propertize visible-string
> +					   'face 'notmuch-search-matching-authors)
> +		;; The invisible string may contain both matching and
> +		;; non-matching authors.
> +		invisible-string (notmuch-search-author-propertize invisible-string (- visible-length matching-length)))))
>  
>        ;; If there is any invisible text, add it as a tooltip to the
>        ;; visible text.
> @@ -768,8 +773,9 @@ non-authors is found, assume that all of the authors match."
>  			'face 'notmuch-search-subject)))
>  
>     ((string-equal field "authors")
> -    (notmuch-search-insert-authors
> -     format-string (notmuch-sanitize (plist-get result :authors))))
> +    (notmuch-search-insert-authors format-string
> +				   (notmuch-sanitize (mapconcat 'identity (plist-get result :authors_matched) ", "))
> +				   (notmuch-sanitize (mapconcat 'identity (plist-get result :authors_non_matched) ", "))))
>  
>     ((string-equal field "tags")
>      (let ((tags (plist-get result :tags))
> -- 
> 2.1.1
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 3/3] test: Update tests for 'authors_matched' and authors_non_matched'.
  2014-10-24 17:44 ` [PATCH v2 3/3] test: Update tests for 'authors_matched' and authors_non_matched' David Edmondson
@ 2015-01-18 18:12   ` Mark Walters
  0 siblings, 0 replies; 11+ messages in thread
From: Mark Walters @ 2015-01-18 18:12 UTC (permalink / raw)
  To: David Edmondson, notmuch


Hi

Only one small comment here: it would be nice to have a test that has a
non-empty authors_non_matched field.

Best wishes

Mark

On Fri, 24 Oct 2014, David Edmondson <dme@dme.org> wrote:
> ---
>  test/T160-json.sh            | 9 +++++++++
>  test/T170-sexp.sh            | 4 ++--
>  test/T470-missing-headers.sh | 8 ++++++++
>  3 files changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/test/T160-json.sh b/test/T160-json.sh
> index c1cf649..0a8df18 100755
> --- a/test/T160-json.sh
> +++ b/test/T160-json.sh
> @@ -25,6 +25,10 @@ test_expect_equal_json "$output" "[{\"thread\": \"XXX\",
>   \"matched\": 1,
>   \"total\": 1,
>   \"authors\": \"Notmuch Test Suite\",
> + \"authors_matched\": [
> +     \"Notmuch Test Suite\"
> + ],
> + \"authors_non_matched\": [],
>   \"subject\": \"json-search-subject\",
>   \"query\": [\"id:$gen_msg_id\", null],
>   \"tags\": [\"inbox\",
> @@ -59,6 +63,11 @@ test_expect_equal_json "$output" "[{\"thread\": \"XXX\",
>   \"matched\": 1,
>   \"total\": 1,
>   \"authors\": \"Notmuch Test Suite\",
> + \"authors\": \"Notmuch Test Suite\",
> + \"authors_matched\": [
> +     \"Notmuch Test Suite\"
> + ],
> + \"authors_non_matched\": [],
>   \"subject\": \"json-search-utf8-body-sübjéct\",
>   \"query\": [\"id:$gen_msg_id\", null],
>   \"tags\": [\"inbox\",
> diff --git a/test/T170-sexp.sh b/test/T170-sexp.sh
> index 667e319..f2a08bf 100755
> --- a/test/T170-sexp.sh
> +++ b/test/T170-sexp.sh
> @@ -19,7 +19,7 @@ test_expect_equal "$output" "((((:id \"${gen_msg_id}\" :match t :excluded nil :f
>  test_begin_subtest "Search message: sexp"
>  add_message "[subject]=\"sexp-search-subject\"" "[date]=\"Sat, 01 Jan 2000 12:00:00 -0000\"" "[body]=\"sexp-search-message\""
>  output=$(notmuch search --format=sexp "sexp-search-message" | notmuch_search_sanitize)
> -test_expect_equal "$output" "((:thread \"0000000000000002\" :timestamp 946728000 :date_relative \"2000-01-01\" :matched 1 :total 1 :authors \"Notmuch Test Suite\" :subject \"sexp-search-subject\" :query (\"id:$gen_msg_id\" nil) :tags (\"inbox\" \"unread\")))"
> +test_expect_equal "$output" "((:thread \"0000000000000002\" :timestamp 946728000 :date_relative \"2000-01-01\" :matched 1 :total 1 :authors \"Notmuch Test Suite\" :authors_matched (\"Notmuch Test Suite\") :authors_non_matched () :subject \"sexp-search-subject\" :query (\"id:$gen_msg_id\" nil) :tags (\"inbox\" \"unread\")))"
>  
>  test_begin_subtest "Show message: sexp, utf-8"
>  add_message "[subject]=\"sexp-show-utf8-body-sübjéct\"" "[date]=\"Sat, 01 Jan 2000 12:00:00 -0000\"" "[body]=\"jsön-show-méssage\""
> @@ -44,7 +44,7 @@ test_expect_equal "$output" "((((:id \"$id\" :match t :excluded nil :filename \"
>  test_begin_subtest "Search message: sexp, utf-8"
>  add_message "[subject]=\"sexp-search-utf8-body-sübjéct\"" "[date]=\"Sat, 01 Jan 2000 12:00:00 -0000\"" "[body]=\"jsön-search-méssage\""
>  output=$(notmuch search --format=sexp "jsön-search-méssage" | notmuch_search_sanitize)
> -test_expect_equal "$output" "((:thread \"0000000000000005\" :timestamp 946728000 :date_relative \"2000-01-01\" :matched 1 :total 1 :authors \"Notmuch Test Suite\" :subject \"sexp-search-utf8-body-sübjéct\" :query (\"id:$gen_msg_id\" nil) :tags (\"inbox\" \"unread\")))"
> +test_expect_equal "$output" "((:thread \"0000000000000005\" :timestamp 946728000 :date_relative \"2000-01-01\" :matched 1 :total 1 :authors \"Notmuch Test Suite\" :authors_matched (\"Notmuch Test Suite\") :authors_non_matched () :subject \"sexp-search-utf8-body-sübjéct\" :query (\"id:$gen_msg_id\" nil) :tags (\"inbox\" \"unread\")))"
>  
>  
>  test_done
> diff --git a/test/T470-missing-headers.sh b/test/T470-missing-headers.sh
> index cb38301..250a370 100755
> --- a/test/T470-missing-headers.sh
> +++ b/test/T470-missing-headers.sh
> @@ -34,6 +34,10 @@ test_expect_equal_json "$output" '
>  [
>      {
>          "authors": "",
> +        "authors_matched": [
> +            ""
> +        ],
> +        "authors_non_matched": [],
>          "date_relative": "2001-01-05",
>          "matched": 1,
>          "subject": "",
> @@ -48,6 +52,10 @@ test_expect_equal_json "$output" '
>      },
>      {
>          "authors": "Notmuch Test Suite",
> +        "authors_matched": [
> +            "Notmuch Test Suite"
> +        ],
> +        "authors_non_matched": [],
>          "date_relative": "1970-01-01",
>          "matched": 1,
>          "subject": "",
> -- 
> 2.1.1
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/3] search: Separately report matching and non-matching authors.
  2015-01-18 17:59   ` Mark Walters
@ 2015-01-19  9:14     ` David Edmondson
  0 siblings, 0 replies; 11+ messages in thread
From: David Edmondson @ 2015-01-19  9:14 UTC (permalink / raw)
  To: Mark Walters, notmuch

On Sun, Jan 18 2015, Mark Walters wrote:
> On Fri, 24 Oct 2014, David Edmondson <dme@dme.org> wrote:
>> In addition to the 'authors' attribute of each search result, include
>> 'authors_matched' and 'authors_non_matched' attributes. Both
>> attributes are always included and are formatted as a list of
>> authors. If there are no matching authors, the 'authors_non_matched'
>> attribute is set to the empty list.
>
> Hi
>
> Sorry to be so slow reviewing this. Would it be possible to do the
> matching/non-matching stuff in lib/thread.cc and just call that from
> notmuch-search.c? I guess you would need to add a matched_authors, and
> unmatched_authors string to the notmuch_thread struct.
>
> Doing this in search.c seems to redo things that the thread code is
> already doing but maybe I don't really know this code.

Is that different to what I did originally? Austin suggested the
approach in this version (id:20141024124016.GG7970@csail.mit.edu),
unless I misunderstood him.

> Best wishes
>
> Mark
>
>> ---
>>  notmuch-search.c | 105 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 105 insertions(+)
>>
>> diff --git a/notmuch-search.c b/notmuch-search.c
>> index bc9be45..18c3b20 100644
>> --- a/notmuch-search.c
>> +++ b/notmuch-search.c
>> @@ -22,6 +22,8 @@
>>  #include "sprinter.h"
>>  #include "string-util.h"
>>  
>> +#include <glib.h>
>> +
>>  typedef enum {
>>      OUTPUT_SUMMARY,
>>      OUTPUT_THREADS,
>> @@ -69,6 +71,105 @@ get_thread_query (notmuch_thread_t *thread,
>>      return 0;
>>  }
>>  
>> +/* Return a more pleasent rendering of the mail address
>> + * `nasty_author'. */
>> +static const char *
>> +_nice_author (void *ctx, const char *nasty_author)
>> +{
>> +    const char *nice_author = NULL;
>> +
>> +    InternetAddressList *list = internet_address_list_parse_string (nasty_author);
>> +    if (list) {
>> +	InternetAddress *address = internet_address_list_get_address (list, 0);
>> +	if (address) {
>> +	    nice_author = internet_address_get_name (address);
>> +	    if (nice_author == NULL) {
>> +		InternetAddressMailbox *mailbox = INTERNET_ADDRESS_MAILBOX (address);
>> +		nice_author = internet_address_mailbox_get_addr (mailbox);
>> +	    }
>> +	}
>> +	/* Duplicate the string before `g_object_unref' destroys
>> +	 * it. */
>> +	if (nice_author)
>> +	    nice_author = talloc_strdup (ctx, nice_author);
>> +
>> +	g_object_unref (G_OBJECT (list));
>> +    }
>> +
>> +    if (nice_author)
>> +	return nice_author;
>> +    else
>> +	return nasty_author;
>> +}
>> +
>> +static int
>> +_enumerate_authors (sprinter_t *format,
>> +		 notmuch_thread_t *thread)
>> +{
>> +    notmuch_messages_t *messages;
>> +    GHashTable *matched_hash = g_hash_table_new_full (g_str_hash, g_str_equal, NULL, NULL);
>> +    GHashTable *unmatched_hash = g_hash_table_new_full (g_str_hash, g_str_equal, NULL, NULL);
>> +    GPtrArray *matched_array = g_ptr_array_new ();
>> +    GPtrArray *unmatched_array = g_ptr_array_new ();
>> +
>> +    /* Iterate over the messages in the thread collecting matching and
>> +     * non-matching authors. */
>> +    for (messages = notmuch_thread_get_messages (thread);
>> +	 notmuch_messages_valid (messages);
>> +	 notmuch_messages_move_to_next (messages))
>> +    {
>> +	notmuch_message_t *message = notmuch_messages_get (messages);
>> +	const char *author = _nice_author (thread, notmuch_message_get_header (message, "from"));
>> +
>> +	if (author) {
>> +	    GHashTable *hash;
>> +	    GPtrArray *array;
>> +
>> +	    if (notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_MATCH)) {
>> +		hash = matched_hash;
>> +		array = matched_array;
>> +	    } else {
>> +		hash = unmatched_hash;
>> +		array = unmatched_array;
>> +	    }
>> +
>> +	    if (!g_hash_table_lookup_extended (hash, author, NULL, NULL)) {
>> +		char *copy = talloc_strdup (thread, author);
>> +		g_hash_table_insert (hash, copy, NULL);
>> +		g_ptr_array_add (array, (char *) copy);
>> +	    }
>> +	}
>> +    }
>> +
>> +    /* Output the matched authors. */
>> +    unsigned int i;
>> +    format->map_key (format, "authors_matched");
>> +    format->begin_list (format);
>> +    for (i = 0; i < matched_array->len; i++)
>> +	format->string (format, (char *) g_ptr_array_index( matched_array, i));
>> +    format->end (format);
>> +
>> +    /* Output the non-matched authors, but not if they were seen
>> +     * already in the matched authors list. */
>> +    format->map_key (format, "authors_non_matched");
>> +    format->begin_list (format);
>> +    for (i = 0; i < unmatched_array->len; i++) {
>> +	char *author = (char *) g_ptr_array_index( unmatched_array, i);
>> +
>> +	if (!g_hash_table_lookup_extended (matched_hash, author, NULL, NULL))
>> +	    format->string (format, author);
>> +    }
>> +    format->end (format);
>> +
>> +    g_hash_table_unref (matched_hash);
>> +    g_hash_table_unref (unmatched_hash);
>> +
>> +    g_ptr_array_free (matched_array, TRUE);
>> +    g_ptr_array_free (unmatched_array, TRUE);
>> +
>> +    return 0;
>> +}
>> +
>>  static int
>>  do_search_threads (sprinter_t *format,
>>  		   notmuch_query_t *query,
>> @@ -152,6 +253,10 @@ do_search_threads (sprinter_t *format,
>>  		format->integer (format, total);
>>  		format->map_key (format, "authors");
>>  		format->string (format, authors);
>> +		if (_enumerate_authors (format, thread) < 0) {
>> +		    fprintf (stderr, "Out of memory\n");
>> +		    return 1;
>> +		}
>>  		format->map_key (format, "subject");
>>  		format->string (format, subject);
>>  		if (notmuch_format_version >= 2) {
>> -- 
>> 2.1.1
>>
>> _______________________________________________
>> notmuch mailing list
>> notmuch@notmuchmail.org
>> http://notmuchmail.org/mailman/listinfo/notmuch

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 2/3] emacs: Improved display of matching/non-matching authors.
  2015-01-18 18:10   ` Mark Walters
@ 2015-01-19  9:18     ` David Edmondson
  0 siblings, 0 replies; 11+ messages in thread
From: David Edmondson @ 2015-01-19  9:18 UTC (permalink / raw)
  To: Mark Walters, notmuch

On Sun, Jan 18 2015, Mark Walters wrote:
> On Fri, 24 Oct 2014, David Edmondson <dme@dme.org> wrote:
>> Rather than splitting the :authors attribute, which is error prone,
>> use the separate :authors_matched and :authors_non_matched attributes.
>>
>> This improves the display of authors should one of them include a pipe
>> symbol (|) in their 'from' address.
>
> Hi
>
> I haven't fully understood this code yet: do you need to join the
> matching and non-matching authors and then split again for propertizing?
> It feels like it would be nicer to keep them separate and only combine
> at the very end.
>
> Does this all get nicer with your formatting search results series?

Yes. I tried re-writing this code to use the already split authors here,
but given that I was going to re-write it for the formatting changes
anyway, I gave up.

>
> Best wishes
>
> Mark
>
>
>> ---
>>  emacs/notmuch.el | 64 +++++++++++++++++++++++++++++++-------------------------
>>  1 file changed, 35 insertions(+), 29 deletions(-)
>>
>> diff --git a/emacs/notmuch.el b/emacs/notmuch.el
>> index b44a907..688b37c 100644
>> --- a/emacs/notmuch.el
>> +++ b/emacs/notmuch.el
>> @@ -672,22 +672,24 @@ foreground and blue background."
>>  	;; Reverse the list so earlier entries take precedence
>>  	(reverse notmuch-search-line-faces)))
>>  
>> -(defun notmuch-search-author-propertize (authors)
>> +(defun notmuch-search-author-propertize (authors matching-length)
>>    "Split `authors' into matching and non-matching authors and
>>  propertize appropriately. If no boundary between authors and
>>  non-authors is found, assume that all of the authors match."
>> -  (if (string-match "\\(.*\\)|\\(.*\\)" authors)
>> -      (concat (propertize (concat (match-string 1 authors) ",")
>> -			  'face 'notmuch-search-matching-authors)
>> -	      (propertize (match-string 2 authors)
>> -			  'face 'notmuch-search-non-matching-authors))
>> -    (propertize authors 'face 'notmuch-search-matching-authors)))
>> -
>> -(defun notmuch-search-insert-authors (format-string authors)
>> +  (let ((match-part (substring authors 0 matching-length))
>> +	(non-match-part (substring authors matching-length)))
>> +
>> +      (concat (propertize match-part 'face 'notmuch-search-matching-authors)
>> +	      (propertize non-match-part 'face 'notmuch-search-non-matching-authors))))
>> +
>> +(defun notmuch-search-insert-authors (format-string matching-authors non-matching-authors)
>>    ;; Save the match data to avoid interfering with
>>    ;; `notmuch-search-process-filter'.
>>    (save-match-data
>> -    (let* ((formatted-authors (format format-string authors))
>> +    (let* ((authors (if (string= "" non-matching-authors)
>> +			matching-authors
>> +		      (concat matching-authors ", " non-matching-authors)))
>> +	   (formatted-authors (format format-string authors))
>>  	   (formatted-sample (format format-string ""))
>>  	   (visible-string formatted-authors)
>>  	   (invisible-string "")
>> @@ -703,9 +705,9 @@ non-authors is found, assume that all of the authors match."
>>  	    (setq visible-string (substring formatted-authors 0 visible-length)
>>  		  invisible-string (substring formatted-authors visible-length))
>>  	    ;; If possible, truncate the visible string at a natural
>> -	    ;; break (comma or pipe), as incremental search doesn't
>> -	    ;; match across the visible/invisible border.
>> -	    (when (string-match "\\(.*\\)\\([,|] \\)\\([^,|]*\\)" visible-string)
>> +	    ;; break (comma), as incremental search doesn't match
>> +	    ;; across the visible/invisible border.
>> +	    (when (string-match "\\(.*\\)\\(, \\)\\([^,]*\\)" visible-string)
>>  	      ;; Second clause is destructive on `visible-string', so
>>  	      ;; order is important.
>>  	      (setq invisible-string (concat (match-string 3 visible-string)
>> @@ -721,20 +723,23 @@ non-authors is found, assume that all of the authors match."
>>  				       ? ))))
>>  
>>        ;; Use different faces to show matching and non-matching authors.
>> -      (if (string-match "\\(.*\\)|\\(.*\\)" visible-string)
>> -	  ;; The visible string contains both matching and
>> -	  ;; non-matching authors.
>> -	  (setq visible-string (notmuch-search-author-propertize visible-string)
>> -		;; The invisible string must contain only non-matching
>> -		;; authors, as the visible-string contains both.
>> -		invisible-string (propertize invisible-string
>> -					     'face 'notmuch-search-non-matching-authors))
>> -	;; The visible string contains only matching authors.
>> -	(setq visible-string (propertize visible-string
>> -					 'face 'notmuch-search-matching-authors)
>> -	      ;; The invisible string may contain both matching and
>> -	      ;; non-matching authors.
>> -	      invisible-string (notmuch-search-author-propertize invisible-string)))
>> +      (let ((visible-length (length visible-string))
>> +	    (matching-length (length matching-authors)))
>> +
>> +	(if (> visible-length matching-length)
>> +	    ;; The visible string contains both matching and
>> +	    ;; non-matching authors.
>> +	    (setq visible-string (notmuch-search-author-propertize visible-string matching-length)
>> +		  ;; The invisible string must contain only non-matching
>> +		  ;; authors, as the visible-string contains both.
>> +		  invisible-string (propertize invisible-string
>> +					       'face 'notmuch-search-non-matching-authors))
>> +	  ;; The visible string contains only matching authors.
>> +	  (setq visible-string (propertize visible-string
>> +					   'face 'notmuch-search-matching-authors)
>> +		;; The invisible string may contain both matching and
>> +		;; non-matching authors.
>> +		invisible-string (notmuch-search-author-propertize invisible-string (- visible-length matching-length)))))
>>  
>>        ;; If there is any invisible text, add it as a tooltip to the
>>        ;; visible text.
>> @@ -768,8 +773,9 @@ non-authors is found, assume that all of the authors match."
>>  			'face 'notmuch-search-subject)))
>>  
>>     ((string-equal field "authors")
>> -    (notmuch-search-insert-authors
>> -     format-string (notmuch-sanitize (plist-get result :authors))))
>> +    (notmuch-search-insert-authors format-string
>> +				   (notmuch-sanitize (mapconcat 'identity (plist-get result :authors_matched) ", "))
>> +				   (notmuch-sanitize (mapconcat 'identity (plist-get result :authors_non_matched) ", "))))
>>  
>>     ((string-equal field "tags")
>>      (let ((tags (plist-get result :tags))
>> -- 
>> 2.1.1
>>
>> _______________________________________________
>> notmuch mailing list
>> notmuch@notmuchmail.org
>> http://notmuchmail.org/mailman/listinfo/notmuch

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2015-01-19  9:18 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-10-24 17:44 [PATCH v2 0/3] Improve the display of matching/non-matching authors David Edmondson
2014-10-24 17:44 ` [PATCH v2 1/3] search: Separately report matching and non-matching authors David Edmondson
2015-01-18 14:19   ` David Bremner
2015-01-18 17:59   ` Mark Walters
2015-01-19  9:14     ` David Edmondson
2014-10-24 17:44 ` [PATCH v2 2/3] emacs: Improved display of matching/non-matching authors David Edmondson
2015-01-18 14:27   ` David Bremner
2015-01-18 18:10   ` Mark Walters
2015-01-19  9:18     ` David Edmondson
2014-10-24 17:44 ` [PATCH v2 3/3] test: Update tests for 'authors_matched' and authors_non_matched' David Edmondson
2015-01-18 18:12   ` Mark Walters

Code repositories for project(s) associated with this public inbox

	https://yhetil.org/notmuch.git/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).