On Sun, 9 Nov 2025 at 12:15, Ihor Radchenko <[email protected]> wrote:

> Pedro Andres Aranda Gutierrez <[email protected]> writes:
>
> > OK further experiments I have done show that
> > if there is a \setCJK...font{} declaration (for example from a
> > .dir-locals.el),
> > you need xeCJK. Therefore we will keep the code as is.
> > If we do not add this package in the presence of explicit CJK fonts, we
> > don't produce compilable documents.
>
>       ;; If the CJK font families have been included
>       ;; Check for polyglossia and/or babel and warn?
>       ;; Or advise for these packages to be added to
> `org-latex-package-alist' ??
>       (when (and cjk-packages (equal compiler "xelatex"))
>         (message "Adding the CJK packages")
>         (goto-char (point-min))
>         (forward-line 2)
>         (insert "\\usepackage[CJKspace]{xeCJK}\n"))
>
> Then, can you update the comment explaining why xeCJK is necessary?
>
> > Could we keep that as a FIXME for a refinement and discuss it further in
> > the future?
>
> Sure.
>
> >> It is right from
> >> 包时,org当中的加粗转为中间文件*tex*时是 - 200B*tex*200B escaping
> >> emphasis markup, as full with spaces are not used in Chinese writing.
> >
> > And it appears when the default font is lmroman.
> > Setting the main font to 'Noto Serif' got rid of the message.
> >
> > The other option is the LATEX_HEADER in your proposal plus
> > #+LATEX: \ctexset{space=auto}
> > which I got from
> >
> https://emacs-china.org/t/help-implementing-better-out-of-the-box-xelatex-export-in-org/30405/6
> > (thanks LuciusChen for this!)
> > With that, using lmroman will not trigger the warning and it will not
> > interfere with fonts that define the 200B character.
> >
> > Which begs the question whether a solution with an extra document class
> for
> > this could not be the cleanest answer in the future. (But leave it as
> FFS,
> > please)
>
> My main goal is making export work better with defaults
> (org-latex-default-class = "article"). We can, of course, change the
> default class depending on document language, but that sounds rather
> brittle. It will also make things less useful for people who, say,
> prefer koma-letter classes.
>
> So, \usepackage{ctex} sounds better compared to \documentclass{ctexart}
>
OK...

Anyway, back to the code review.
>
> >    (defcustom org-latex-fontspec-config nil
> >     ...
> >      `:features': string or list of strings with font features.
> >                   A potential fallback will be appended.
> >    ...
> >    (defcustom org-latex-fontspec-default-features nil
> >      "List of default features for the fontspec package.
> >    When nil, no default features are assumed.
> >    When non-nil, the value should be an alist of (FEATURE . VALUE) that
> is
> >    used to generate:
>
> The way to define features is not the same for org-latex-fontspec-config
> and org-latex-fontspec-default-features. Why not make
> org-latex-fontspec-default-features support feature string?
>

Good point. Maybe in the next iteration, we can *ADD* this possibility to
the current settings. Please keep that FFS.

>    (defcustom org-latex-polyglossia-font-config nil
> >     `:font': a string with the system font name, mandatory
> >     `:variant': a string for the font variant, (e.g. \"sf\", \"tt\",
> etc.)
> >     `:tag': a string will substitute the language in the font definition.
> >     `:props': a string for extra properties (e.g.\"Script=Hebrew\")
>
> When reading the above, I barely noticed "mandatory". What about
> something like
>
>   `:font' (mandatory): ...
>
Right, changed,

> Same in `org-latex-fontspec-config'.
>
Done

> >    (if-let* ((lang-alist (assoc lang org-latex-language-alist))
> >                               (lang-plist (cdr lang-alist)))
> >                         (setq lang-tag (plist-get lang-plist
> :polyglossia)))
>
> Nitpick: if-let* -> when-let*
>
Yup

> >     defun org-latex--lualatex-babel-config (info)
> >       "Return preamble components for babel on lualatex/xelatex.
> >     INFO is the export communication channel.
> >
> >     Prefer #+LATEX_COMPILER: over `org-latex-compiler' and
> >     and #+LANGUAGE over `org-export-default-language'.
>
> The above paragraph is probably redundant.
>
Removed

>
> >               do (let* ((props (alist-get bab-lang doc-babel-font-config
> nil nil #'string=))
> >                         (provide (or (plist-get props :provide)
> "import")))
> >                    ;; \\babelprovide needs language and provide
> >                    ;; it doesn't work on the default language
> >                    (unless provide
> >                      (setq provide "import"))
>
> (unless provide ... is redundant because we have (or ... "import") just
> above.
>
Right :-)

>
> >                                  (when (null font)
> >                                    (error "Babel: font name missing
> script: %s lang: %s" script lang))
>
> It is not clear from this error which variable the user should fix.
>
Hoperfully clearer now

> >
> >      ;; It there are font features, generate the declaration
> >      (when current-default-features
> >        (let ((def-feat-list
> >               (cl-loop for (feat . val) in current-default-features
> >                        collect (concat feat "=" val) into result
> >                        finally return (mapconcat #'identity result ",\n
> "))))
> >          (insert (format "\\defaultfontfeatures{\n  %s\n}\n"
> >                          def-feat-list))))
>
> I think that this exact code appears twice, copy-pasted. Should it be a
> separate helper function?
>
Why not... changed.

>
> >    (defun org-latex-list-or-null-p (object)
> >      "Return non-nil when `object' is a list or nil"
> >      (or (null object)
> >          (listp object)))


> A simpler name like org-list-or-null-p would make more sense. It is a
> very generic predicate, not really linked to latex
>
Right...

> > (defun org-latex--lualatex-fontspec-config (info)
> > ...
>
> See the attached diff with my suggestions. I had a really hard time
> reading the code here and tried to rename the variables to make things
> easier. With the diff, things are certainly easier to read for me, but
> not sure about you. Please consider though.
>
I could have written it in a similar way too ...

> Also, one test is failing on the latest branch.
>
Not anymore...

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


-- 
Fragen sind nicht da, um beantwortet zu werden,
Fragen sind da um gestellt zu werden
Georg Kreisler

"Sagen's Paradeiser" (ORF: Als Radiohören gefährlich war) => write BE!
Year 1 of the New Koprocracy

Reply via email to