unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
* [PATCH] emacs: fix notmuch-show-update-tags to support duplicate files
@ 2022-09-22 21:26 Tomi Ollila
  2022-11-05 17:45 ` David Bremner
  0 siblings, 1 reply; 9+ messages in thread
From: Tomi Ollila @ 2022-09-22 21:26 UTC (permalink / raw)
  To: notmuch; +Cc: tomi.ollila

With duplicate files, the headerlines of messages in notmuch-show
buffer contains (initially) 1/n at the end of line.

Update the regexp used to search and replace tag changes to
match the current line -- drop unnecessary capturing of the
(tags), but capture the duplicates indicator.

Update the headerline pretty much like notmuch-show-insert-headerline
does, like the changes introduced mostly in commit 5ea5a5557d9a.
---

Is this getting too complex (well, we may have other stuff with
similar complexity there ;/) ?

Is there any better solutions ?

 emacs/notmuch-show.el | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index ec998ede..0527c3a5 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -453,13 +453,20 @@ (defun notmuch-show-update-tags (tags)
   "Update the displayed tags of the current message."
   (save-excursion
     (goto-char (notmuch-show-message-top))
-    (when (re-search-forward "(\\([^()]*\\))$" (line-end-position) t)
-      (let ((inhibit-read-only t))
-	(replace-match (concat "("
-			       (notmuch-tag-format-tags
-				tags
-				(notmuch-show-get-prop :orig-tags))
-			       ")"))))))
+    (when (re-search-forward "([^()]*) *\\([^()]*\\)$" (line-end-position) t)
+      (let ((inhibit-read-only t)
+	    (tags-str (notmuch-tag-format-tags
+		       tags (notmuch-show-get-prop :orig-tags)))
+	    (txt (match-string 1)))
+	(replace-match (concat "(" tags-str ")"
+			       (and (string-lessp "" txt)
+				    (notmuch-show-spaces-n
+				     (max 0 (- (window-width)
+					       (- (match-beginning 0)
+						  (notmuch-show-message-top))
+					       (length tags-str)
+					       (length txt) 3))))
+			       txt))))))
 
 (defun notmuch-clean-address (address)
   "Try to clean a single email ADDRESS for display. Return a cons
-- 
2.30.2

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

* Re: [PATCH] emacs: fix notmuch-show-update-tags to support duplicate files
  2022-09-22 21:26 [PATCH] emacs: fix notmuch-show-update-tags to support duplicate files Tomi Ollila
@ 2022-11-05 17:45 ` David Bremner
  2022-11-06 14:08   ` Tomi Ollila
  0 siblings, 1 reply; 9+ messages in thread
From: David Bremner @ 2022-11-05 17:45 UTC (permalink / raw)
  To: Tomi Ollila, notmuch; +Cc: tomi.ollila

Tomi Ollila <tomi.ollila@iki.fi> writes:


> Is this getting too complex (well, we may have other stuff with
> similar complexity there ;/) ?
>
> Is there any better solutions ?

I want to try redrawing the whole headerline. It should be more robust
against headerline format changes, but replacing the headerline has its
own complexities. I guess once I get something working, we can compare.

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

* Re: [PATCH] emacs: fix notmuch-show-update-tags to support duplicate files
  2022-11-05 17:45 ` David Bremner
@ 2022-11-06 14:08   ` Tomi Ollila
  2022-11-11 21:48     ` [PATCH 1/3] emacs/show: use plist to pass message info to n-s-insert-headerline David Bremner
  0 siblings, 1 reply; 9+ messages in thread
From: Tomi Ollila @ 2022-11-06 14:08 UTC (permalink / raw)
  To: David Bremner, notmuch

On Sat, Nov 05 2022, David Bremner wrote:

> Tomi Ollila <tomi.ollila@iki.fi> writes:
>
>
>> Is this getting too complex (well, we may have other stuff with
>> similar complexity there ;/) ?
>>
>> Is there any better solutions ?
>
> I want to try redrawing the whole headerline. It should be more robust
> against headerline format changes, but replacing the headerline has its
> own complexities. I guess once I get something working, we can compare.

I agree -- there should be one (1) function that takes care of drawing 
the headerline. this could get ouf-of-control easier when done different
ways in places far, far away...

now that I think of it, narrowing may not bring any advantage, but function
which writes headerline to current (point) -- when doing initial drawing
nothing has to be done beforehand, in case of updating, first delete line
and then redraw...

