Re: [O] [patch, ox-latex] captions and latex-environments

[Nicolas Goaziou]

Hello,

Rasmus <address@hidden> writes:

> The patch looks a bit dodgy, maybe because I used magit, which I don’t
> 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,

-- 
Nicolas Goaziou