* [RFC][PATCH] emacs: Provide scaffolding so that the new `shr' HTML renderer can run.
@ 2011-12-19 15:45 David Edmondson
2011-12-19 17:50 ` Aaron Ecay
2011-12-20 6:38 ` Chris Gray
0 siblings, 2 replies; 11+ messages in thread
From: David Edmondson @ 2011-12-19 15:45 UTC (permalink / raw)
To: notmuch
Latest gnus provides a new HTML renderer entirely in lisp. It requires
some minor but ugly scaffolding to allow use with notmuch.
---
This is pretty horrible, but works in the cases that I tested. It
would be useful if a few other people could test. Note that `cid:'
images included with a text/html part are not inserted in the correct
place, but that also appears to be the case for gnus.
This should be tagged `notmuch::wip' in the new world order!
emacs/notmuch-show.el | 14 +++++++++++++-
1 files changed, 13 insertions(+), 1 deletions(-)
diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 63b01e5..f52f233 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -320,6 +320,13 @@ message at DEPTH in the current thread."
;; ange-ftp, which is reasonable to use here.
(mm-write-region (point-min) (point-max) file nil nil nil 'no-conversion t)))))
+;; Nonsense required to have the new gnus `shr' HTML display code
+;; work.
+(defvar gnus-summary-buffer)
+(defvar gnus-inhibit-images)
+(if (not (fboundp 'gnus-blocked-images))
+ (defun gnus-blocked-images () nil))
+
(defun notmuch-show-mm-display-part-inline (msg part nth content-type)
"Use the mm-decode/mm-view functions to display a part in the
current buffer, if possible."
@@ -331,7 +338,12 @@ current buffer, if possible."
(let ((content (notmuch-show-get-bodypart-content msg part nth)))
(insert content)
(set-buffer display-buffer)
- (mm-display-part handle)
+
+ ;; Nonsense required to have the new gnus `shr' HTML
+ ;; display code work.
+ (let ((gnus-inhibit-images nil))
+ (makunbound 'gnus-summary-buffer) ; Blech.
+ (mm-display-part handle))
t)
nil)))))
--
1.7.7.3
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [RFC][PATCH] emacs: Provide scaffolding so that the new `shr' HTML renderer can run.
2011-12-19 15:45 [RFC][PATCH] emacs: Provide scaffolding so that the new `shr' HTML renderer can run David Edmondson
@ 2011-12-19 17:50 ` Aaron Ecay
2011-12-20 6:38 ` Chris Gray
1 sibling, 0 replies; 11+ messages in thread
From: Aaron Ecay @ 2011-12-19 17:50 UTC (permalink / raw)
To: David Edmondson, notmuch
David,
This patch doesn’t allow users to have their own settings for
shr-{inhibit,block}-images, since it forces these values to nil (via the
corresponding gnus variables).
(For those looking to follow the code, here’s the call path:
notmuch-show-mm-display-part-inline ->
mm-display-part ->
mm-display-inline -> (via lookup table)
mm-inline-text-html -> (via LUT)
mm-shr
The last function is where this objectionable rebinding happens)
Would a better approach be:
- clone the mm-shr function, replacing the vile gnus-specific code with
something functionally similar that takes the values from notmuch-foo
variables instead
- add '(notmuch-shr . notmuch-mm-shr) to the mm-text-html-renderer-alist
variable
- tell notmuch users to customize mm-text-html-renderer to 'notmuch-shr
if they want to use this renderer
The default values of the notmuch-{inhibit,block}-images variables
should prevent network requests for images being made by default* – your
proposed nil/nil values don’t do this, AFAICT. Not doing network
requests for HTML email has been Spam Mitigation/Identity Protection 101
for a long time;** notmuch’s default behavior in this area has always
bothered me.
(Also, while we are on the subject of wonky defaults, why is
notmuch-show-all-multipart/alternative-parts set to t by default? This
is certainly not the best setting.)
FWIW, the patch does seem to allow using shr as the HTML renderer, but
for the reasons expressed I don’t think it solves the broader problem.
Aaron
* I think that inhibit -> nil, block -> ".*" should achieve this, but I’m
not sure/haven’t tested
** For example, Gmail makes you click a link at the top of an email
before it will load any network images contained therein.
--
Aaron Ecay
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [RFC][PATCH] emacs: Provide scaffolding so that the new `shr' HTML renderer can run.
2011-12-19 15:45 [RFC][PATCH] emacs: Provide scaffolding so that the new `shr' HTML renderer can run David Edmondson
2011-12-19 17:50 ` Aaron Ecay
@ 2011-12-20 6:38 ` Chris Gray
2011-12-20 8:35 ` David Edmondson
` (2 more replies)
1 sibling, 3 replies; 11+ messages in thread
From: Chris Gray @ 2011-12-20 6:38 UTC (permalink / raw)
To: David Edmondson, notmuch
On Mon, 19 Dec 2011 15:45:59 +0000, David Edmondson <dme@dme.org> wrote:
> Latest gnus provides a new HTML renderer entirely in lisp. It requires
> some minor but ugly scaffolding to allow use with notmuch.
> ---
>
> This is pretty horrible, but works in the cases that I tested. It
> would be useful if a few other people could test. Note that `cid:'
> images included with a text/html part are not inserted in the correct
> place, but that also appears to be the case for gnus.
>
> This should be tagged `notmuch::wip' in the new world order!
>
> emacs/notmuch-show.el | 14 +++++++++++++-
> 1 files changed, 13 insertions(+), 1 deletions(-)
>
> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> index 63b01e5..f52f233 100644
> --- a/emacs/notmuch-show.el
> +++ b/emacs/notmuch-show.el
> @@ -320,6 +320,13 @@ message at DEPTH in the current thread."
> ;; ange-ftp, which is reasonable to use here.
> (mm-write-region (point-min) (point-max) file nil nil nil 'no-conversion t)))))
>
> +;; Nonsense required to have the new gnus `shr' HTML display code
> +;; work.
> +(defvar gnus-summary-buffer)
> +(defvar gnus-inhibit-images)
I believe that gnus-inhibit-images is defined in gnus-art.el. Perhaps
that file should just be require'd rather than having this workaround.
> +(if (not (fboundp 'gnus-blocked-images))
> + (defun gnus-blocked-images () nil))
> +
> (defun notmuch-show-mm-display-part-inline (msg part nth content-type)
> "Use the mm-decode/mm-view functions to display a part in the
> current buffer, if possible."
> @@ -331,7 +338,12 @@ current buffer, if possible."
> (let ((content (notmuch-show-get-bodypart-content msg part nth)))
> (insert content)
> (set-buffer display-buffer)
> - (mm-display-part handle)
> +
> + ;; Nonsense required to have the new gnus `shr' HTML
> + ;; display code work.
> + (let ((gnus-inhibit-images nil))
> + (makunbound 'gnus-summary-buffer) ; Blech.
This is working around a bug in gnus. I think the better solution would
be for gnus to fix the bug. The following patch against gnus works for
me. (I have tried submitting it to the gnus bug list, but have not been
able to check if it got through.)
Cheers,
Chris
commit 45e5a06ea63d8d9f9a962db7d739c7d7056ef712 (HEAD, refs/heads/master)
Author: Chris Gray <chrismgray@gmail.com>
Date: Mon Dec 19 23:33:30 2011 -0700
mm-decode.el (mm-shr): Added check to bufferp for gnus-summary-buffer
If gnus-summary-buffer is a string, then (buffer-name
gnus-summary-buffer) gives an error.
Modified lisp/mm-decode.el
diff --git a/lisp/mm-decode.el b/lisp/mm-decode.el
index 44e2af5..7005dd7 100644
--- a/lisp/mm-decode.el
+++ b/lisp/mm-decode.el
@@ -1724,6 +1724,7 @@ If RECURSIVE, search recursively."
(buffer-string))))))
shr-inhibit-images shr-blocked-images charset char)
(if (and (boundp 'gnus-summary-buffer)
+ (bufferp gnus-summary-buffer)
(buffer-name gnus-summary-buffer))
(with-current-buffer gnus-summary-buffer
(setq shr-inhibit-images gnus-inhibit-images
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [RFC][PATCH] emacs: Provide scaffolding so that the new `shr' HTML renderer can run.
2011-12-20 6:38 ` Chris Gray
@ 2011-12-20 8:35 ` David Edmondson
2011-12-20 14:27 ` Chris Gray
2011-12-20 8:40 ` Aaron Ecay
2012-01-23 14:43 ` Florian Friesdorf
2 siblings, 1 reply; 11+ messages in thread
From: David Edmondson @ 2011-12-20 8:35 UTC (permalink / raw)
To: Chris Gray, notmuch
On Mon, 19 Dec 2011 23:38:36 -0700, Chris Gray <chrismgray@gmail.com> wrote:
> > + (makunbound 'gnus-summary-buffer) ; Blech.
>
> This is working around a bug in gnus. I think the better solution would
> be for gnus to fix the bug. The following patch against gnus works for
> me. (I have tried submitting it to the gnus bug list, but have not been
> able to check if it got through.)
I wonder if `boundp' is just a typo for `bufferp'?
More on the other comments after thought.
dme.
--
David Edmondson, http://dme.org
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC][PATCH] emacs: Provide scaffolding so that the new `shr' HTML renderer can run.
2011-12-20 8:35 ` David Edmondson
@ 2011-12-20 14:27 ` Chris Gray
2011-12-20 14:33 ` David Edmondson
0 siblings, 1 reply; 11+ messages in thread
From: Chris Gray @ 2011-12-20 14:27 UTC (permalink / raw)
To: David Edmondson, notmuch
On Tue, 20 Dec 2011 08:35:42 +0000, David Edmondson <dme@dme.org> wrote:
> On Mon, 19 Dec 2011 23:38:36 -0700, Chris Gray <chrismgray@gmail.com> wrote:
> > > + (makunbound 'gnus-summary-buffer) ; Blech.
> >
> > This is working around a bug in gnus. I think the better solution would
> > be for gnus to fix the bug. The following patch against gnus works for
> > me. (I have tried submitting it to the gnus bug list, but have not been
> > able to check if it got through.)
>
> I wonder if `boundp' is just a typo for `bufferp'?
I originally thought so as well, but bufferp blows up if given an
unbound variable and buffer-name blows up if given a string.
Cheers,
Chris
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC][PATCH] emacs: Provide scaffolding so that the new `shr' HTML renderer can run.
2011-12-20 14:27 ` Chris Gray
@ 2011-12-20 14:33 ` David Edmondson
2011-12-20 16:09 ` Chris Gray
0 siblings, 1 reply; 11+ messages in thread
From: David Edmondson @ 2011-12-20 14:33 UTC (permalink / raw)
To: Chris Gray, notmuch
On Tue, 20 Dec 2011 07:27:24 -0700, Chris Gray <chrismgray@gmail.com> wrote:
> On Tue, 20 Dec 2011 08:35:42 +0000, David Edmondson <dme@dme.org> wrote:
> > On Mon, 19 Dec 2011 23:38:36 -0700, Chris Gray <chrismgray@gmail.com> wrote:
> > > > + (makunbound 'gnus-summary-buffer) ; Blech.
> > >
> > > This is working around a bug in gnus. I think the better solution would
> > > be for gnus to fix the bug. The following patch against gnus works for
> > > me. (I have tried submitting it to the gnus bug list, but have not been
> > > able to check if it got through.)
> >
> > I wonder if `boundp' is just a typo for `bufferp'?
>
> I originally thought so as well, but bufferp blows up if given an
> unbound variable and buffer-name blows up if given a string.
When would `gnus-summary-buffer' ever be unbound?
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC][PATCH] emacs: Provide scaffolding so that the new `shr' HTML renderer can run.
2011-12-20 14:33 ` David Edmondson
@ 2011-12-20 16:09 ` Chris Gray
0 siblings, 0 replies; 11+ messages in thread
From: Chris Gray @ 2011-12-20 16:09 UTC (permalink / raw)
To: David Edmondson, notmuch
On Tue, 20 Dec 2011 14:33:01 +0000, David Edmondson <dme@dme.org> wrote:
> On Tue, 20 Dec 2011 07:27:24 -0700, Chris Gray <chrismgray@gmail.com> wrote:
> > On Tue, 20 Dec 2011 08:35:42 +0000, David Edmondson <dme@dme.org> wrote:
> > > On Mon, 19 Dec 2011 23:38:36 -0700, Chris Gray <chrismgray@gmail.com> wrote:
> > > > > + (makunbound 'gnus-summary-buffer) ; Blech.
> > > >
> > > > This is working around a bug in gnus. I think the better solution would
> > > > be for gnus to fix the bug. The following patch against gnus works for
> > > > me. (I have tried submitting it to the gnus bug list, but have not been
> > > > able to check if it got through.)
> > >
> > > I wonder if `boundp' is just a typo for `bufferp'?
> >
> > I originally thought so as well, but bufferp blows up if given an
> > unbound variable and buffer-name blows up if given a string.
>
> When would `gnus-summary-buffer' ever be unbound?
In the optimal situation where we were able to use mm-decode without
requiring gnus.el, then gnus-summary-buffer would be unbound.
Cheers,
Chris
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC][PATCH] emacs: Provide scaffolding so that the new `shr' HTML renderer can run.
2011-12-20 6:38 ` Chris Gray
2011-12-20 8:35 ` David Edmondson
@ 2011-12-20 8:40 ` Aaron Ecay
2011-12-20 17:06 ` Chris Gray
2012-01-23 14:43 ` Florian Friesdorf
2 siblings, 1 reply; 11+ messages in thread
From: Aaron Ecay @ 2011-12-20 8:40 UTC (permalink / raw)
To: Chris Gray, David Edmondson, notmuch
On Mon, 19 Dec 2011 23:38:36 -0700, Chris Gray <chrismgray@gmail.com> wrote:
> This is working around a bug in gnus.
Arguably this is true, but the real “bug” (conceptual error) is that the
MIME handling libraries and gnus are a little too tightly coupled. Why
should notmuch users have to load gnus (gnus-art.el does (require 'gnus),
which brings in tens of thousands of lines of Elisp)[1], or customize
gnus-* variables to use general-purpose MIME viewing, HTML rendering
facilities?
The best GNUS-side solution would be to make mm-shr GNUS-agnostic, and
probably to introduce shr-{inhibit,blocked}-images as customizable
variables in their own right (which could inherit their values from the
gnus-* versions under the right circumstances).
I hope that the GNUS folks are receptive to this approach, but if they
aren’t I think it’s better for notmuch to not go the way of requiring
that GNUS be loaded to function.
Aaron
[1] I see that (featurep 'gnus) returns t for me, so that horse is
already out of the barn. But it isn’t something we should be
seeking to perpetuate.
--
Aaron Ecay
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [RFC][PATCH] emacs: Provide scaffolding so that the new `shr' HTML renderer can run.
2011-12-20 8:40 ` Aaron Ecay
@ 2011-12-20 17:06 ` Chris Gray
0 siblings, 0 replies; 11+ messages in thread
From: Chris Gray @ 2011-12-20 17:06 UTC (permalink / raw)
To: notmuch
On Tue, 20 Dec 2011 03:40:44 -0500, Aaron Ecay <aaronecay@gmail.com> wrote:
> On Mon, 19 Dec 2011 23:38:36 -0700, Chris Gray <chrismgray@gmail.com> wrote:
> > This is working around a bug in gnus.
>
> Arguably this is true, but the real “bug” (conceptual error) is that the
> MIME handling libraries and gnus are a little too tightly coupled. Why
> should notmuch users have to load gnus (gnus-art.el does (require 'gnus),
> which brings in tens of thousands of lines of Elisp)[1], or customize
> gnus-* variables to use general-purpose MIME viewing, HTML rendering
> facilities?
I agree with this 100%. However, we should still try to use emacs
coding best-practices, which means not defining variables that are
defined in other packages. Unfortunately, getting the gnus folks to fix
their library might take a while, so it might be okay to relax those
best-practices while we wait, but we should be aware of the fact that
that's what we're doing.
Cheers,
Chris
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC][PATCH] emacs: Provide scaffolding so that the new `shr' HTML renderer can run.
2011-12-20 6:38 ` Chris Gray
2011-12-20 8:35 ` David Edmondson
2011-12-20 8:40 ` Aaron Ecay
@ 2012-01-23 14:43 ` Florian Friesdorf
2012-01-23 17:14 ` Chris Gray
2 siblings, 1 reply; 11+ messages in thread
From: Florian Friesdorf @ 2012-01-23 14:43 UTC (permalink / raw)
To: Chris Gray, David Edmondson, notmuch
On Mon, 19 Dec 2011 23:38:36 -0700, Chris Gray <chrismgray@gmail.com> wrote:
> > +;; Nonsense required to have the new gnus `shr' HTML display code
> > +;; work.
> > +(defvar gnus-summary-buffer)
> > +(defvar gnus-inhibit-images)
>
> I believe that gnus-inhibit-images is defined in gnus-art.el. Perhaps
> that file should just be require'd rather than having this workaround.
With notmuch-0.11 (require 'gnus-art') "fixes" the void
gnus-inhibit-images error.
> > +(if (not (fboundp 'gnus-blocked-images))
> > + (defun gnus-blocked-images () nil))
> > +
> > (defun notmuch-show-mm-display-part-inline (msg part nth content-type)
> > "Use the mm-decode/mm-view functions to display a part in the
> > current buffer, if possible."
> > @@ -331,7 +338,12 @@ current buffer, if possible."
> > (let ((content (notmuch-show-get-bodypart-content msg part nth)))
> > (insert content)
> > (set-buffer display-buffer)
> > - (mm-display-part handle)
> > +
> > + ;; Nonsense required to have the new gnus `shr' HTML
> > + ;; display code work.
> > + (let ((gnus-inhibit-images nil))
> > + (makunbound 'gnus-summary-buffer) ; Blech.
>
> This is working around a bug in gnus. I think the better solution would
> be for gnus to fix the bug. The following patch against gnus works for
> me. (I have tried submitting it to the gnus bug list, but have not been
> able to check if it got through.)
emacs git master has a fix for that, so you seem to have got through.
--
Florian Friesdorf <flo@chaoflow.net>
GPG FPR: 7A13 5EEE 1421 9FC2 108D BAAF 38F8 99A3 0C45 F083
Jabber/XMPP: flo@chaoflow.net
IRC: chaoflow on freenode,ircnet,blafasel,OFTC
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC][PATCH] emacs: Provide scaffolding so that the new `shr' HTML renderer can run.
2012-01-23 14:43 ` Florian Friesdorf
@ 2012-01-23 17:14 ` Chris Gray
0 siblings, 0 replies; 11+ messages in thread
From: Chris Gray @ 2012-01-23 17:14 UTC (permalink / raw)
To: Florian Friesdorf, David Edmondson, notmuch
On Mon, 23 Jan 2012 15:43:08 +0100, Florian Friesdorf <flo@chaoflow.net> wrote:
> With notmuch-0.11 (require 'gnus-art') "fixes" the void
> gnus-inhibit-images error.
This is true, though as Aaron Ecay pointed out in the discussion of this
patch, it does bring in all of gnus. My current workaround to the bug
of gnus-inhibit-images being void is to load gnus, so that's just as
bad, but it would be great if we didn't have to do that.
> emacs git master has a fix for that, so you seem to have got through.
I got a notification that it went in the gnus git repo a couple of weeks
ago, so it is nice to know that it has been propagated to the emacs repo
as well.
Cheers,
Chris
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2012-01-23 17:15 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-12-19 15:45 [RFC][PATCH] emacs: Provide scaffolding so that the new `shr' HTML renderer can run David Edmondson
2011-12-19 17:50 ` Aaron Ecay
2011-12-20 6:38 ` Chris Gray
2011-12-20 8:35 ` David Edmondson
2011-12-20 14:27 ` Chris Gray
2011-12-20 14:33 ` David Edmondson
2011-12-20 16:09 ` Chris Gray
2011-12-20 8:40 ` Aaron Ecay
2011-12-20 17:06 ` Chris Gray
2012-01-23 14:43 ` Florian Friesdorf
2012-01-23 17:14 ` Chris Gray
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).