emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
From: Nicolas Goaziou <mail@nicolasgoaziou.fr>
To: Rasmus <rasmus@gmx.us>
Cc: emacs-orgmode@gnu.org
Subject: Re: [bug?, org-element] latex-environment delimiters must be at BOL
Date: Tue, 22 Jul 2014 09:54:09 +0200	[thread overview]
Message-ID: <87ha29g76m.fsf@nicolasgoaziou.fr> (raw)
In-Reply-To: <87egxf12n7.fsf@gmx.us> (rasmus@gmx.us's message of "Mon, 21 Jul 2014 11:29:00 +0200")

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

  reply	other threads:[~2014-07-22  7:53 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-07-10 11:54 [bug?, org-element] latex-environment delimiters must be at BOL Rasmus
2014-07-16 13:04 ` Nicolas Goaziou
2014-07-18 15:49   ` Rasmus
2014-07-20 17:49     ` Nicolas Goaziou
2014-07-21  9:29       ` Rasmus
2014-07-22  7:54         ` Nicolas Goaziou [this message]
2014-07-22  9:08           ` Rasmus
2014-07-22 10:03             ` Nicolas Goaziou
2014-07-22 13:28               ` Rasmus
2014-07-23  8:29                 ` Nicolas Goaziou

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.orgmode.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87ha29g76m.fsf@nicolasgoaziou.fr \
    --to=mail@nicolasgoaziou.fr \
    --cc=emacs-orgmode@gnu.org \
    --cc=rasmus@gmx.us \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).