Subject: Re: bug#28402: 25.2; shr.el uses shr-tag-img despite set shr-external-rendering-functions

From: Eli Zaretskii <[email-masked]>
Date: Thu, 05 Oct 2017 14:01:33 +0300

> From: Lars Ingebrigtsen <[email-masked]>
> Cc: Vasilij Schneidermann <[email-masked]>,  [email-masked]
> 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.