"Pedro A. Aranda" <[email protected]> writes:

> Subject: [PATCH] LaTeX export: add org-latex-fontspec-config

Applied, onto main, with changes.
I have fixed a number of typos and formatting problems.
I have also moved org-get-string-scripts to org-macs.el to avoid even more
circular dependencies in org.el and because it is a pretty generic function.
I removed some leftover comments from the earlier iterations that are
no longer relevant.
Finally, there was one missed uncommented (message ...). From our
previous discussion, I judged that it was unintentional omission.
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=aab2bc0c9

Additionally, as a separate patch, I reworded one warning.
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=de710748b

>>> (org-latex-guess-fontspec): New funciton to insert the fontspec
>>> configuration when the fontspec package is requested in the LaTeX
>>> prelude.
>> 
>> Shouldn't "prelude" be preamble?
>
> As mentioned before, both are used interchangeably, but anyhow...
> changed

The patch did not contain this change.
I made the change myself on top of the patch.
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=0197489cf

>>> +  :group 'org-export-latex
>>> +  :package-version '(Org . "10.0")
>>> +  :type 'alist
>> 
>> We could use a more detailed type definition.
>
> FFS

Please elaborate.
If you disagree with the suggested change in the patch, it would be
useful to provide more details on why.

I have improved the :type definition in
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=63f1e2e86

>>> +                (matched (string-match "\\\\usepackage{fontspec}\n" 
>>> header))
>> 
>> Why trailing \n?
>> Also, what about \usepackage[...]{fontspec}?
> Removed.

You missed my comment about

   Also, what about \usepackage[...]{fontspec}?

I have updated the regexp in
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=33f6197aa

> EXTRA stuff to make the patch as feature-completeas possible:
>
> 1.- I included the :languages -> :language in the INFO channel from 
> feature/all-tex-fonts. 

I have removed this change from the applied patch.
The :languages property in the patch is unused by the actual code, so
the news entry is not very useful - you changed nothing for the users.
I think we discussed similar situations in the past - adding new code
APIs that are not used in the code (at least, partially) in anticipation
of new features is not a good practice.

> We can't use it, however, because some exporters 
> rely on the original #+LANGUAGE implementation overwriting the current 
> language.

Could you please elaborate what exactly is broken?

> 2.- I reintroduced handling CJK fonts with fontspec+xelatex (w/ test and 
> documentation).

Ok.

> 3.- unicode-math is only included when needed, i.e. when we use 
> \setmonofont.

Ok. I note that you used \RequirePackage. I think \usepackage should be
used instead. Given that you asked to apply the patch quickly, I have
changed it to \usepackage without waiting for your input in a separate
followup patch.
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=127788853
If you believe that \RequirePackage should be used, let's discuss it
later.

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

Reply via email to