emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
* [patch, ox-latex] captions and latex-environments
@ 2017-03-16 12:02 Rasmus
  2017-03-16 12:09 ` Rasmus
  0 siblings, 1 reply; 10+ messages in thread
From: Rasmus @ 2017-03-16 12:02 UTC (permalink / raw)
  To: emacs-orgmode

[-- Attachment #1: Type: text/plain, Size: 726 bytes --]

Hi,

I often generate tables directly from R.  At the moment, I have to do
something like the following because org-latex-latex-environment doesn’t
support captions.

    #+name: tbl:1
    #+caption: cap
    #+begin_table
    #+begin_center
    #+include: "tbl.tex"
    #+end_center
    #+end_table

This is a bit too verbose IMO.
With the attached patch this can be cut down to,

    #+name: tbl:1
    #+caption: cap
    #+include: "tbl.tex"

It should respect ‘org-latex-custom-lang-environments’ and
‘org-latex-caption-above’...

Would this be an appropriate change or is it too hackish?  I have not
looked at other backends, but I suspect this is mostly a latex thing.

Thanks,
Rasmus

-- 
When in doubt, do it!

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-ox-latex-Support-caption-for-latex-environment.patch --]
[-- Type: text/x-diff, Size: 4421 bytes --]

From 4304552a0c8a72c6aeb2805a8cf703eddb5da123 Mon Sep 17 00:00:00 2001
From: Rasmus <rasmus@gmx.us>
Date: Thu, 16 Mar 2017 12:45:10 +0100
Subject: [PATCH] ox-latex: Support caption for latex-environment

* lisp/ox-latex.el (org-latex-environment--type): New function
  determining type of a latex-environment.
  (org-latex-latex-environment): Add support for caption.
  (org-latex--caption/label-string): Use correct type for non-floating
  latex-environments.
---
 lisp/ox-latex.el | 78 ++++++++++++++++++++++++++++++++++++++++++++------------
 1 file changed, 62 insertions(+), 16 deletions(-)

diff --git a/lisp/ox-latex.el b/lisp/ox-latex.el
index 2727359cb..f226dc7ae 100644
--- a/lisp/ox-latex.el
+++ b/lisp/ox-latex.el
@@ -1312,14 +1312,19 @@ For non-floats, see `org-latex--wrap-label'."
      (t
       (format (if nonfloat "\\captionof{%s}%s{%s%s}\n"
 		"\\caption%s%s{%s%s}\n")
-	      (if nonfloat
-		  (cl-case type
-		    (paragraph "figure")
-		    (src-block (if (plist-get info :latex-listings)
-				   "listing"
-				 "figure"))
-		    (t (symbol-name type)))
-		"")
+	      (let ((type* (if (eq type 'latex-environment)
+			       (org-latex-environment--type element)
+			     type)))
+		(if nonfloat
+		    (cl-case type*
+		      (paragraph "figure")
+		      (image "figure")
+		      (special-block "figure")
+		      (src-block (if (plist-get info :latex-listings)
+				     "listing"
+				   "figure"))
+		      (t (symbol-name type*)))
+		  ""))
 	      (if short (format "[%s]" (org-export-data short info)) "")
 	      label
 	      (org-export-data main info))))))
@@ -2250,24 +2255,65 @@ CONTENTS is nil.  INFO is a plist holding contextual information."
 
 ;;;; Latex Environment
 
