Pedro Andres Aranda Gutierrez <[email protected]> writes:
>> Nothing will break, yes.
>> But having beamer-related code in other files is simply a bad coding
>> practice, especially if that's such a core functionality as defining
>> themes. This is not about correctness, but about code quality.
>
>> If you have library functionality split across multiple other libraries,
>> it makes it difficult to understand how the code works. It also makes it
>> potentially difficult to change ox-latex -- it will effectively gain an
>> extra dependency to account for.
>
> IMHO trying to compact code too much makes things worse.
> There are ways to move code around when you understand what the code does...
Sure. But keeping code in a single library is not too much in my book.
> Just look at the attached 0001-ox-latex-... patch. Came after I broke my
> head over the "real" patch.
>
> My suggestion may be an overkill, yes. I will be happy if you find
>> something simpler.
>
>
> Apparently there was... if the code had only been easier to read. See
> 0001-ox-beamer... patch
I am trying to not make it harder to read at least :)
> lisp/ox-beamer.el: New option BEAMER_THEME_PRE.
> (org-beamer-theme-settings): new function to manage theme-related
> settings.
*New (capitalized). Similar in other places.
These are small things, but let's keep the commit message format
consistent. Also, all sentences should end with ".".
> @@ -13162,6 +13162,19 @@ settings (see [[*Export Settings]]).
> #+cindex: @samp{BEAMER_OUTER_THEME}, keyword
> The Beamer outer theme.
>
> + - =BEAMER_THEME_PRE= ::
It is a sublist under previous item. Is it intentional?
> +*** ox-beamer: New LaTeX preamble sequence
> +
> +When exporting to Beamer, all theme-related configuration will be
> +generated right after the document class declaration in concordance
> +with the examples shipped with the Beamer class.
It is worth mentioning what was the previous default.
> +;; Generate the preamble up to the theme
> +(defun org-beamer-class-template(info)
> + "Generate the class template from INFO.
> +This will include class-pre, class and theme definitions."
> + (let* ((class-pre (plist-get info :latex-class-pre))
> + (class-options (concat "[" (plist-get info :latex-class-options)
> "]")))
> + ;; See org-latex--mk-options:
> + ;; Accept class options with and without square brackets
> + (setq class-options (replace-regexp-in-string "\\`\\[+" "["
> class-options))
> + (setq class-options (replace-regexp-in-string "\\]+\\'" "]"
> class-options))
> + (concat class-pre (and class-pre "\n")
> + "\\documentclass" class-options "{beamer}\n"
> + (org-beamer-theme-settings info))))
Now, we are copying the logic from ox-latex. If we ever change how
ox-latex handles things, and forget this place, it will be a bug we
could have avoided.
What about very dumb approach - call `org-latex-make-preamble', find
\documentclass... inside, and then insert the theme definition right
after \documentclass, in the already-expanded preamble?
> +(ert-deftest ox-beamer/org-beamer-pre-theme ()
> + "Test that the theme is in its new place and beamer-pre is included."
> + (org-test-with-exported-text
> + 'beamer
> + "#+OPTIONS: toc:nil
> +#+LATEX_CLASS_OPTIONS: presentation,t
> +#+BEAMER_THEME_PRE: \\usepackage{geometry}
> +#+BEAMER_THEME: Boadilla
> +* A frame
> +Here is an example.
> +"
> + (goto-char (point-min))
> + (should (search-forward
> "\\documentclass[presentation,t]{beamer}\n\\usepackage{geometry}\n\\usetheme{Boadilla}\n"))))
Nitpick: You do not have to use \n in the search forward. Can keep the
actual newlines as in the org-test-with-exported-text argument. It will
be easier to read.
> -;;; ox-latex.el --- LaTeX Backend for Org Export Engine -*- lexical-binding:
> t; -*-
> +;; ox-latex.el --- LaTeX Backend for Org Export Engine -*- lexical-binding:
> t; -*-
Why this change?
The number of ;'s is not meaningless. It defines outline level in
outline-minor-mode.
--
Ihor Radchenko // yantar92,
Org mode maintainer,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>
