Hello,
Rasmus <ras...@gmx.us> 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
