[RFC PATCH] gnus: Add piem-gnus-mid-to-thread

[RFC PATCH] gnus: Add piem-gnus-mid-to-thread

[Xinglu Chen]

Inserts a string of the whole thread in mbox format.

---
The function works as expected when running
'M-: (funcall (piem-gnus-mid-thread t))', but when running
piem-b4-am-from-mid interactively, it just inserts an empty string.

Any ideas of why this happens?  Sorry, my elisp skills are not that great.

 piem-gnus.el | 30 ++++++++++++++++++++++++++++--
 1 file changed, 28 insertions(+), 2 deletions(-)

diff --git a/piem-gnus.el b/piem-gnus.el
index 5f10be8..f91f91c 100644
--- a/piem-gnus.el
+++ b/piem-gnus.el
@@ -58,6 +58,30 @@
 ;; If there is an easy way to generate an mbox for a thread in Gnus, a
 ;; function for `piem-mid-to-thread-functions' should be defined.
 
+(defun piem-gnus-mid-to-thread (_mid)
+  (lambda ()
+    (save-excursion
+      ;; Cursor has to be at the root of the thread
+      (gnus-summary-refer-parent-article most-positive-fixnum)
+      (let ((articles (gnus-summary-articles-in-thread))
+	    message
+	    ;; Just show raw message
+	    (gnus-have-all-headers t)
+	    gnus-article-prepare-hook
+	    gnus-article-decode-hook
+	    gnus-display-mime-function
+	    gnus-break-pages)
+	(mapc (lambda (article)
+		(gnus-summary-display-article article)
+		(push (format "From mboxrd@z Thu Jan  1 00:00:00 1970\n%s\n"
+			      (with-current-buffer gnus-article-buffer
+				(buffer-substring-no-properties
+				 (point-min)
+				 (point-max))))
+			      message))
+		articles)
+	      (insert (apply #'concat (nreverse message)))))))
+
 (defun piem-gnus-am-ready-mbox ()
   "Return a function that inserts an am-ready mbox.
 If the buffer has any MIME parts that look like a patch, use
@@ -97,10 +121,12 @@ the mode if ARG is omitted or nil."
       (progn
         (add-hook 'piem-am-ready-mbox-functions #'piem-gnus-am-ready-mbox)
         (add-hook 'piem-get-inbox-functions #'piem-gnus-get-inbox)
-        (add-hook 'piem-get-mid-functions #'piem-gnus-get-mid))
+        (add-hook 'piem-get-mid-functions #'piem-gnus-get-mid)
+	(add-hook 'piem-mid-to-thread-functions #'piem-gnus-mid-to-thread))
     (remove-hook 'piem-am-ready-mbox-functions #'piem-gnus-am-ready-mbox)
     (remove-hook 'piem-get-inbox-functions #'piem-gnus-get-inbox)
-    (remove-hook 'piem-get-mid-functions #'piem-gnus-get-mid)))
+    (remove-hook 'piem-get-mid-functions #'piem-gnus-get-mid)
+    (remove-hook 'piem-mid-to-thread-functions #'piem-gnus-mid-to-thread)))
 
 ;;; piem-gnus.el ends here
 (provide 'piem-gnus)

base-commit: 57f802b2a43ac64c28a5a9ddb9da0afaf910975e
-- 
2.29.2

Re: [RFC PATCH] gnus: Add piem-gnus-mid-to-thread

[Kyle Meyer]

Xinglu Chen writes:

> Inserts a string of the whole thread in mbox format.

Great, thanks for looking into this.

> ---
> The function works as expected when running
> 'M-: (funcall (piem-gnus-mid-thread t))', but when running
> piem-b4-am-from-mid interactively, it just inserts an empty string.
>
> Any ideas of why this happens?  Sorry, my elisp skills are not that great.

Yes, I think you just need to move the lambda deeper so that the
messages are collected outside of the returned function.

> diff --git a/piem-gnus.el b/piem-gnus.el
> index 5f10be8..f91f91c 100644
> --- a/piem-gnus.el
> +++ b/piem-gnus.el
> @@ -58,6 +58,30 @@
>  ;; If there is an easy way to generate an mbox for a thread in Gnus, a
>  ;; function for `piem-mid-to-thread-functions' should be defined.
>  
> +(defun piem-gnus-mid-to-thread (_mid)

Hmm, yeah, I don't think we can convince Gnus to do much useful with the
message ID here.  However, perhaps there should at least be a check that
the given message ID matches the one for the article at point before
returning a function (i.e. claiming piem-gnus-mid-to-thread can generate
a thread for the given MID).

Also, I think this would need to be guarded by

  (derived-mode-p 'gnus-summary-mode)

or perhaps

  (derived-mode-p 'gnus-article-mode 'gnus-summary-mode)

to make sure we don't start doing this collection in buffers where it
won't work.

> +  (lambda ()

This is the lambda I was referring to in the beginning of the message.
It should be moved to deeper so that it contains only (insert ...).

> +    (save-excursion
> +      ;; Cursor has to be at the root of the thread
> +      (gnus-summary-refer-parent-article most-positive-fixnum)
> +      (let ((articles (gnus-summary-articles-in-thread))
> +	    message
> +	    ;; Just show raw message
> +	    (gnus-have-all-headers t)
> +	    gnus-article-prepare-hook
> +	    gnus-article-decode-hook
> +	    gnus-display-mime-function
> +	    gnus-break-pages)

Is the above set of variables (or this approach in general) based off of
an example from the Gnus source code?

If so, I should probably read through it to orient myself.  I am mostly
clueless when it comes to writing Gnus-related elisp and pretty much
stumble around its source code looking for something that seems close to
what I want.

> +	(mapc (lambda (article)
> +		(gnus-summary-display-article article)
> +		(push (format "From mboxrd@z Thu Jan  1 00:00:00 1970\n%s\n"
> +			      (with-current-buffer gnus-article-buffer
> +				(buffer-substring-no-properties
> +				 (point-min)
> +				 (point-max))))
> +			      message))
> +		articles)

For mboxrd, I suppose there needs to be some From-munging.

> +	      (insert (apply #'concat (nreverse message)))))))

Re: [RFC PATCH] gnus: Add piem-gnus-mid-to-thread

[yoctocell]

On Mon, Jan 18 2021, Kyle Meyer wrote:

>> ---
>> The function works as expected when running
>> 'M-: (funcall (piem-gnus-mid-thread t))', but when running
>> piem-b4-am-from-mid interactively, it just inserts an empty string.
>>
>> Any ideas of why this happens?  Sorry, my elisp skills are not that great.
>
> Yes, I think you just need to move the lambda deeper so that the
> messages are collected outside of the returned function.

Ok, I will see if this works.

>> diff --git a/piem-gnus.el b/piem-gnus.el
>> index 5f10be8..f91f91c 100644
>> --- a/piem-gnus.el
>> +++ b/piem-gnus.el
>> @@ -58,6 +58,30 @@
>>  ;; If there is an easy way to generate an mbox for a thread in Gnus, a
>>  ;; function for `piem-mid-to-thread-functions' should be defined.
>>  
>> +(defun piem-gnus-mid-to-thread (_mid)
>
> Hmm, yeah, I don't think we can convince Gnus to do much useful with the
> message ID here.  However, perhaps there should at least be a check that
> the given message ID matches the one for the article at point before
> returning a function (i.e. claiming piem-gnus-mid-to-thread can generate
> a thread for the given MID).
>
> Also, I think this would need to be guarded by
>
>   (derived-mode-p 'gnus-summary-mode)

Yeah, that's a good idea.  `gnus-summary-refer-parent-article` doesn't
seem to work in article mode.

>> +    (save-excursion
>> +      ;; Cursor has to be at the root of the thread
>> +      (gnus-summary-refer-parent-article most-positive-fixnum)
>> +      (let ((articles (gnus-summary-articles-in-thread))
>> +	    message
>> +	    ;; Just show raw message
>> +	    (gnus-have-all-headers t)
>> +	    gnus-article-prepare-hook
>> +	    gnus-article-decode-hook
>> +	    gnus-display-mime-function
>> +	    gnus-break-pages)
>
> Is the above set of variables (or this approach in general) based off of
> an example from the Gnus source code?

Yeah, I had to do quite a bit of digging to figure this out.  You can
find them in the definition of `gnus-summary-show-article`.

>> +	(mapc (lambda (article)
>> +		(gnus-summary-display-article article)
>> +		(push (format "From mboxrd@z Thu Jan  1 00:00:00 1970\n%s\n"
>> +			      (with-current-buffer gnus-article-buffer
>> +				(buffer-substring-no-properties
>> +				 (point-min)
>> +				 (point-max))))
>> +			      message))
>> +		articles)
>
> For mboxrd, I suppose there needs to be some From-munging.

Sorry, I don't really know much about mbox, I just copied the line from
the mbox file generated by public-inbox :)

Re: [RFC PATCH] gnus: Add piem-gnus-mid-to-thread

[Kyle Meyer]

yoctocell writes:

> On Mon, Jan 18 2021, Kyle Meyer wrote:
>> For mboxrd, I suppose there needs to be some From-munging.
>
> Sorry, I don't really know much about mbox, I just copied the line from
> the mbox file generated by public-inbox :)

No worries.  The initial From line you're adding looks fine.  I was
referring to escaping lines in the message body that start with "From "
(or, for mboxrd, more precisely ">?From").  This is described a bit at
<https://en.wikipedia.org/wiki/Mbox>.

Here's an example from a public-inbox instance:

  $ curl -fSs https://yhetil.org/orgmode/871rpt9zc4.fsf@kyleam.com/raw | grep 'From '
  From mboxrd@z Thu Jan  1 00:00:00 1970
  >From what I gather from briefly searching around (mostly based on this

Re: [RFC PATCH] gnus: Add piem-gnus-mid-to-thread

[Xinglu Chen]

On Tue, Jan 19 2021, Kyle Meyer wrote:

> No worries.  The initial From line you're adding looks fine.  I was
> referring to escaping lines in the message body that start with "From "
> (or, for mboxrd, more precisely ">?From").  This is described a bit at
> <https://en.wikipedia.org/wiki/Mbox>.
>
> Here's an example from a public-inbox instance:
>
>   $ curl -fSs https://yhetil.org/orgmode/871rpt9zc4.fsf@kyleam.com/raw | grep 'From '
>   From mboxrd@z Thu Jan  1 00:00:00 1970
>   >From what I gather from briefly searching around (mostly based on this

Ok, thanks for clarifying.

[RFC PATCH v2] gnus: Add piem-gnus-mid-to-thread

[Xinglu Chen]

Inserts a string of the whole thread in mbox format.
---
Changes from v1

- Add safety check for major mode and message id before trying to generate
the mbox file
- Escape "^From" in the body to prevent ambiguity

 piem-gnus.el | 40 ++++++++++++++++++++++++++++++++++++----
 1 file changed, 36 insertions(+), 4 deletions(-)

diff --git a/piem-gnus.el b/piem-gnus.el
index 5f10be8..828ce1f 100644
--- a/piem-gnus.el
+++ b/piem-gnus.el
@@ -55,8 +55,38 @@
             (match-string 1 mid)
           mid)))))
 
-;; If there is an easy way to generate an mbox for a thread in Gnus, a
-;; function for `piem-mid-to-thread-functions' should be defined.
+(defun piem-gnus-mid-to-thread (mid)
+  (when (and (derived-mode-p 'gnus-summary-mode)
+	     (string-equal (substring
+			    (mail-header-id (gnus-summary-article-header))
+			    1 -1)	; Remove "<" and ">"
+			   mid))
+    (save-excursion
+      ;; Cursor has to be at the root of the thread
+      (gnus-summary-refer-parent-article most-positive-fixnum)
+      (let ((articles (gnus-summary-articles-in-thread))
+	    message
+	    ;; Just show raw message
+	    (gnus-have-all-headers t)
+	    gnus-article-prepare-hook
+	    gnus-article-decode-hook
+	    gnus-display-mime-function
+	    gnus-break-pages)
+	(mapc (lambda (article)
+		(gnus-summary-display-article article)
+		(push (format
+		       "From mboxrd@z Thu Jan  1 00:00:00 1970\n%s\n"
+		       (replace-regexp-in-string ; From-munge
+			"^From "
+			"^>From "
+			(with-current-buffer gnus-article-buffer
+			  (buffer-substring-no-properties
+			   (point-min)
+			   (point-max)))))
+		      message))
+	      articles)
+	(lambda ()
+	  (insert (apply #'concat (nreverse message))))))))
 
 (defun piem-gnus-am-ready-mbox ()
   "Return a function that inserts an am-ready mbox.
@@ -97,10 +127,12 @@ the mode if ARG is omitted or nil."
       (progn
         (add-hook 'piem-am-ready-mbox-functions #'piem-gnus-am-ready-mbox)
         (add-hook 'piem-get-inbox-functions #'piem-gnus-get-inbox)
-        (add-hook 'piem-get-mid-functions #'piem-gnus-get-mid))
+        (add-hook 'piem-get-mid-functions #'piem-gnus-get-mid)
+	(add-hook 'piem-mid-to-thread-functions #'piem-gnus-mid-to-thread))
     (remove-hook 'piem-am-ready-mbox-functions #'piem-gnus-am-ready-mbox)
     (remove-hook 'piem-get-inbox-functions #'piem-gnus-get-inbox)
-    (remove-hook 'piem-get-mid-functions #'piem-gnus-get-mid)))
+    (remove-hook 'piem-get-mid-functions #'piem-gnus-get-mid)
+    (remove-hook 'piem-mid-to-thread-functions #'piem-gnus-mid-to-thread)))
 
 ;;; piem-gnus.el ends here
 (provide 'piem-gnus)

base-commit: 57f802b2a43ac64c28a5a9ddb9da0afaf910975e
-- 
2.29.2

Re: [RFC PATCH v2] gnus: Add piem-gnus-mid-to-thread

[Kyle Meyer]

Xinglu Chen writes:

> Inserts a string of the whole thread in mbox format.
> ---
> Changes from v1
>
> - Add safety check for major mode and message id before trying to generate
> the mbox file
> - Escape "^From" in the body to prevent ambiguity

I played around with this a bit, and all series seemed to get applied
without issues.  I think interface-wise it'd be a bit nicer if we could
do this work behind the scenes without the user seeing any changes in
the layout/rendering of Gnus buffers, but I'm not sure that's really
feasible.

> diff --git a/piem-gnus.el b/piem-gnus.el
> index 5f10be8..828ce1f 100644
> --- a/piem-gnus.el
> +++ b/piem-gnus.el
> @@ -55,8 +55,38 @@
>              (match-string 1 mid)
>            mid)))))
>  
> -;; If there is an easy way to generate an mbox for a thread in Gnus, a
> -;; function for `piem-mid-to-thread-functions' should be defined.
> +(defun piem-gnus-mid-to-thread (mid)
> +  (when (and (derived-mode-p 'gnus-summary-mode)
> +	     (string-equal (substring
> +			    (mail-header-id (gnus-summary-article-header))
> +			    1 -1)	; Remove "<" and ">"
> +			   mid))

Great, thanks for adding those guards.

> +    (save-excursion
> +      ;; Cursor has to be at the root of the thread
> +      (gnus-summary-refer-parent-article most-positive-fixnum)
> +      (let ((articles (gnus-summary-articles-in-thread))
> +	    message

I'm going to s/message/&s/ on apply.  It's a cosmetic nit, but I'll trip
over this whenever I need to revisit this code if I don't make that
change.

> +	    ;; Just show raw message
> +	    (gnus-have-all-headers t)
> +	    gnus-article-prepare-hook
> +	    gnus-article-decode-hook
> +	    gnus-display-mime-function
> +	    gnus-break-pages)

When compiling, this gives the following warnings:

  In toplevel form:
  piem-gnus.el:58:1:Warning: Unused lexical variable
      ‘gnus-display-mime-function’
  piem-gnus.el:58:1:Warning: Unused lexical variable ‘gnus-article-decode-hook’
  piem-gnus.el:58:1:Warning: Unused lexical variable ‘gnus-article-prepare-hook’

I'll silence those by adding

  (require 'gnus-art)

> +	(mapc (lambda (article)
> +		(gnus-summary-display-article article)
> +		(push (format
> +		       "From mboxrd@z Thu Jan  1 00:00:00 1970\n%s\n"
> +		       (replace-regexp-in-string ; From-munge
> +			"^From "
> +			"^>From "

There's a spurious "^" making it into the replacement.  Also this
doesn't handle mboxrd's ">From " escaping.  I'll squash the following
in.

diff --git a/piem-gnus.el b/piem-gnus.el
index 4293049..d81c086 100644
--- a/piem-gnus.el
+++ b/piem-gnus.el
@@ -77,8 +77,8 @@ (defun piem-gnus-mid-to-thread (mid)
                 (push (format
                        "From mboxrd@z Thu Jan  1 00:00:00 1970\n%s\n"
                        (replace-regexp-in-string ; From-munge
-                        "^From "
-                        "^>From "
+                        "^>*From "
+                        ">\\&"
                         (with-current-buffer gnus-article-buffer
                           (buffer-substring-no-properties
                            (point-min)

> +			(with-current-buffer gnus-article-buffer
> +			  (buffer-substring-no-properties
> +			   (point-min)
> +			   (point-max)))))
> +		      message))
> +	      articles)
> +	(lambda ()
> +	  (insert (apply #'concat (nreverse message))))))))

Mostly just thinking aloud: this could be rewritten without concat by
using (dolist ... (insert ...)), and it might eventually be worth
considering building the mbox up in a temporary buffer rather than as a
list of strings.  But I think it's fine as is.

Also, I've replaced the tabs with spaces.  Sorry for not having set
indent-tabs-mode for the project.  I've now done that in 9d60b3e.

Pushed (c9228b9).

Thank you!

Re: [RFC PATCH v2] gnus: Add piem-gnus-mid-to-thread

[Xinglu Chen]

On Wed, Jan 20 2021, Kyle Meyer wrote:

> I played around with this a bit, and all series seemed to get applied
> without issues.  I think interface-wise it'd be a bit nicer if we could
> do this work behind the scenes without the user seeing any changes in
> the layout/rendering of Gnus buffers, but I'm not sure that's really
> feasible.

I couldn't really find a way to do that, all the functions seem perform
side effects instead of passing data around.

> Mostly just thinking aloud: this could be rewritten without concat by
> using (dolist ... (insert ...)), and it might eventually be worth
> considering building the mbox up in a temporary buffer rather than as a
> list of strings.  But I think it's fine as is.
>
> Also, I've replaced the tabs with spaces.  Sorry for not having set
> indent-tabs-mode for the project.  I've now done that in 9d60b3e.
>
> Pushed (c9228b9).
>
> Thank you!

Thanks for applying!