[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [O] [PATCH] ox-latex: provide width and height options for images
From: |
Nicolas Goaziou |
Subject: |
Re: [O] [PATCH] ox-latex: provide width and height options for images |
Date: |
Wed, 27 Feb 2013 09:23:44 +0100 |
Hello,
Aaron Ecay <address@hidden> writes:
Thank you for your patch. Here are a few comments.
> These are implemented with \resizebox, and thus are uniform across
> different types of image inclusion (\includegraphics, \input of tikz
> images). This differs from the older way of using width and height
> optional args to \includegraphics.
I tend to agree with Rasmus. It would be better to keep height and width
options in \includegraphics when possible.
> Thus, the default value for org-latex-image-default-options is left
> untouched, to avoid breaking compatibility with older code. After a
> transition period, the 0.9\linewidth value should be moved into
> org-latex-image-default-width, and the -options variable set to the
> empty string.
We don't need this precaution. The exporter code for 8.0 introduced many
incompatibilities already. Also, this one is easy to discover.
> +(defun org-not-nil-or-empty (v)
> + "Return V if V is not nil, the string \"nil\", or a string
> +consisting of solely whitespace. Otherwise return nil."
> + (and (org-not-nil v)
> + (org-string-nw-p v)
> + v))
I'm not sure it's worth creating a new function for it. Anyway, the
first line of a docstring should be a sentence on its own.
> (defcustom org-latex-image-default-option "width=.9\\linewidth"
> "Default option for images."
> :group 'org-export-latex
> :type 'string)
We can set it to "".
> +(defcustom org-latex-image-default-width ""
> + "Default option for images."
> + :group 'org-export-latex
> + :type 'string)
> +
> +(defcustom org-latex-image-default-height ""
> + "Default option for images."
> + :group 'org-export-latex
> + :type 'string)
I think it's a good step forward. It will need to be documented in the
comments at the beginning of ox-latex.el, where all attributes
properties relative to different syntactical elements are explained.
> (defcustom org-latex-default-figure-position "htb"
> "Default position for latex figures."
> :group 'org-export-latex
> @@ -1755,6 +1768,15 @@ used as a communication channel."
> (format "[%s]" org-latex-default-figure-position))
> (t ""))))
> (comment-include (if (plist-get attr :comment-include) "%" ""))
> + ;; It is possible to specify width and height in the
> + ;; ATTR_LATEX line, and also via default variables.
> + (width (format "%s" (or (plist-get attr :width)
> + org-latex-image-default-width)))
> + (height (format "%s" (or (plist-get attr :height)
> + org-latex-image-default-height)))
> + (resize (format "\\resizebox{%s}{%s}{%%s}"
> + (if (org-not-nil-or-empty width) width "!")
> + (if (org-not-nil-or-empty height) height "!")))
Here, you can obtain \resizebox{!}{!}{%s}, which is wrong, isn't it?
> ;; Options for "includegraphics" macro. Make sure it is
> ;; a string with square brackets when non empty. Default to
> ;; `org-latex-image-default-option' when possible.
> @@ -1766,9 +1788,10 @@ used as a communication channel."
> ((eq float 'float) "[width=0.7\\textwidth]")
> ((eq float 'wrap) "[width=0.48\\textwidth]")
> (t ""))))
This needs to be changed as these options would interfere with :width
argument. For example, (eq float 'float) could set :width property if it
is undefined. Obviously, this means the check has to be done before
WIDTH and HEIGHT strings are built.
> - (image-code (if (equal filetype "tikz")
> - (format "\\input{%s}" path)
> - (format "\\includegraphics%s{%s}" options path))))
> + (image-code (format resize
> + (if (equal filetype "tikz")
> + (format "\\input{%s}" path)
> + (format "\\includegraphics%s{%s}" options
> path)))))
See comments above.
Thank you again,
Regards,
--
Nicolas Goaziou
- Re: [O] [PATCH] ob-R.el, ox-latex.el: support for tikz graphics, (continued)
- Re: [O] [PATCH] ob-R.el, ox-latex.el: support for tikz graphics, Nicolas Goaziou, 2013/02/25
- Re: [O] [PATCH] ob-R.el, ox-latex.el: support for tikz graphics, Achim Gratz, 2013/02/26
- Re: [O] [PATCH] ob-R.el, ox-latex.el: support for tikz graphics, Myles English, 2013/02/26
- Re: [O] [PATCH] ob-R.el, ox-latex.el: support for tikz graphics, Nicolas Goaziou, 2013/02/26
- Re: [O] [PATCH] ob-R.el, ox-latex.el: support for tikz graphics, Achim Gratz, 2013/02/26
- Re: [O] [PATCH] ob-R.el, ox-latex.el: support for tikz graphics, Aaron Ecay, 2013/02/26
- [O] [PATCH] ox-latex: provide width and height options for images, Aaron Ecay, 2013/02/26
- Re: [O] [PATCH] ox-latex: provide width and height options for images, Rasmus, 2013/02/26
- Re: [O] [PATCH] ox-latex: provide width and height options for images, Aaron Ecay, 2013/02/26
- Re: [O] [PATCH] ox-latex: provide width and height options for images, Achim Gratz, 2013/02/27
- Re: [O] [PATCH] ox-latex: provide width and height options for images,
Nicolas Goaziou <=
Re: [O] [PATCH] ob-R.el, ox-latex.el: support for tikz graphics, Achim Gratz, 2013/02/26