Re: Patch: default CSS class for SVG images in HTML export

[Nicolas Goaziou <mail@nicolasgoaziou.fr>]

Hello,

Jarmo Hurri <jarmo.hurri@iki.fi> writes:

> The attached patch
> - adds a default CSS class org-svg to SVG images embedded in exported
>   HTML if no other class is set for the image using attr_html
> - documents the existence of this class.

Thank you.

> If this patch is ok,

It looks so, barring minor issues suggested below.

> then what is missing are the default CSS settings for the new class
> org-svg in constant org-html-style-default. I simply did not know what
> to put there. The manual says that this constant has basic settings
> for _all_ defined CSS entities. Either defaults need to be set for the
> new class, or the text in the manual needs to be changed. Or we have
> to accept that the manual is not logically coherent. :-)

The Manual is the Truth!

What setting would equivalent to the current one?

> @@ -11750,6 +11750,7 @@ div.footnotes       @r{footnote section headline}
>  p.footnote          @r{footnote definition paragraph, containing a footnote}
>  .footref            @r{a footnote reference number (always a <sup>)}
>  .footnum            @r{footnote number in footnote definition (always <sup>)}
> +.org-svg            @r{default class for a linked SVG image}

The manual would use @samp{svg}, but I think @file{.svg} would be more
accurate, per (info "(texinfo @file)").

>  (defun org-html--svg-image (source attributes info)
>    "Return \"object\" appropriate for embedding svg file SOURCE
> -with assoicated ATTRIBUTES. INFO is a plist used as a
> +with associated ATTRIBUTES. INFO is a plist used as a
>  communication channel.

The docstring was incorrect before your patch; this is a good time to
fix it. In particular:

- first line must stand on its own, i.e., it must be one or more
  complete sentences;

- Sentences have to be separated with two spaces.

Could you reword it?

>  The special attribute \"fallback\" can be used to specify a fallback
> -image file to use if the object embedding is not supported."
> +image file to use if the object embedding is not supported.
> +
> +CSS class \"org-svg\" is assigned as the class of the object
> +unless a different class is specified with an attribute."
>    (let ((fallback (plist-get attributes :fallback))
> +	(css-class (plist-get attributes :class))
>  	(attrs (org-html--make-attribute-string
>  		(plist-put attributes :fallback nil))))
> -  (format "<object type=\"image/svg+xml\" data=\"%s\" %s>\n%s</object>"
> -	  source attrs
> +  (format "<object type=\"image/svg+xml\" data=\"%s\" %s %s>\n%s</object>"
> +	  source
> +	  attrs
> +	  (if css-class
> +	      ""
> +	    "class=\"org-svg\"")

This would ignore ":class nil", which probably means no class at all.

You can do instead:

  (let ((fallback (plist-get attributes :fallback))
        (attrs (org-html--make-attribute-string
                (org-combine-plists
                 ;; Remove fallback attribute, which is not meant to
                 ;; appear directly in the attributes string, and
                 ;; provide a default class if none is set.
                 '(:class "org-class") attributes '(:fallback nil)))))
    (format ...))
  

Regards,

-- 
Nicolas Goaziou