+(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)))
+	 (latex-begin-re (cadr (assoc "begin" org-latex-regexps)))
+	 (env (progn (string-match latex-begin-re value)
+		     (match-string 2 value))))
+    (cond
+     ((string-match org-latex-math-environments-re value) 'math)
+     ((string-match-p "tab\\(le\\|ular\\)" env) 'table)
+     ((string-match-p "figure" env) 'image)
+     ;; Default coding environments
+     ((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)
+			    (match-string 2 s)
+			  s)))
+		    org-latex-custom-lang-environments))
+	   env))
+      'src-block)
+     (t 'special-block))))
+
 (defun org-latex-latex-environment (latex-environment _contents info)
   "Transcode a LATEX-ENVIRONMENT element from Org to LaTeX.
 CONTENTS is nil.  INFO is a plist holding contextual information."
   (when (plist-get info :with-latex)
-    (let ((value (org-remove-indentation
-		  (org-element-property :value latex-environment))))
-      (if (not (org-element-property :name latex-environment)) value
+    (let* ((value (org-remove-indentation
+		   (org-element-property :value latex-environment)))
+	   (type (org-latex-environment--type latex-environment))
+	   (caption (if (eq type 'math)
+			(org-latex--label latex-environment info nil t)
+		      (org-latex--caption/label-string latex-environment info)))
+	   (caption-above-p
+	    (memq type (append (plist-get info :latex-caption-above) '(math)))))
+      (if (not (or (org-element-property :name latex-environment)
+		   (org-element-property :caption latex-environment)))
+	  value
 	;; 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'
+	;; 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))
 	  (buffer-string))))))
 
-
 ;;;; Latex Fragment
 
 (defun org-latex-latex-fragment (latex-fragment _contents _info)
-- 
2.12.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [patch, ox-latex] captions and latex-environments
  2017-03-16 12:02 [patch, ox-latex] captions and latex-environments Rasmus
@ 2017-03-16 12:09 ` Rasmus
  2017-03-17  7:22   ` Nicolas Goaziou
  0 siblings, 1 reply; 10+ messages in thread
From: Rasmus @ 2017-03-16 12:09 UTC (permalink / raw)
  To: emacs-orgmode

[-- Attachment #1: Type: text/plain, Size: 218 bytes --]

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.

-- 
This message is brought to you by the department of redundant departments

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-ox-latex-Support-caption-for-latex-environment.patch --]
[-- Type: text/x-diff, Size: 4421 bytes --]

From 4304552a0c8a72c6aeb2805a8cf703eddb5da123 Mon Sep 17 00:00:00 2001
From: Rasmus <rasmus@gmx.us>
Date: Thu, 16 Mar 2017 12:45:10 +0100
Subject: [PATCH] ox-latex: Support caption for latex-environment

* lisp/ox-latex.el (org-latex-environment--type): New function
  determining type of a latex-environment.
  (org-latex-latex-environment): Add support for caption.
  (org-latex--caption/label-string): Use correct type for non-floating
  latex-environments.
---
 lisp/ox-latex.el | 78 ++++++++++++++++++++++++++++++++++++++++++++------------
 1 file changed, 62 insertions(+), 16 deletions(-)

diff --git a/lisp/ox-latex.el b/lisp/ox-latex.el
index 2727359cb..f226dc7ae 100644
--- a/lisp/ox-latex.el
+++ b/lisp/ox-latex.el
@@ -1312,14 +1312,19 @@ For non-floats, see `org-latex--wrap-label'."
      (t
       (format (if nonfloat "\\captionof{%s}%s{%s%s}\n"
 		"\\caption%s%s{%s%s}\n")
-	      (if nonfloat
-		  (cl-case type
-		    (paragraph "figure")
-		    (src-block (if (plist-get info :latex-listings)
-				   "listing"
-				 "figure"))
-		    (t (symbol-name type)))
-		"")
+	      (let ((type* (if (eq type 'latex-environment)
+			       (org-latex-environment--type element)
+			     type)))
+		(if nonfloat
+		    (cl-case type*
+		      (paragraph "figure")
+		      (image "figure")
+		      (special-block "figure")
+		      (src-block (if (plist-get info :latex-listings)
+				     "listing"
+				   "figure"))
+		      (t (symbol-name type*)))
+		  ""))
 	      (if short (format "[%s]" (org-export-data short info)) "")
 	      label
 	      (org-export-data main info))))))
@@ -2250,24 +2255,65 @@ CONTENTS is nil.  INFO is a plist holding contextual information."
 
 ;;;; Latex Environment
 
+(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)))
+	 (latex-begin-re (cadr (assoc "begin" org-latex-regexps)))
+	 (env (progn (string-match latex-begin-re value)
+		     (match-string 2 value))))
+    (cond
+     ((string-match org-latex-math-environments-re value) 'math)
+     ((string-match-p "tab\\(le\\|ular\\)" env) 'table)
+     ((string-match-p "figure" env) 'image)
+     ;; Default coding environments
+     ((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)
+			    (match-string 2 s)
+			  s)))
+		    org-latex-custom-lang-environments))
+	   env))
+      'src-block)
+     (t 'special-block))))
+
 (defun org-latex-latex-environment (latex-environment _contents info)
   "Transcode a LATEX-ENVIRONMENT element from Org to LaTeX.
 CONTENTS is nil.  INFO is a plist holding contextual information."
   (when (plist-get info :with-latex)
-    (let ((value (org-remove-indentation
-		  (org-element-property :value latex-environment))))
-      (if (not (org-element-property :name latex-environment)) value
+    (let* ((value (org-remove-indentation
+		   (org-element-property :value latex-environment)))
+	   (type (org-latex-environment--type latex-environment))
+	   (caption (if (eq type 'math)
+			(org-latex--label latex-environment info nil t)
+		      (org-latex--caption/label-string latex-environment info)))
+	   (caption-above-p
+	    (memq type (append (plist-get info :latex-caption-above) '(math)))))
+      (if (not (or (org-element-property :name latex-environment)
+		   (org-element-property :caption latex-environment)))
+	  value
 	;; 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'
+	;; 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))
 	  (buffer-string))))))
 
-
 ;;;; Latex Fragment
 
 (defun org-latex-latex-fragment (latex-fragment _contents _info)
-- 
2.12.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [patch, ox-latex] captions and latex-environments
  2017-03-16 12:09 ` Rasmus
@ 2017-03-17  7:22   ` Nicolas Goaziou
  2017-03-17  9:23     ` Rasmus
  0 siblings, 1 reply; 10+ messages in thread
From: Nicolas Goaziou @ 2017-03-17  7:22 UTC (permalink / raw)
  To: Rasmus; +Cc: emacs-orgmode

Hello,

Rasmus <rasmus@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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [patch, ox-latex] captions and latex-environments
  2017-03-17  7:22   ` Nicolas Goaziou
@ 2017-03-17  9:23     ` Rasmus
  2017-03-18  9:44       ` Nicolas Goaziou
  0 siblings, 1 reply; 10+ messages in thread
From: Rasmus @ 2017-03-17  9:23 UTC (permalink / raw)
  To: emacs-orgmode

[-- Attachment #1: Type: text/plain, Size: 2318 bytes --]

Hi,

Thanks for the feedback.

Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:

>> +(defun org-latex-environment--type (latex-environment)
>
> It should be `org-latex--environment-type'.

Yes.

> I'd rather avoid using `org-latex-regexps', which predates the parser.
> A hard-coded regexp is better.

OK.


>> +	 (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 ...))

Don't the element `latex-environment' always start \begin{.}?
Cf. org-element--latex-begin-environment.

Another danger is that someone writes something like,

    \begin{center}
    \begin{table}
    ...

I wouldn’t worry too much about this, though.

>> +    (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.

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?

>> +	;; 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.

Good point.  Thanks.

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.

Thanks,
Rasmus

-- 
It was you, Jezebel, it was you

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-ox-latex-Support-caption-for-latex-environment.patch --]
[-- Type: text/x-diff, Size: 5016 bytes --]

From a3e7dd757a4bcba7dfdd5f54e2e703c1b37ce09a Mon Sep 17 00:00:00 2001
From: Rasmus <rasmus@gmx.us>
Date: Thu, 16 Mar 2017 12:45:10 +0100
Subject: [PATCH] ox-latex: Support caption for latex-environment

* lisp/ox-latex.el (org-latex--environment-type): New function
  determining type of a latex-environment.
  (org-latex-latex-environment): Add support for caption.
  (org-latex--caption/label-string): Use correct type for non-floating
  latex-environments.
* etc/ORG-NEWS: Add entry.
---
 etc/ORG-NEWS     |  2 +-
 lisp/ox-latex.el | 77 ++++++++++++++++++++++++++++++++++++++++++++------------
 2 files changed, 62 insertions(+), 17 deletions(-)

diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS
index f9c1196e5..c3bc60efa 100644
--- a/etc/ORG-NEWS
+++ b/etc/ORG-NEWS
@@ -183,7 +183,7 @@ user to specify the name of the output file upon exporting the
 document.  This also has an effect on publishing.
 **** Horizontal rules are no longer ignored in LaTeX table math mode
 **** Use ~compilation-mode~ for compilation output
-
+**** ~latex-environment~ elements support ~caption~ keywords for LaTeX export
 *** ~org-edit-special~ can edit LaTeX environments
 
 Using ~C-c '~ on a LaTeX environment opens a sub-editing buffer.  By
diff --git a/lisp/ox-latex.el b/lisp/ox-latex.el
index 2727359cb..0d492e421 100644
--- a/lisp/ox-latex.el
+++ b/lisp/ox-latex.el
@@ -1312,14 +1312,19 @@ For non-floats, see `org-latex--wrap-label'."
      (t
       (format (if nonfloat "\\captionof{%s}%s{%s%s}\n"
 		"\\caption%s%s{%s%s}\n")
-	      (if nonfloat
-		  (cl-case type
-		    (paragraph "figure")
-		    (src-block (if (plist-get info :latex-listings)
-				   "listing"
-				 "figure"))
-		    (t (symbol-name type)))
-		"")
+	      (let ((type* (if (eq type 'latex-environment)
+			       (org-latex--environment-type element)
+			     type)))
+		(if nonfloat
+		    (cl-case type*
+		      (paragraph "figure")
+		      (image "figure")
+		      (special-block "figure")
+		      (src-block (if (plist-get info :latex-listings)
+				     "listing"
+				   "figure"))
+		      (t (symbol-name type*)))
+		  ""))
 	      (if short (format "[%s]" (org-export-data short info)) "")
 	      label
 	      (org-export-data main info))))))
@@ -2250,24 +2255,64 @@ CONTENTS is nil.  INFO is a plist holding contextual information."
 
 ;;;; Latex Environment
 
+(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)) "")))
+    (cond
+     ((string-match-p org-latex-math-environments-re value) 'math)
+     ((string-match-p
+       (regexp-opt '("table" "longtable" "tabular" "tabu" "longtabu")) env)
+      '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)
+			    (match-string 2 s)
+			  s)))
+		    org-latex-custom-lang-environments))
+	   env))
+      'src-block)
+     (t 'special-block))))
+
 (defun org-latex-latex-environment (latex-environment _contents info)
   "Transcode a LATEX-ENVIRONMENT element from Org to LaTeX.
 CONTENTS is nil.  INFO is a plist holding contextual information."
   (when (plist-get info :with-latex)
-    (let ((value (org-remove-indentation
-		  (org-element-property :value latex-environment))))
-      (if (not (org-element-property :name latex-environment)) value
+    (let* ((value (org-remove-indentation
+		   (org-element-property :value latex-environment)))
+	   (type (org-latex--environment-type latex-environment))
+	   (caption (if (eq type 'math)
+			(org-latex--label latex-environment info nil t)
+		      (org-latex--caption/label-string latex-environment info)))
+	   (caption-above-p
+	    (memq type (append (plist-get info :latex-caption-above) '(math)))))
+      (if (not (or (org-element-property :name latex-environment)
+		   (org-element-property :caption latex-environment)))
+	  value
 	;; 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'
+	;; 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))
+	    (goto-char (point-max))
+	    (forward-line -1))
+	  (insert caption)
 	  (buffer-string))))))
 
-
 ;;;; Latex Fragment
 
 (defun org-latex-latex-fragment (latex-fragment _contents _info)
-- 
2.12.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [patch, ox-latex] captions and latex-environments
  2017-03-17  9:23     ` Rasmus
@ 2017-03-18  9:44       ` Nicolas Goaziou
  2017-03-20 14:34         ` Rasmus
  0 siblings, 1 reply; 10+ messages in thread
From: Nicolas Goaziou @ 2017-03-18  9:44 UTC (permalink / raw)
  To: Rasmus; +Cc: emacs-orgmode

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [patch, ox-latex] captions and latex-environments
  2017-03-18  9:44       ` Nicolas Goaziou
@ 2017-03-20 14:34         ` Rasmus
  2017-03-23 16:17           ` Nicolas Goaziou
  0 siblings, 1 reply; 10+ messages in thread
From: Rasmus @ 2017-03-20 14:34 UTC (permalink / raw)
  To: mail; +Cc: emacs-orgmode

Hi,

Nicolas Goaziou <mail@nicolasgoaziou.fr> 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.

OK.  AFAIR, org-element--latex-begin-environment shouldn't be used in
other libraries.

>> Another danger is that someone writes something like,
>>
>>     \begin{center}
>>     \begin{table}
>>     ...
>
> You can start the regexp with "\\`[ \t]*..."

The point is, that the env. will be center rather than table.  So the code
would detect the wrong environment.

>> +      '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.

Thanks.

>> +			    (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.

I agree this is fragile, and I’m happy to remove that part of the
matching.

The support of captions in custom-lang-environments is not really the
point here. The point of this patch is to be able to add captions to
latex-environments.  E.g. externally generated files, e.g. tables, to be
included.

Rasmus

-- 
I hear there's rumors on the, uh, Internets. . .

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [patch, ox-latex] captions and latex-environments
  2017-03-20 14:34         ` Rasmus
@ 2017-03-23 16:17           ` Nicolas Goaziou
  2017-03-24 16:25             ` Rasmus
  0 siblings, 1 reply; 10+ messages in thread
From: Nicolas Goaziou @ 2017-03-23 16:17 UTC (permalink / raw)
  To: Rasmus; +Cc: emacs-orgmode

Hello,

Rasmus <rasmus@gmx.us> writes:

>>> Another danger is that someone writes something like,
>>>
>>>     \begin{center}
>>>     \begin{table}
>>>     ...
>>
>> You can start the regexp with "\\`[ \t]*..."
>
> The point is, that the env. will be center rather than table.  So the code
> would detect the wrong environment.

But in this case, the caption is ambiguous anyway. Therefore, I see
nothing wrong here.


Regards,

-- 
Nicolas Goaziou

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [patch, ox-latex] captions and latex-environments
  2017-03-23 16:17           ` Nicolas Goaziou
@ 2017-03-24 16:25             ` Rasmus
  2017-03-27 12:02               ` Nicolas Goaziou
  0 siblings, 1 reply; 10+ messages in thread
From: Rasmus @ 2017-03-24 16:25 UTC (permalink / raw)
  To: mail; +Cc: emacs-orgmode

Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:

> Hello,
>
> Rasmus <rasmus@gmx.us> writes:
>
>>>> Another danger is that someone writes something like,
>>>>
>>>>     \begin{center}
>>>>     \begin{table}
>>>>     ...
>>>
>>> You can start the regexp with "\\`[ \t]*..."
>>
>> The point is, that the env. will be center rather than table.  So the code
>> would detect the wrong environment.
>
> But in this case, the caption is ambiguous anyway. Therefore, I see
> nothing wrong here.

I pushed a simplified version that doesn't consider
org-latex-custom-lang-environments.  If anyone ever needs that we can add
it.  I expect it’s mainly useful for tables and images anyway.

Let me know if there’s any problems.

Rasmus

-- 
Warning: Everything saved will be lost

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [patch, ox-latex] captions and latex-environments
  2017-03-24 16:25             ` Rasmus
@ 2017-03-27 12:02               ` Nicolas Goaziou
  2017-03-27 12:30                 ` Rasmus
  0 siblings, 1 reply; 10+ messages in thread
From: Nicolas Goaziou @ 2017-03-27 12:02 UTC (permalink / raw)
  To: Rasmus; +Cc: emacs-orgmode

Hello,

Rasmus <rasmus@gmx.us> writes:

> I pushed a simplified version that doesn't consider
> org-latex-custom-lang-environments.

Thank you.

> If anyone ever needs that we can add
> it.

I'm not sure to understand why would anyone need it. I haven't looked
hard enough, tho.

Regards,

-- 
Nicolas Goaziou

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [patch, ox-latex] captions and latex-environments
  2017-03-27 12:02               ` Nicolas Goaziou
@ 2017-03-27 12:30                 ` Rasmus
  0 siblings, 0 replies; 10+ messages in thread
From: Rasmus @ 2017-03-27 12:30 UTC (permalink / raw)
  To: mail; +Cc: emacs-orgmode

Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:

> I'm not sure to understand why would anyone need it. I haven't looked
> hard enough, tho.

Me neither.  Perhaps someone would automatically generate code blocks for
inclusion...

In any case, no use case exists at the moment.

Rasmus

-- 
Got mashed potatoes. Ain't got no T-Bone. No T-Bone

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2017-03-27 12:30 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-03-16 12:02 [patch, ox-latex] captions and latex-environments Rasmus
2017-03-16 12:09 ` Rasmus
2017-03-17  7:22   ` Nicolas Goaziou
2017-03-17  9:23     ` Rasmus
2017-03-18  9:44       ` Nicolas Goaziou
2017-03-20 14:34         ` Rasmus
2017-03-23 16:17           ` Nicolas Goaziou
2017-03-24 16:25             ` Rasmus
2017-03-27 12:02               ` Nicolas Goaziou
2017-03-27 12:30                 ` Rasmus

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).