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

[Nicolas Goaziou <mail@nicolasgoaziou.fr>]

Hello,

Rasmus <rasmus@gmx.us> writes:

>> Since environments do not necessary start with \begin{...}, I think the
>> following is better
>>
>>   (and (string-match ...)
>>        (match-string ...))
>
> Don't the element `latex-environment' always start \begin{.}?
> Cf. org-element--latex-begin-environment.

At the moment, they do, but I have a patch somewhere to make "\[...\]"
an element instead of an object (e.g., to avoid filling it). It was
accepted, but it some similar change could happen. I'm just suggesting
to make it more robust right from the start.

> Another danger is that someone writes something like,
>
>     \begin{center}
>     \begin{table}
>     ...

You can start the regexp with "\\`[ \t]*..."

> Longtable in an omission, but would have been matched. For some reason
> I thought tabu was no longer supported. Browsing the tabu CTAN page
> there’s links to all sorts of exotic table packages, like "stabular",
> "bigtabular", and "supertabular". I wonder if it’s enough to match the
> ones supported by Org by default, or whether it would be better to
> have it as a defvar that could be hacked if necessary?

tabu/longtabu are supported by Org by default. We could make the list
a defconst anyway.

> I have attached the patch with the changes again and added a changelog
> entry.  I am not sure this needs to be documented, but I’m happy to
> include a couple lines somewhere.

I think the entry in ORG-NEWS is enough.

> +(defun org-latex--environment-type (latex-environment)
> +  "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)))
> +	 (env (or (and (string-match "\\\\begin{\\([A-Za-z0-9*]+\\)}" value)
> +		       (match-string 1 value)) "")))

For clarity, "" should be indented at the same level as (and ...).

> +    (cond
> +     ((string-match-p org-latex-math-environments-re value) 'math)
> +     ((string-match-p
> +       (regexp-opt '("table" "longtable" "tabular" "tabu" "longtabu")) env)

Nitpick: you could wrap the above into `eval-when-compile' for a small
optimization.

> +      'table)
> +     ((string-match-p "figure" env) 'image)
> +     ((or (string-match-p "\\(\\(lst\\)?listing\\|verbatim\\|minted\\)" env)
> +	  (string-match-p
> +	   (regexp-opt
> +	    (mapcar (lambda (str)
> +		      (let ((s (cadr str)))
> +			(if (string-match latex-begin-re s)

You removed `latex-begin-re' binding so this is going to fail.

> +			    (match-string 2 s)
> +			  s)))
> +		    org-latex-custom-lang-environments))

I'm not sure it is necessary. AFAIU,
`org-latex-custom-lang-environments' already provides a way to insert
captions.  The matching process above seems fragile.

Thank you.

Regards,

-- 
Nicolas Goaziou