* bug#28402: 25.2; shr.el uses shr-tag-img despite set shr-external-rendering-functions @ 2017-09-09 19:39 Vasilij Schneidermann 2017-09-11 16:04 ` Eli Zaretskii 2017-09-13 17:27 ` Lars Ingebrigtsen 0 siblings, 2 replies; 18+ messages in thread From: Vasilij Schneidermann @ 2017-09-09 19:39 UTC (permalink / raw) To: 28402; +Cc: larsi I'm using shr.el for a package and override `shr-tag-img` by let-binding `shr-external-rendering-functions` to an alist containing an alternative rendering function that deals better with local images. Despite this `shr-tag-img` is still directly called when rendering tables with images inside them. The same problem applies to other directly called rendering functions, as opposed to indirectly called ones as seen in `shr-descend`, the function honoring `shr-external-rendering-functions`. ^ permalink raw reply [flat|nested] 18+ messages in thread
* bug#28402: 25.2; shr.el uses shr-tag-img despite set shr-external-rendering-functions 2017-09-09 19:39 bug#28402: 25.2; shr.el uses shr-tag-img despite set shr-external-rendering-functions Vasilij Schneidermann @ 2017-09-11 16:04 ` Eli Zaretskii 2017-09-13 17:22 ` Vasilij Schneidermann 2017-09-13 17:27 ` Lars Ingebrigtsen 1 sibling, 1 reply; 18+ messages in thread From: Eli Zaretskii @ 2017-09-11 16:04 UTC (permalink / raw) To: Vasilij Schneidermann; +Cc: larsi, 28402 > From: Vasilij Schneidermann <v.schneidermann@gmail.com> > Date: Sat, 9 Sep 2017 21:39:39 +0200 > Cc: larsi@gnus.org > > I'm using shr.el for a package and override `shr-tag-img` by let-binding > `shr-external-rendering-functions` to an alist containing an alternative > rendering function that deals better with local images. Despite this > `shr-tag-img` is still directly called when rendering tables with images > inside them. The same problem applies to other directly called > rendering functions, as opposed to indirectly called ones as seen in > `shr-descend`, the function honoring `shr-external-rendering-functions`. From a cursory look into shr.el, it looks like shr-external-rendering-functions were introduced to support some specific eww.el needs, not as a general lever for tweaking shr.el from outside it. That's why only a handful of functions honor that variable. Would you like to submit a patch that would make all shr-tag-* functions consult that variable first? ^ permalink raw reply [flat|nested] 18+ messages in thread
* bug#28402: 25.2; shr.el uses shr-tag-img despite set shr-external-rendering-functions 2017-09-11 16:04 ` Eli Zaretskii @ 2017-09-13 17:22 ` Vasilij Schneidermann 2017-09-13 18:37 ` Eli Zaretskii 0 siblings, 1 reply; 18+ messages in thread From: Vasilij Schneidermann @ 2017-09-13 17:22 UTC (permalink / raw) To: Eli Zaretskii; +Cc: larsi, 28402 > From a cursory look into shr.el, it looks like > shr-external-rendering-functions were introduced to support some > specific eww.el needs, not as a general lever for tweaking shr.el from > outside it. That's why only a handful of functions honor that > variable. I suspected as much. > Would you like to submit a patch that would make all shr-tag-* > functions consult that variable first? Before doing that I'd like to get a confirmation from the author whether it's intended for shr-external-rendering-functions to become public API and whether it should work globally, hence why I included them in the CC. ^ permalink raw reply [flat|nested] 18+ messages in thread
* bug#28402: 25.2; shr.el uses shr-tag-img despite set shr-external-rendering-functions 2017-09-13 17:22 ` Vasilij Schneidermann @ 2017-09-13 18:37 ` Eli Zaretskii 2017-09-24 13:10 ` Vasilij Schneidermann 0 siblings, 1 reply; 18+ messages in thread From: Eli Zaretskii @ 2017-09-13 18:37 UTC (permalink / raw) To: Vasilij Schneidermann; +Cc: larsi, 28402 > Date: Wed, 13 Sep 2017 19:22:32 +0200 > From: Vasilij Schneidermann <v.schneidermann@gmail.com> > Cc: 28402@debbugs.gnu.org, larsi@gnus.org > > > Would you like to submit a patch that would make all shr-tag-* > > functions consult that variable first? > > Before doing that I'd like to get a confirmation from the author whether > it's intended for shr-external-rendering-functions to become public API > and whether it should work globally, hence why I included them in the > CC. Fair enough. Fortunately, it sounds like Lars have just agreed to that. Thanks. ^ permalink raw reply [flat|nested] 18+ messages in thread
* bug#28402: 25.2; shr.el uses shr-tag-img despite set shr-external-rendering-functions 2017-09-13 18:37 ` Eli Zaretskii @ 2017-09-24 13:10 ` Vasilij Schneidermann 2017-09-29 7:31 ` Eli Zaretskii 0 siblings, 1 reply; 18+ messages in thread From: Vasilij Schneidermann @ 2017-09-24 13:10 UTC (permalink / raw) To: Eli Zaretskii; +Cc: larsi, 28402 [-- Attachment #1: Type: text/plain, Size: 131 bytes --] > Fair enough. Fortunately, it sounds like Lars have just agreed to > that. Alright, I finally got to creating a patch for this. [-- Attachment #2: 0001-Add-indirection-to-all-shr-tag-calls.patch --] [-- Type: text/x-diff, Size: 3625 bytes --] From abe3f5702999b3b11645d924c7d05f48f4a265c4 Mon Sep 17 00:00:00 2001 From: Vasilij Schneidermann <mail@vasilij.de> Date: Sun, 24 Sep 2017 14:59:10 +0200 Subject: [PATCH] Add indirection to all shr-tag-* calls The 'shr-external-rendering-functions' variable was previously only honored in the shr-descend function, now all direct calls to the shr-tag-* functions have been replaced by a call to 'shr-indirect-call' which tries using an alternative rendering function first. * lisp/net/shr.el (shr-indirect-call): New helper function. (shr-descend, shr-tag-object, shr-tag-video): (shr-collect-extra-strings-in-table): Fix callers --- lisp/net/shr.el | 35 +++++++++++++++++++---------------- 1 file changed, 19 insertions(+), 16 deletions(-) diff --git a/lisp/net/shr.el b/lisp/net/shr.el index 7af6148e47..fe5197b35f 100644 --- a/lisp/net/shr.el +++ b/lisp/net/shr.el @@ -470,12 +470,20 @@ shr-generic (shr-insert sub) (shr-descend sub)))) +(defun shr-indirect-call (tag-name dom &rest args) + (let ((function (intern (concat "shr-tag-" (symbol-name tag-name)) obarray)) + ;; Allow other packages to override (or provide) rendering + ;; of elements. + (external (cdr (assq tag-name shr-external-rendering-functions)))) + (cond (external + (apply external dom args)) + ((fboundp function) + (apply function dom args)) + (t + (apply 'shr-generic dom args))))) + (defun shr-descend (dom) - (let ((function - (intern (concat "shr-tag-" (symbol-name (dom-tag dom))) obarray)) - ;; Allow other packages to override (or provide) rendering - ;; of elements. - (external (cdr (assq (dom-tag dom) shr-external-rendering-functions))) + (let ((tag-name (dom-tag dom)) (style (dom-attr dom 'style)) (shr-stylesheet shr-stylesheet) (shr-depth (1+ shr-depth)) @@ -490,12 +498,7 @@ shr-descend (setq style nil))) ;; If we have a display:none, then just ignore this part of the DOM. (unless (equal (cdr (assq 'display shr-stylesheet)) "none") - (cond (external - (funcall external dom)) - ((fboundp function) - (funcall function dom)) - (t - (shr-generic dom))) + (shr-indirect-call tag-name dom) (when (and shr-target-id (equal (dom-attr dom 'id) shr-target-id)) ;; If the element was empty, we don't have anything to put the @@ -1404,7 +1407,7 @@ shr-tag-object (when url (cond (image - (shr-tag-img dom url) + (shr-indirect-call 'img dom url) (setq dom nil)) (multimedia (shr-insert " [multimedia] ") @@ -1469,7 +1472,7 @@ shr-tag-video (unless url (setq url (car (shr--extract-best-source dom)))) (if (> (length image) 0) - (shr-tag-img nil image) + (shr-indirect-call 'img nil image) (shr-insert " [video] ")) (shr-urlify start (shr-expand-url url)))) @@ -1964,9 +1967,9 @@ shr-collect-extra-strings-in-table do (setq tag (dom-tag child)) and unless (memq tag '(comment style)) if (eq tag 'img) - do (shr-tag-img child) + do (shr-indirect-call 'img child) else if (eq tag 'object) - do (shr-tag-object child) + do (shr-indirect-call 'object child) else do (setq recurse t) and if (eq tag 'tr) @@ -1980,7 +1983,7 @@ shr-collect-extra-strings-in-table do (setq flags nil) else if (car flags) do (setq recurse nil) - (shr-tag-table child) + (shr-indirect-call 'table child) end end end end end end end end end end when recurse append (shr-collect-extra-strings-in-table child flags))) -- 2.14.1 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* bug#28402: 25.2; shr.el uses shr-tag-img despite set shr-external-rendering-functions 2017-09-24 13:10 ` Vasilij Schneidermann @ 2017-09-29 7:31 ` Eli Zaretskii 2017-10-05 10:02 ` Eli Zaretskii 2017-10-05 10:18 ` Lars Ingebrigtsen 0 siblings, 2 replies; 18+ messages in thread From: Eli Zaretskii @ 2017-09-29 7:31 UTC (permalink / raw) To: Vasilij Schneidermann; +Cc: larsi, 28402 > Date: Sun, 24 Sep 2017 15:10:50 +0200 > From: Vasilij Schneidermann <v.schneidermann@gmail.com> > Cc: 28402@debbugs.gnu.org, larsi@gnus.org > > > Fair enough. Fortunately, it sounds like Lars have just agreed to > > that. > > Alright, I finally got to creating a patch for this. Thanks, this LGTM. Lars, any comments? If no objections are voiced in a few days, I will push to the emacs-26 branch. ^ permalink raw reply [flat|nested] 18+ messages in thread
* bug#28402: 25.2; shr.el uses shr-tag-img despite set shr-external-rendering-functions 2017-09-29 7:31 ` Eli Zaretskii @ 2017-10-05 10:02 ` Eli Zaretskii 2017-10-05 10:18 ` Lars Ingebrigtsen 1 sibling, 0 replies; 18+ messages in thread From: Eli Zaretskii @ 2017-10-05 10:02 UTC (permalink / raw) To: v.schneidermann, larsi; +Cc: 28402-done > Date: Fri, 29 Sep 2017 10:31:15 +0300 > From: Eli Zaretskii <eliz@gnu.org> > Cc: larsi@gnus.org, 28402@debbugs.gnu.org > > If no objections are voiced in a few days, I will push to the emacs-26 > branch. Done. ^ permalink raw reply [flat|nested] 18+ messages in thread
* bug#28402: 25.2; shr.el uses shr-tag-img despite set shr-external-rendering-functions 2017-09-29 7:31 ` Eli Zaretskii 2017-10-05 10:02 ` Eli Zaretskii @ 2017-10-05 10:18 ` Lars Ingebrigtsen 2017-10-05 11:01 ` Eli Zaretskii 1 sibling, 1 reply; 18+ messages in thread From: Lars Ingebrigtsen @ 2017-10-05 10:18 UTC (permalink / raw) To: Eli Zaretskii; +Cc: Vasilij Schneidermann, 28402 Eli Zaretskii <eliz@gnu.org> writes: > If no objections are voiced in a few days, I will push to the emacs-26 > branch. Sorry; I didn't have a look at this before you applied. It mostly looks fine, but this bit isn't: (defun shr-descend (dom) - (let ((function - (intern (concat "shr-tag-" (symbol-name (dom-tag dom))) obarray)) - ;; Allow other packages to override (or provide) rendering - ;; of elements. - (external (cdr (assq (dom-tag dom) shr-external-rendering-functions))) + (let ((tag-name (dom-tag dom)) (style (dom-attr dom 'style)) (shr-stylesheet shr-stylesheet) (shr-depth (1+ shr-depth)) @@ -490,12 +498,7 @@ shr-descend (setq style nil))) ;; If we have a display:none, then just ignore this part of the DOM. (unless (equal (cdr (assq 'display shr-stylesheet)) "none") - (cond (external - (funcall external dom)) - ((fboundp function) - (funcall function dom)) - (t - (shr-generic dom))) + (shr-indirect-call tag-name dom) shr rendering of deep HTML structures uses a lot of stack, and we see this in practice sometimes, where shr refuses to render HTML because it's too deeply nested (and runs into the Emacs max-depth stack thing). This indirect call will make the stack 30% deeper, I think? As well as slower, since it's an extra funcall for each and every HTML node. So this part should be reverted. -- (domestic pets only, the antidote for overdose, milk.) bloggy blog: http://lars.ingebrigtsen.no ^ permalink raw reply [flat|nested] 18+ messages in thread
* bug#28402: 25.2; shr.el uses shr-tag-img despite set shr-external-rendering-functions 2017-10-05 10:18 ` Lars Ingebrigtsen @ 2017-10-05 11:01 ` Eli Zaretskii 2017-10-05 11:18 ` Lars Ingebrigtsen 0 siblings, 1 reply; 18+ messages in thread From: Eli Zaretskii @ 2017-10-05 11:01 UTC (permalink / raw) To: Lars Ingebrigtsen; +Cc: v.schneidermann, 28402 > From: Lars Ingebrigtsen <larsi@gnus.org> > Cc: Vasilij Schneidermann <v.schneidermann@gmail.com>, 28402@debbugs.gnu.org > Date: Thu, 05 Oct 2017 12:18:40 +0200 > > (defun shr-descend (dom) > - (let ((function > - (intern (concat "shr-tag-" (symbol-name (dom-tag dom))) obarray)) > - ;; Allow other packages to override (or provide) rendering > - ;; of elements. > - (external (cdr (assq (dom-tag dom) shr-external-rendering-functions))) > + (let ((tag-name (dom-tag dom)) > (style (dom-attr dom 'style)) > (shr-stylesheet shr-stylesheet) > (shr-depth (1+ shr-depth)) > @@ -490,12 +498,7 @@ shr-descend > (setq style nil))) > ;; If we have a display:none, then just ignore this part of the DOM. > (unless (equal (cdr (assq 'display shr-stylesheet)) "none") > - (cond (external > - (funcall external dom)) > - ((fboundp function) > - (funcall function dom)) > - (t > - (shr-generic dom))) > + (shr-indirect-call tag-name dom) > > shr rendering of deep HTML structures uses a lot of stack, and we see > this in practice sometimes, where shr refuses to render HTML because > it's too deeply nested (and runs into the Emacs max-depth stack thing). > > This indirect call will make the stack 30% deeper, I think? As well as > slower, since it's an extra funcall for each and every HTML node. > > So this part should be reverted. Wouldn't reverting it make the entire change rather pointless? If the issue is with stack usage, we could increase the stack when shr-descend is called. As for speed, the old code had such an indirection as well, albeit via funcall, no? So I don't think I understand the nature of your objections. ^ permalink raw reply [flat|nested] 18+ messages in thread
* bug#28402: 25.2; shr.el uses shr-tag-img despite set shr-external-rendering-functions 2017-10-05 11:01 ` Eli Zaretskii @ 2017-10-05 11:18 ` Lars Ingebrigtsen 2017-10-05 13:14 ` Eli Zaretskii 0 siblings, 1 reply; 18+ messages in thread From: Lars Ingebrigtsen @ 2017-10-05 11:18 UTC (permalink / raw) To: Eli Zaretskii; +Cc: v.schneidermann, 28402 Eli Zaretskii <eliz@gnu.org> writes: >> - (cond (external >> - (funcall external dom)) >> - ((fboundp function) >> - (funcall function dom)) >> - (t >> - (shr-generic dom))) >> + (shr-indirect-call tag-name dom) [...] > Wouldn't reverting it make the entire change rather pointless? No, the other calls to shr-indirect are fine, and was what was missing. > If the issue is with stack usage, we could increase the stack when > shr-descend is called. As for speed, the old code had such an > indirection as well, albeit via funcall, no? The old call calls virtually always ends up with (funcall function dom) The new code calls shr-indirect-call which will will then (funcall function dom) So one extra function call for each node. -- (domestic pets only, the antidote for overdose, milk.) bloggy blog: http://lars.ingebrigtsen.no ^ permalink raw reply [flat|nested] 18+ messages in thread
* bug#28402: 25.2; shr.el uses shr-tag-img despite set shr-external-rendering-functions 2017-10-05 11:18 ` Lars Ingebrigtsen @ 2017-10-05 13:14 ` Eli Zaretskii 2017-10-05 13:22 ` Lars Ingebrigtsen 0 siblings, 1 reply; 18+ messages in thread From: Eli Zaretskii @ 2017-10-05 13:14 UTC (permalink / raw) To: Lars Ingebrigtsen; +Cc: v.schneidermann, 28402 > From: Lars Ingebrigtsen <larsi@gnus.org> > Cc: v.schneidermann@gmail.com, 28402@debbugs.gnu.org > Date: Thu, 05 Oct 2017 13:18:37 +0200 > > The old call calls virtually always ends up with > > (funcall function dom) > > The new code calls shr-indirect-call which will will then > > (funcall function dom) > > So one extra function call for each node. What if we make shr-indirect-call a defsubst? Would that take care of this issue? ^ permalink raw reply [flat|nested] 18+ messages in thread
* bug#28402: 25.2; shr.el uses shr-tag-img despite set shr-external-rendering-functions 2017-10-05 13:14 ` Eli Zaretskii @ 2017-10-05 13:22 ` Lars Ingebrigtsen 2017-10-05 13:44 ` Eli Zaretskii 0 siblings, 1 reply; 18+ messages in thread From: Lars Ingebrigtsen @ 2017-10-05 13:22 UTC (permalink / raw) To: Eli Zaretskii; +Cc: v.schneidermann, 28402 Eli Zaretskii <eliz@gnu.org> writes: > What if we make shr-indirect-call a defsubst? Would that take care of > this issue? Isn't it still (at least) an extra stack frame with a bunch of variable bindings? This function is the central bit in shr, and should be as fast as possible. -- (domestic pets only, the antidote for overdose, milk.) bloggy blog: http://lars.ingebrigtsen.no ^ permalink raw reply [flat|nested] 18+ messages in thread
* bug#28402: 25.2; shr.el uses shr-tag-img despite set shr-external-rendering-functions 2017-10-05 13:22 ` Lars Ingebrigtsen @ 2017-10-05 13:44 ` Eli Zaretskii 2017-10-05 13:52 ` Lars Ingebrigtsen 0 siblings, 1 reply; 18+ messages in thread From: Eli Zaretskii @ 2017-10-05 13:44 UTC (permalink / raw) To: Lars Ingebrigtsen; +Cc: v.schneidermann, 28402 > From: Lars Ingebrigtsen <larsi@gnus.org> > Cc: v.schneidermann@gmail.com, 28402@debbugs.gnu.org > Date: Thu, 05 Oct 2017 15:22:46 +0200 > > Eli Zaretskii <eliz@gnu.org> writes: > > > What if we make shr-indirect-call a defsubst? Would that take care of > > this issue? > > Isn't it still (at least) an extra stack frame with a bunch of variable > bindings? Yes, but is that really that significant? You sound like saying that any non-trivial change in shr-descend should be rejected for these reasons. Is that really so? Do we have measurements that would back up such extreme care? > This function is the central bit in shr, and should be as fast as > possible. As fast as possible, but not faster, I presume ;-) Anyway, I see your point. Vasilij, any thoughts or suggestions to avoid reverting that part, while still keeping Lars happy? ^ permalink raw reply [flat|nested] 18+ messages in thread
* bug#28402: 25.2; shr.el uses shr-tag-img despite set shr-external-rendering-functions 2017-10-05 13:44 ` Eli Zaretskii @ 2017-10-05 13:52 ` Lars Ingebrigtsen 2017-10-05 20:08 ` Vasilij Schneidermann 0 siblings, 1 reply; 18+ messages in thread From: Lars Ingebrigtsen @ 2017-10-05 13:52 UTC (permalink / raw) To: Eli Zaretskii; +Cc: v.schneidermann, 28402 Eli Zaretskii <eliz@gnu.org> writes: > Yes, but is that really that significant? You sound like saying that > any non-trivial change in shr-descend should be rejected for these > reasons. Is that really so? Do we have measurements that would back > up such extreme care? No, I'm just saying that any additional features added to that function should be considered carefully (to see whether that added feature is worth the performance degradation). And I don't think we should do anything to "pretty it up" just because, if that has any performance impact at all. In a thing like shr, it's really the case of a death by a thousand cuts: Each single improvement adds a slight performance hit, and then after a couple of years you end up with something that's pretty, but completely unusable. (It's already too slow as it is.) So I protect `shr-descend' fiercely. :-) (But late, as always... Sorry for not saying this before you applied the patch; it would have been less work for all of us.) -- (domestic pets only, the antidote for overdose, milk.) bloggy blog: http://lars.ingebrigtsen.no ^ permalink raw reply [flat|nested] 18+ messages in thread
* bug#28402: 25.2; shr.el uses shr-tag-img despite set shr-external-rendering-functions 2017-10-05 13:52 ` Lars Ingebrigtsen @ 2017-10-05 20:08 ` Vasilij Schneidermann 2017-10-05 20:10 ` Lars Ingebrigtsen 2017-10-06 12:43 ` Eli Zaretskii 0 siblings, 2 replies; 18+ messages in thread From: Vasilij Schneidermann @ 2017-10-05 20:08 UTC (permalink / raw) To: Lars Ingebrigtsen; +Cc: 28402 > In a thing like shr, it's really the case of a death by a thousand cuts: > Each single improvement adds a slight performance hit, and then after a > couple of years you end up with something that's pretty, but completely > unusable. (It's already too slow as it is.) So I protect `shr-descend' > fiercely. :-) How deeply nested does typical HTML get anyways? I recall an exemplary comment from an article on browser rendering that pointed out that they bail out on degenerate cases, so perhaps it's better to add that to shr rather than hoping you never hit the stack limit by following a conservative coding style. Regarding speed, I only notice it being an issue for documents with tables, everything else is fine. > (But late, as always... Sorry for not saying this before you applied > the patch; it would have been less work for all of us.) I can imagine other variations of defsubst (such as defmacro and define-inline), but it would indeed be the easiest to roll back that part of the code. What I'd additionally do is adding a comment explaining why there's two very similar snippets of code doing the same thing, otherwise the next person proposing a patch will change only one of them and be left head-scratching why it doesn't quite work. ^ permalink raw reply [flat|nested] 18+ messages in thread
* bug#28402: 25.2; shr.el uses shr-tag-img despite set shr-external-rendering-functions 2017-10-05 20:08 ` Vasilij Schneidermann @ 2017-10-05 20:10 ` Lars Ingebrigtsen 2017-10-06 12:43 ` Eli Zaretskii 1 sibling, 0 replies; 18+ messages in thread From: Lars Ingebrigtsen @ 2017-10-05 20:10 UTC (permalink / raw) To: Vasilij Schneidermann; +Cc: 28402 Vasilij Schneidermann <v.schneidermann@gmail.com> writes: > How deeply nested does typical HTML get anyways? Very. > I recall an exemplary comment from an article on browser rendering > that pointed out that they bail out on degenerate cases, so perhaps > it's better to add that to shr rather than hoping you never hit the > stack limit by following a conservative coding style. shr does check and bail. Search for max-specpdl-size in shr.el. > What I'd additionally do is adding a comment explaining why there's > two very similar snippets of code doing the same thing, otherwise the > next person proposing a patch will change only one of them and be left > head-scratching why it doesn't quite work. Sure; please submit a patch to do so. -- (domestic pets only, the antidote for overdose, milk.) bloggy blog: http://lars.ingebrigtsen.no ^ permalink raw reply [flat|nested] 18+ messages in thread
* bug#28402: 25.2; shr.el uses shr-tag-img despite set shr-external-rendering-functions 2017-10-05 20:08 ` Vasilij Schneidermann 2017-10-05 20:10 ` Lars Ingebrigtsen @ 2017-10-06 12:43 ` Eli Zaretskii 1 sibling, 0 replies; 18+ messages in thread From: Eli Zaretskii @ 2017-10-06 12:43 UTC (permalink / raw) To: Vasilij Schneidermann; +Cc: larsi, 28402 > Date: Thu, 5 Oct 2017 22:08:07 +0200 > From: Vasilij Schneidermann <v.schneidermann@gmail.com> > Cc: Eli Zaretskii <eliz@gnu.org>, 28402@debbugs.gnu.org > > I can imagine other variations of defsubst (such as defmacro and > define-inline), but it would indeed be the easiest to roll back that > part of the code. What I'd additionally do is adding a comment > explaining why there's two very similar snippets of code doing the same > thing, otherwise the next person proposing a patch will change only one > of them and be left head-scratching why it doesn't quite work. Thanks, done. ^ permalink raw reply [flat|nested] 18+ messages in thread
* bug#28402: 25.2; shr.el uses shr-tag-img despite set shr-external-rendering-functions 2017-09-09 19:39 bug#28402: 25.2; shr.el uses shr-tag-img despite set shr-external-rendering-functions Vasilij Schneidermann 2017-09-11 16:04 ` Eli Zaretskii @ 2017-09-13 17:27 ` Lars Ingebrigtsen 1 sibling, 0 replies; 18+ messages in thread From: Lars Ingebrigtsen @ 2017-09-13 17:27 UTC (permalink / raw) To: Vasilij Schneidermann; +Cc: 28402 Vasilij Schneidermann <v.schneidermann@gmail.com> writes: > I'm using shr.el for a package and override `shr-tag-img` by let-binding > `shr-external-rendering-functions` to an alist containing an alternative > rendering function that deals better with local images. Despite this > `shr-tag-img` is still directly called when rendering tables with images > inside them. I think it would be nice to alter those direct calls to heed `shr-external-rendering-functions'. Doesn't seem to be much potential for performance degradation that I can see... Perhaps introduce a helper function like `(shr-indirect-call 'img child)' or something... -- (domestic pets only, the antidote for overdose, milk.) bloggy blog: http://lars.ingebrigtsen.no ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2017-10-06 12:43 UTC | newest] Thread overview: 18+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-09-09 19:39 bug#28402: 25.2; shr.el uses shr-tag-img despite set shr-external-rendering-functions Vasilij Schneidermann 2017-09-11 16:04 ` Eli Zaretskii 2017-09-13 17:22 ` Vasilij Schneidermann 2017-09-13 18:37 ` Eli Zaretskii 2017-09-24 13:10 ` Vasilij Schneidermann 2017-09-29 7:31 ` Eli Zaretskii 2017-10-05 10:02 ` Eli Zaretskii 2017-10-05 10:18 ` Lars Ingebrigtsen 2017-10-05 11:01 ` Eli Zaretskii 2017-10-05 11:18 ` Lars Ingebrigtsen 2017-10-05 13:14 ` Eli Zaretskii 2017-10-05 13:22 ` Lars Ingebrigtsen 2017-10-05 13:44 ` Eli Zaretskii 2017-10-05 13:52 ` Lars Ingebrigtsen 2017-10-05 20:08 ` Vasilij Schneidermann 2017-10-05 20:10 ` Lars Ingebrigtsen 2017-10-06 12:43 ` Eli Zaretskii 2017-09-13 17:27 ` Lars Ingebrigtsen
Code repositories for project(s) associated with this public inbox https://git.savannah.gnu.org/cgit/emacs.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).