Re: [bug?, org-element] latex-environment delimiters must be at BOL

[Nicolas Goaziou <mail@nicolasgoaziou.fr>]

Hello,

Rasmus <rasmus@gmx.us> writes:

Thanks for your patch.

> Turns out that I also had to modify the regexp in
> org-element--current-element as this kicks off
> org-element-latex-environment-parser (correct me if I am wrong).

Correct.

> In the patch I define new regexps and use them everywhere.
> Org-element.el seems to rather use spelled-out regexps.  If desirable,
> I can use this practice rather than defining new regexps.

IMO, the way to go would be to define regexps as constants everywhere in
"org-element.el", but that would introduce duplicates with some
constants in "org.el". IOW, there's no clear design about it. Situation
may change when the parser gets more integrated into core.

Meanwhile, it can't hurt to use constants in "org-element.el".

> [BTW: Is there a way to test performance systematically?]

I don't think so.

> The change in org-element-paragraph-parser, namely using the mentioned
> regexp, may be redundant.

That's fine by me.

> This patch does not change the manual as I did not find any direct
> discussion of newlines and LaTeX environments.

OK.

> From f21ae57759ca5a1d4f19424c4eb0be8db4dcbfb5 Mon Sep 17 00:00:00 2001
> From: Rasmus <w530@pank.eu>
> Date: Fri, 18 Jul 2014 17:01:53 +0200
> Subject: [PATCH] org-element.el: allow one-line LaTeX environments

"Allow"

> * org-element.el (org-elment--latex-begin-environment,
> org-element--latex-end-environment): New regexps identifying begining
> and ending of LaTeX environment.

"beginning"

> (org-element-latex-environment-parser, org-element-paragraph-parser,
> org-element--current-element): use org-elment--latex-begin-environment
> and org-element--latex-end-environment

"Use" and a full stop. Also, "elment" -> "element"

> * test-org-element.el (test-org-element/latex-environment-parser):
> test for one-line LaTeX environments.
> (test-org-element/latex-environment-parser): test different start tag
> and end tag.

"Add tests." is sufficient.

>  ;;;; Latex Environment
> +(defconst org-elment--latex-begin-environment
> +  "[ \t]*\\\\begin{\\([A-Za-z0-9*]+\\)}\\(\\[.*?\\]\\|{.*?}\\)*[ \t]*?"
> +  "Holds regexp matching beginning of LaTeX environments.")

There should be a blank line below "Latex Environment". Also, the
docstring should be "Regexp matching...".

The regexp is wrong. It must start at the beginning of line. Also, you
can skip the optional part since you don't intend to analyze it.

  "^[ \t]*\\\\begin{\\([A-Za-z0-9*]+\\)}"

Also, "elment" -> "element".

> +
> +(defconst org-element--latex-end-environment
> +  "[ \t]*\\\\end{%s}[ \t]*"
> +  "Holds regexp matching ending of LaTeX environments.")

Likewise, the end of line is mandatory here, and the optional
whitespaces at the beginning of line not necessary.

  "\\\\end{%s}[ \t]*$"

Note that it isn't a regexp per se, but a format string used to build
a regexp. Maybe the docstring should be explicit about this.

>      (let ((case-fold-search t)
>  	  (code-begin (point)))
> -      (looking-at "[ \t]*\\\\begin{\\([A-Za-z0-9]+\\*?\\)}")
> -      (if (not (re-search-forward (format "^[ \t]*\\\\end{%s}[ \t]*$"
> +      (looking-at org-elment--latex-begin-environment)

"elment" -> "element"

>  			 ;; Stop at valid latex environments.
> -			 (and (looking-at
> -			       "[ \t]*\\\\begin{\\([A-Za-z0-9]+\\*?\\)}")
> +			 (and (looking-at org-elment--latex-begin-environment)

"elment" -> "element"

> -	     ((looking-at
> -	       "[ \t]*\\\\begin{[A-Za-z0-9*]+}\\(\\[.*?\\]\\|{.*?}\\)*[ \t]*$")
> +	     ((looking-at org-elment--latex-begin-environment)

"elment" -> "element"


Regards,

-- 
Nicolas Goaziou