To: Mark Walters <markwalters1009@gmail.com>, notmuch@notmuchmail.org
Subject: Re: [PATCH v2 2/3] emacs: Improved display of
	matching/non-matching	authors.
In-Reply-To: <87oapwj6xg.fsf@qmul.ac.uk>
References: <1414172643-28270-1-git-send-email-dme@dme.org>
	<1414172643-28270-3-git-send-email-dme@dme.org>
	<87oapwj6xg.fsf@qmul.ac.uk>
User-Agent: none
From: David Edmondson <dme@dme.org>
Date: Mon, 19 Jan 2015 09:18:17 +0000
Message-ID: <cund26bku1y.fsf@gargravarr.hh.sledj.net>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: list

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