Re: [BUG] Re: The orgframe construct in the Beamer exporter as a default needs a rethink

[Leo Butler <Leo.Butler@umanitoba.ca>]

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

On Sun, Mar 17 2024, Ihor Radchenko <yantar92@posteo.net> wrote:

> Leo Butler <Leo.Butler@umanitoba.ca> writes:
>
>>>> I'd prefer to keep this information in the INFO channel.
>>>> It will be more consistent.
>>
>> Apologies, I messed up the patch in the previous email.
>>
>> Attached is a patch that compiles cleanly and uses INFO.
>
> Thanks!
>
>> +         (frame (let ((selection
>> +                       (or (and fragilep
>> +                                (or (string-search "\\begin{frame}" contents)
>> +                                    (string-search "\\end{frame}" contents))
>
> Please use `string-match-p'. `string-search' is not available in Emacs
> 27, which we still support.

Done.

>
>> +                                org-beamer-frame-environment)
>> +                           "frame")))
>> +                  (unless (string= selection "frame")
>> +                    (setq info (plist-put info :define-frame t)))
>
> Let's use "beamer" prefix to indicate that this plist entry is
> beamer backend-specific. Something like :beamer-define-frame.

Done.

Attached is the updated patch. In addition, I have written three
regression tests that are attached in a separate patch.

Leo


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-lisp-ox-beamer.el-constrain-use-of-org-beamer-frame-.patch --]
[-- Type: text/x-diff; name="0001-lisp-ox-beamer.el-constrain-use-of-org-beamer-frame-.patch", Size: 2991 bytes --]

From 3b10e0013ba98cbc2b441507da359996e3d8701c Mon Sep 17 00:00:00 2001
From: Leo Butler <leo.butler@umanitoba.ca>
Date: Tue, 12 Mar 2024 15:11:27 -0500
Subject: [PATCH 1/2] lisp/ox-beamer.el: constrain use of
 org-beamer-frame-environment

* lisp/ox-beamer.el (org-beamer--format-frame, org-beamer-template):
Only use `org-beamer-frame-environment' when a frame is marked as
fragile and the frame's contents include either \begin{frame} or
\end{frame}.  When `org-beamer-frame-environment' is used and not
equal to "frame", add the property :beamer-define-frame to INFO and
set it to t.  When that property is t, `org-beamer-template' emits a
definition of the alternative frame environment.

Refs: https://list.orgmode.org/orgmode/87bk7jeik8.fsf@localhost/
https://list.orgmode.org/87a5nux3zr.fsf@t14.reltub.ca/T/
---
 lisp/ox-beamer.el | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/lisp/ox-beamer.el b/lisp/ox-beamer.el
index 4fad37b59..8bae93c11 100644
--- a/lisp/ox-beamer.el
+++ b/lisp/ox-beamer.el
@@ -429,8 +429,21 @@ used as a communication channel."
 	  ;; among `org-beamer-verbatim-elements'.
 	  (org-element-map headline org-beamer-verbatim-elements 'identity
 			   info 'first-match))
-         (frame (or (and fragilep org-beamer-frame-environment)
-                    "frame")))
+         ;; If FRAGILEP is non-nil and CONTENTS contains an occurrence
+         ;; of \begin{frame} or \end{frame}, then set the FRAME
+         ;; environment to be `org-beamer-frame-environment';
+         ;; otherwise, use "frame". If the selected environment is not
+         ;; "frame", then add the property :beamer-define-frame to
+         ;; INFO and set it to t.
+         (frame (let ((selection
+                       (or (and fragilep
+                                (or (string-match-p "\\\\begin{frame}" contents)
+                                    (string-match-p "\\\\end{frame}" contents))
+                                org-beamer-frame-environment)
+                           "frame")))
+                  (unless (string= selection "frame")
+                    (setq info (plist-put info :beamer-define-frame t)))
+                  selection)))
     (concat "\\begin{" frame "}"
 	    ;; Overlay specification, if any. When surrounded by
 	    ;; square brackets, consider it as a default
@@ -851,8 +864,8 @@ holding export options."
      (org-latex--insert-compiler info)
      ;; Document class and packages.
      (org-latex-make-preamble info)
-     ;; Define the alternative frame environment.
-     (unless (equal "frame" org-beamer-frame-environment)
+     ;; Define the alternative frame environment, if needed.
+     (when (plist-get info :beamer-define-frame)
        (format "\\newenvironment<>{%s}[1][]{\\begin{frame}#2[environment=%1$s,#1]}{\\end{frame}}\n"
                org-beamer-frame-environment))
      ;; Insert themes.
-- 
2.43.0


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-testing-lisp-test-ox-beamer.el-New-regression-tests-.patch --]
[-- Type: text/x-diff; name="0002-testing-lisp-test-ox-beamer.el-New-regression-tests-.patch", Size: 4841 bytes --]

From 4377c4a55d1fe879b92f268edebcf03d72821703 Mon Sep 17 00:00:00 2001
From: Leo Butler <leo.butler@umanitoba.ca>
Date: Sun, 17 Mar 2024 08:14:54 -0500
Subject: [PATCH 2/2] testing/lisp/test-ox-beamer.el: New regression tests for
 ox-beamer.

* testing/lisp/test-ox-beamer.el (ox-beamer/orgframe,
ox-beamer/orgframe-in-example, ox-beamer/orgframe-in-one-example): New
file.  Regression tests for ox-beamer.  Test that the
`org-beamer-frame-environment' is defined only when used.
---
 testing/lisp/test-ox-beamer.el | 110 +++++++++++++++++++++++++++++++++
 1 file changed, 110 insertions(+)
 create mode 100644 testing/lisp/test-ox-beamer.el

diff --git a/testing/lisp/test-ox-beamer.el b/testing/lisp/test-ox-beamer.el
new file mode 100644
index 000000000..be83b12e0
--- /dev/null
+++ b/testing/lisp/test-ox-beamer.el
@@ -0,0 +1,110 @@
+;;; test-ox-beamer.el --- tests for ox-beamer.el       -*- lexical-binding: t; -*-
+
+;; Copyright (C) 2024  Leo Butler
+
+;; Author: Leo Butler <leo.butler@umanitoba.ca>
+
+;; This program is free software; you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; This program is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with this program.  If not, see <https://www.gnu.org/licenses/>.
+
+;;; Commentary:
+
+;; Tests checking validity of Org Beamer export output.
+
+;;; Code:
+
+(require 'ox-beamer nil t)
+(unless (featurep 'ox-beamer)
+  (signal 'missing-test-dependency "org-export-beamer"))
+
+\f
+
+(ert-deftest ox-beamer/orgframe ()
+  "Test that `org-beamer-frame-environment' is defined and used."
+  (org-test-with-exported-text
+      'beamer
+      "#+OPTIONS: toc:nil
+* A frame
+Here is an example:
+#+begin_example
+\\begin{frame}
+...
+\\end{frame}
+#+end_example
+"
+    (goto-char (point-min))
+    (should (search-forward (concat "\\newenvironment<>{" org-beamer-frame-environment "}") nil t))
+    (should (search-forward (concat "\\begin{" org-beamer-frame-environment "}") nil t))
+    (should (search-forward (concat "\\end{" org-beamer-frame-environment "}") nil t))))
+
+(ert-deftest ox-beamer/orgframe-in-example ()
+  "Test that `org-beamer-frame-environment' is not defined."
+  (org-test-with-exported-text
+      'beamer
+      (concat "#+OPTIONS: toc:nil
+* A frame
+Here is an example:
+#+begin_example
+\\begin{" org-beamer-frame-environment "}
+...
+\\end{" org-beamer-frame-environment "}
+#+end_example
+")
+    (goto-char (point-min))
+    (should-not (search-forward
+                 (concat "\\newenvironment<>{" org-beamer-frame-environment "}") nil t))
+    (forward-line)
+    (should (search-forward (concat "\\begin{frame}") nil t))
+    (should (search-forward (concat "\\begin{" org-beamer-frame-environment "}")))
+    (should (search-forward (concat "\\end{" org-beamer-frame-environment "}")))
+    (should (search-forward (concat "\\end{frame}") nil t))))
+
+(ert-deftest ox-beamer/orgframe-in-one-example ()
+  "Test that `org-beamer-frame-environment' is defined.
+First frame should use \"frame\" environment, the second uses
+`org-beamer-frame-environment'."
+  (org-test-with-exported-text
+      'beamer
+      (concat "#+OPTIONS: toc:nil
+* A frame
+Here is an example:
+#+begin_example
+\\begin{" org-beamer-frame-environment "}
+...
+\\end{" org-beamer-frame-environment "}
+#+end_example
+
+* A second frame
+Here is a second example:
+#+begin_example
+\\begin{frame}
+...
+\\end{frame}
+#+end_example
+")
+    (goto-char (point-min))
+    (should (search-forward
+             (concat "\\newenvironment<>{" org-beamer-frame-environment "}") nil t))
+    (forward-line)
+    (org-test-ignore-duplicate
+     (should (search-forward (concat "\\begin{frame}") nil t))
+     (should (search-forward (concat "\\begin{" org-beamer-frame-environment "}")))
+     (should (search-forward (concat "\\end{" org-beamer-frame-environment "}")))
+     (should (search-forward (concat "\\end{frame}") nil t))
+     (should (search-forward (concat "\\begin{" org-beamer-frame-environment "}")))
+     (should (search-forward (concat "\\begin{frame}") nil t))
+     (should (search-forward (concat "\\end{frame}") nil t))
+     (should (search-forward (concat "\\end{" org-beamer-frame-environment "}"))))))
+
+(provide 'test-ox-beamer)
+;;; test-ox-beamer.el ends here
-- 
2.43.0