From: Mark Walters <markwalters1009@gmail.com>
To: David Edmondson <dme@dme.org>, notmuch@notmuchmail.org
Subject: Re: [PATCH v2 2/3] emacs: Improved display of
	matching/non-matching	authors.
In-Reply-To: <1414172643-28270-3-git-send-email-dme@dme.org>
References: <1414172643-28270-1-git-send-email-dme@dme.org>
	<1414172643-28270-3-git-send-email-dme@dme.org>
User-Agent: Notmuch/0.18.1+86~gef5e66a (http://notmuchmail.org) Emacs/24.4.1
	(x86_64-pc-linux-gnu)
Date: Sun, 18 Jan 2015 18:10:51 +0000
Message-ID: <87oapwj6xg.fsf@qmul.ac.uk>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: list

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