Tomi

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

* [PATCH 1/3] emacs/show: use plist to pass message info to n-s-insert-headerline
  2022-11-06 14:08   ` Tomi Ollila
@ 2022-11-11 21:48     ` David Bremner
  2022-11-11 21:48       ` [PATCH 2/3] emacs/show: add optional orig-tags argument to n-s-i-headerline David Bremner
  2022-11-11 21:48       ` [PATCH 3/3] emacs/show: use n-s-i-headerline to update tags David Bremner
  0 siblings, 2 replies; 9+ messages in thread
From: David Bremner @ 2022-11-11 21:48 UTC (permalink / raw)
  To: Tomi Ollila, David Bremner, notmuch

This should allow calling notmuch-show-insert-headerline from other
places without duplicating the set of plist accesses.
---
 emacs/notmuch-show.el | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index ec998ede..b7a8de6a 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -530,11 +530,18 @@ Return unchanged ADDRESS if parsing fails."
 	  (plist-put msg :height height)
 	  height))))
 
-(defun notmuch-show-insert-headerline (headers date tags depth duplicate file-count)
+(defun notmuch-show-insert-headerline (msg-plist depth tags)
   "Insert a notmuch style headerline based on HEADERS for a
 message at DEPTH in the current thread."
-  (let ((start (point))
-	(from (notmuch-sanitize
+  (let* ((start (point))
+	 (headers (plist-get msg-plist :headers))
+	 (duplicate (or (plist-get msg-plist :duplicate) 0))
+	 (file-count (length (plist-get msg-plist :filename)))
+	 (date (or (and notmuch-show-relative-dates
+			(plist-get msg-plist :date_relative))
+		   (plist-get headers :Date)))
+    	 (duplicate (or (plist-get msg-plist :duplicate) 0))
+	 (from (notmuch-sanitize
 	       (notmuch-show-clean-address (plist-get headers :From)))))
     (when (string-match "\\cR" from)
       ;; If the From header has a right-to-left character add
@@ -1171,8 +1178,6 @@ is out of range."
 (defun notmuch-show-insert-msg (msg depth)
   "Insert the message MSG at depth DEPTH in the current thread."
   (let* ((headers (plist-get msg :headers))
-	 (duplicate (or (plist-get msg :duplicate) 0))
-	 (files (length (plist-get msg :filename)))
 	 ;; Indentation causes the buffer offset of the start/end
 	 ;; points to move, so we must use markers.
 	 message-start message-end
@@ -1180,11 +1185,7 @@ is out of range."
 	 headers-start headers-end
 	 (bare-subject (notmuch-show-strip-re (plist-get headers :Subject))))
     (setq message-start (point-marker))
-    (notmuch-show-insert-headerline headers
-				    (or (and notmuch-show-relative-dates
-					     (plist-get msg :date_relative))
-					(plist-get headers :Date))
-				    (plist-get msg :tags) depth duplicate files)
+    (notmuch-show-insert-headerline msg depth (plist-get msg :tags))
     (setq content-start (point-marker))
     ;; Set `headers-start' to point after the 'Subject:' header to be
     ;; compatible with the existing implementation. This just sets it
-- 
2.35.1

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

* [PATCH 2/3] emacs/show: add optional orig-tags argument to n-s-i-headerline
  2022-11-11 21:48     ` [PATCH 1/3] emacs/show: use plist to pass message info to n-s-insert-headerline David Bremner
@ 2022-11-11 21:48       ` David Bremner
  2022-11-11 21:48       ` [PATCH 3/3] emacs/show: use n-s-i-headerline to update tags David Bremner
  1 sibling, 0 replies; 9+ messages in thread
From: David Bremner @ 2022-11-11 21:48 UTC (permalink / raw)
  To: Tomi Ollila, David Bremner, notmuch

This will support use of this function in notmuch-show-update-tags.
---
 emacs/notmuch-show.el | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index b7a8de6a..5fb5ab31 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -530,7 +530,7 @@ Return unchanged ADDRESS if parsing fails."
 	  (plist-put msg :height height)
 	  height))))
 
-(defun notmuch-show-insert-headerline (msg-plist depth tags)
+(defun notmuch-show-insert-headerline (msg-plist depth tags &optional orig-tags)
   "Insert a notmuch style headerline based on HEADERS for a
 message at DEPTH in the current thread."
   (let* ((start (point))
@@ -556,7 +556,7 @@ message at DEPTH in the current thread."
 	    " ("
 	    date
 	    ") ("
-	    (notmuch-tag-format-tags tags tags)
+	    (notmuch-tag-format-tags tags (or orig-tags tags))
 	    ")")
     (insert
      (if (> file-count 1)
-- 
2.35.1

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

* [PATCH 3/3] emacs/show: use n-s-i-headerline to update tags
  2022-11-11 21:48     ` [PATCH 1/3] emacs/show: use plist to pass message info to n-s-insert-headerline David Bremner
  2022-11-11 21:48       ` [PATCH 2/3] emacs/show: add optional orig-tags argument to n-s-i-headerline David Bremner
@ 2022-11-11 21:48       ` David Bremner
  2022-11-13 18:11         ` Tomi Ollila
  1 sibling, 1 reply; 9+ messages in thread
From: David Bremner @ 2022-11-11 21:48 UTC (permalink / raw)
  To: Tomi Ollila, David Bremner, notmuch

Although this has more steps than the previous regular expression
search and replace, it should be more robust against changes in the
headerline format, such as the inclusion of duplicate numbers (which
broke the previous version).
---
 emacs/notmuch-show.el | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 5fb5ab31..c4e88be2 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -452,14 +452,20 @@ operation on the contents of the current buffer."
 (defun notmuch-show-update-tags (tags)
   "Update the displayed tags of the current message."
   (save-excursion
-    (goto-char (notmuch-show-message-top))
-    (when (re-search-forward "(\\([^()]*\\))$" (line-end-position) t)
-      (let ((inhibit-read-only t))
-	(replace-match (concat "("
-			       (notmuch-tag-format-tags
-				tags
-				(notmuch-show-get-prop :orig-tags))
-			       ")"))))))
+    (let ((inhibit-read-only t)
+	  (start (notmuch-show-message-top))
+	  (depth (notmuch-show-get-prop :depth))
+	  (orig-tags (notmuch-show-get-prop :orig-tags))
+	  (props (notmuch-show-get-message-properties))
+	  (extent (notmuch-show-message-extent)))
+      (goto-char start)
+      (notmuch-show-insert-headerline props depth tags orig-tags)
+      (put-text-property start (1+ start)
+			 :notmuch-message-properties props)
+      (put-text-property (car extent) (cdr extent) :notmuch-message-extent extent)
+      ;; delete original headerline, but do not save to kill ring
+      (delete-region
+       (point) (save-excursion (end-of-line nil) (1+ (point)))))))
 
 (defun notmuch-clean-address (address)
   "Try to clean a single email ADDRESS for display. Return a cons
-- 
2.35.1

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

* Re: [PATCH 3/3] emacs/show: use n-s-i-headerline to update tags
  2022-11-11 21:48       ` [PATCH 3/3] emacs/show: use n-s-i-headerline to update tags David Bremner
@ 2022-11-13 18:11         ` Tomi Ollila
  2022-11-15 11:59           ` David Bremner
  0 siblings, 1 reply; 9+ messages in thread
From: Tomi Ollila @ 2022-11-13 18:11 UTC (permalink / raw)
  To: David Bremner, David Bremner, notmuch

On Fri, Nov 11 2022, David Bremner wrote:

> Although this has more steps than the previous regular expression
> search and replace, it should be more robust against changes in the
> headerline format, such as the inclusion of duplicate numbers (which
> broke the previous version).
> ---
>  emacs/notmuch-show.el | 22 ++++++++++++++--------
>  1 file changed, 14 insertions(+), 8 deletions(-)
>
> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> index 5fb5ab31..c4e88be2 100644
> --- a/emacs/notmuch-show.el
> +++ b/emacs/notmuch-show.el
> @@ -452,14 +452,20 @@ operation on the contents of the current buffer."
>  (defun notmuch-show-update-tags (tags)
>    "Update the displayed tags of the current message."
>    (save-excursion
> -    (goto-char (notmuch-show-message-top))
> -    (when (re-search-forward "(\\([^()]*\\))$" (line-end-position) t)
> -      (let ((inhibit-read-only t))
> -	(replace-match (concat "("
> -			       (notmuch-tag-format-tags
> -				tags
> -				(notmuch-show-get-prop :orig-tags))
> -			       ")"))))))
> +    (let ((inhibit-read-only t)
> +	  (start (notmuch-show-message-top))
> +	  (depth (notmuch-show-get-prop :depth))
> +	  (orig-tags (notmuch-show-get-prop :orig-tags))
> +	  (props (notmuch-show-get-message-properties))
> +	  (extent (notmuch-show-message-extent)))
> +      (goto-char start)
> +      (notmuch-show-insert-headerline props depth tags orig-tags)
> +      (put-text-property start (1+ start)
> +			 :notmuch-message-properties props)
> +      (put-text-property (car extent) (cdr extent) :notmuch-message-extent extent)
> +      ;; delete original headerline, but do not save to kill ring
> +      (delete-region
> +       (point) (save-excursion (end-of-line nil) (1+ (point)))))))

Series looks good to me (as much as I understand it)

This last changed line got me checking docstring of (line-end-position),
which says:

 -- line-end-position is a built-in function in ‘C source code’.
 -- Probably introduced at or before Emacs version 20.4.
 -- This function does not change global state, including the match data.

and then grepping thru notmuch source code:

notmuch-tree.el:497:      (delete-region (point) (1+ (line-end-position)))

-- it looks to me this could be simpler and faster alternative.

Tomi

PS: something that is nasty but probably does not hit here (there is
newline always)

(delete-region (line-end-position) 1000000)
-> Debugger entered--Lisp error: (args-out-of-range #<buffer *scratch*> 215 1000000

>  
>  (defun notmuch-clean-address (address)
>    "Try to clean a single email ADDRESS for display. Return a cons
> -- 
> 2.35.1
>
> _______________________________________________
> notmuch mailing list -- notmuch@notmuchmail.org
> To unsubscribe send an email to notmuch-leave@notmuchmail.org\r

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

* Re: [PATCH 3/3] emacs/show: use n-s-i-headerline to update tags
  2022-11-13 18:11         ` Tomi Ollila
@ 2022-11-15 11:59           ` David Bremner
  2022-11-15 14:01             ` Tomi Ollila
  0 siblings, 1 reply; 9+ messages in thread
From: David Bremner @ 2022-11-15 11:59 UTC (permalink / raw)
  To: Tomi Ollila, notmuch

Tomi Ollila <tomi.ollila@iki.fi> writes:

>
> and then grepping thru notmuch source code:
>
> notmuch-tree.el:497:      (delete-region (point) (1+ (line-end-position)))
>
> -- it looks to me this could be simpler and faster alternative.
>

Applied to master with your suggested modification. Thanks for the
review.

d

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

* Re: [PATCH 3/3] emacs/show: use n-s-i-headerline to update tags
  2022-11-15 11:59           ` David Bremner
@ 2022-11-15 14:01             ` Tomi Ollila
  0 siblings, 0 replies; 9+ messages in thread
From: Tomi Ollila @ 2022-11-15 14:01 UTC (permalink / raw)
  To: David Bremner, notmuch

On Tue, Nov 15 2022, David Bremner wrote:

> Tomi Ollila <tomi.ollila@iki.fi> writes:
>
>>
>> and then grepping thru notmuch source code:
>>
>> notmuch-tree.el:497:      (delete-region (point) (1+ (line-end-position)))
>>
>> -- it looks to me this could be simpler and faster alternative.
>>
>
> Applied to master with your suggested modification. Thanks for the
> review.

Great! now it works...

(insert " " (notmuch-version)) notmuch version 0.37+27~gff8ef59

Tomi

>
> d

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

end of thread, other threads:[~2022-11-15 14:02 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-22 21:26 [PATCH] emacs: fix notmuch-show-update-tags to support duplicate files Tomi Ollila
2022-11-05 17:45 ` David Bremner
2022-11-06 14:08   ` Tomi Ollila
2022-11-11 21:48     ` [PATCH 1/3] emacs/show: use plist to pass message info to n-s-insert-headerline David Bremner
2022-11-11 21:48       ` [PATCH 2/3] emacs/show: add optional orig-tags argument to n-s-i-headerline David Bremner
2022-11-11 21:48       ` [PATCH 3/3] emacs/show: use n-s-i-headerline to update tags David Bremner
2022-11-13 18:11         ` Tomi Ollila
2022-11-15 11:59           ` David Bremner
2022-11-15 14:01             ` Tomi Ollila

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).