From mboxrd@z Thu Jan 1 00:00:00 1970 From: Nicolas Goaziou Subject: Re: [patch, ox-latex] captions and latex-environments Date: Fri, 17 Mar 2017 08:22:44 +0100 Message-ID: <87tw6se5a3.fsf@nicolasgoaziou.fr> References: <87k27pv38z.fsf@gmx.us> <87bmt1v2xs.fsf@gmx.us> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:48795) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1comDp-0003aJ-KM for emacs-orgmode@gnu.org; Fri, 17 Mar 2017 03:22:50 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1comDo-0006Ea-Gk for emacs-orgmode@gnu.org; Fri, 17 Mar 2017 03:22:49 -0400 Received: from relay3-d.mail.gandi.net ([2001:4b98:c:538::195]:49761) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1comDo-0006EE-BH for emacs-orgmode@gnu.org; Fri, 17 Mar 2017 03:22:48 -0400 In-Reply-To: <87bmt1v2xs.fsf@gmx.us> (rasmus@gmx.us's message of "Thu, 16 Mar 2017 13:09:03 +0100") 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" To: Rasmus Cc: emacs-orgmode@gnu.org Hello, Rasmus writes: > The patch looks a bit dodgy, maybe because I used magit, which I don=E2= =80=99t > really understand, instead of the shell. I have attached it anew. Thank you. Some comments follow. > +(defun org-latex-environment--type (latex-environment) It should be `org-latex--environment-type'. > + "Return the TYPE of LATEX-ENVIRONMENT. > + > +The TYPE is determined from the actual latex environment, and > +could be a member of `org-latex-caption-above' or `math'." > + (let* ((value (org-remove-indentation > + (org-element-property :value latex-environment))) > + (latex-begin-re (cadr (assoc "begin" org-latex-regexps))) I'd rather avoid using `org-latex-regexps', which predates the parser. A hard-coded regexp is better. > + (env (progn (string-match latex-begin-re value) > + (match-string 2 value)))) Since environments do not necessary start with \begin{...}, I think the following is better (and (string-match ...) (match-string ...)) > + (cond > + ((string-match org-latex-math-environments-re value) 'math) > + ((string-match-p "tab\\(le\\|ular\\)" env) 'table) This is a bit sloppy. In particular, it doesn't match all table environments supported out of the box, e.g., "longtabu". Also, a list of strings, compiler into a regexp with `regexp-opt' may be better. > ;; Environment is labeled: label must be within the environment > ;; (otherwise, a reference pointing to that element will count > - ;; the section instead). > + ;; the section instead). Also insert caption if `latex-environment' Missing space after the full stop. > + ;; is not a math environment. > (with-temp-buffer > (insert value) > - (goto-char (point-min)) > - (forward-line) > - (insert (org-latex--label latex-environment info nil t)) > + (if caption-above-p > + (progn > + (goto-char (point-min)) > + (forward-line) > + (insert caption)) > + (goto-char (point-max)) > + (forward-line -1) > + (insert caption)) Nitpick: you can move (insert caption) outside the (if ...) and de-duplicate it. Regards, --=20 Nicolas Goaziou