From mboxrd@z Thu Jan 1 00:00:00 1970 From: Nicolas Goaziou Subject: Re: [PATCH] ox-latex: provide width and height options for images Date: Wed, 27 Feb 2013 09:23:44 +0100 Message-ID: <87621etban.fsf@gmail.com> References: <87obf63mq6.fsf@gmail.com> <1361906554-26709-1-git-send-email-aaronecay@gmail.com> Mime-Version: 1.0 Content-Type: text/plain Return-path: Received: from eggs.gnu.org ([208.118.235.92]:47631) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UAcIx-0007Pj-TJ for emacs-orgmode@gnu.org; Wed, 27 Feb 2013 03:24:01 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UAcIw-0006Eu-Ie for emacs-orgmode@gnu.org; Wed, 27 Feb 2013 03:23:59 -0500 Received: from mail-wg0-f46.google.com ([74.125.82.46]:38724) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UAcIw-0006Ej-CE for emacs-orgmode@gnu.org; Wed, 27 Feb 2013 03:23:58 -0500 Received: by mail-wg0-f46.google.com with SMTP id fg15so219438wgb.1 for ; Wed, 27 Feb 2013 00:23:57 -0800 (PST) In-Reply-To: <1361906554-26709-1-git-send-email-aaronecay@gmail.com> (Aaron Ecay's message of "Tue, 26 Feb 2013 14:22:34 -0500") List-Id: "General discussions about Org-mode." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-orgmode-bounces+geo-emacs-orgmode=m.gmane.org@gnu.org Sender: emacs-orgmode-bounces+geo-emacs-orgmode=m.gmane.org@gnu.org To: Aaron Ecay Cc: emacs-orgmode@gnu.org Hello, Aaron Ecay 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