Pedro Andres Aranda Gutierrez <[email protected]> writes: >> > * doc/org-manual.org >> > (*** Controlling font setup for LuaLaTeX andXeLaTeX): New section >> > describing org-latex-fontspec-config. >> >> You should just use "(Controlling font setup for LuaLaTeX andXeLaTeX)" >> to refer to section. >> > I'm confused... I've seen it with the stars in other log messages... > anyhow. Fixed
Could you show me those log messages you are referring to? >> Also, 'org-latex-fontspec-config' (quoted). >> > Fixed using `' if you don't mind. I do not mind. Either way is fine, as documented in https://orgmode.org/worg/org-contribute.html#commit-messages >> And you missed the section in ORG-NEWS where you put the announcement. >> > Never noticed that elsewhere ... will do. I admit that I do not always ask to put it into the changelog. Mostly mentioned here for completeness. >> > (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? >> > Both can be used... but anyhow... changed We use preamble in some functions in ox-latex. Mixing two is confusing, IMHO. Also, the new patch does not have "prelude" changed. >> Nit: Maybe fn can go right after "the TeX Gyre (TG) Collection of >> Fonts", not after "could be". >> > The resulting PDF looks cluttered, so I preferred this way. > > All the next were the to enhance readability... Ok. >> > +the missing glyph set, by using the =:fallback= property for the >> >> ~:fallback~ (similar problems later on as well) >> > Changed But not similar problems. There are many instances of similar issue with =:prop= instead of ~:prop~. Again, this is minor. If not other comments, I could simply fix this myself. >> > +*** New custom variable ~org-latex-fontspec-config~ >> > + >> > +When using LuaLaTeX (or XeLaTeX in a reduced scope), you can use the >> > +new custom variable ~org-latex-fontspec-config~ to change the >> > +document's fonts and add fallback fonts for missing scripts. >> >> I think we can refer to the new section in the manual for more details. Looks like you missed this comment. >> > "Return complete document string after Beamer conversion. >> > CONTENTS is the transcoded contents string. INFO is a plist >> > holding export options." >> > + ;; Before doing anything else, add the script information >> > + ;; to the INFO channel. Used by org-latex-make-preamble >> > + ;; to add fallback fonts for lualatex. >> > + (setq info (plist-put info >> > + :doc-scripts >> > + (org-get-string-scripts contents))) >> > + >> >> May we do the same in :filter-body instead? Then, we can just arrange >> calling org-get-string-scripts in ox-latex without a need to repeat the >> procedure in ox-beamer. > > Will see and put that as the last patch in [1] See my reply below. >> > +(defcustom org-latex-fontspec-config nil >> > + "An alist with the configuration for the fontspec package. >> > + >> > +This configuration will be generated when using lualatex or xelatex. >> > + >> > +Each element is defined as >> > +(`font-name' . `font-plist') >> > + where `font-name' one of \"main\", \"sans\", \"mono\" or \"math\" >> > + and `font-plist' is a plist. The keys for this plist are >> > + `:font': font name for font installed in your system >> > + `:features': string or list of strings with font features (optional). >> > + `:fallback': an alist of (`script' . `mapping') to map _Emacs_ script >> names >> > + to their fallback font (optional). The exporter will >> warn you >> > + about scripts in your document that need a fallback >> font." >> >> These are not many details. >> We should at least refer to the new manual section. Not addressed. >> > + :group 'org-export-latex >> > + :package-version '(Org . "10.0") >> > + :type 'alist >> >> We could use a more detailed type definition. Not addressed. >> > +(defun org-latex-guess-fontspec (header info) >> > + "Add the fontspec package configuration passed in INFO to HEADER. >> > + >> > +The HEADER contains \"\\usepackage{fontspec}\", >> > +and INFO contains fontspec font conguration and >> > +add the fontspec configuration after the package." >> > + (when-let* ((fonst (plist-get info :latex-fontspec-config)) >> >> fonst? >> > I'm not using it in the function. It's there for when-let*. Then, could just use (_ ...) Each element of VARLIST is a list (SYMBOL VALUEFORM) that binds SYMBOL to the value of VALUEFORM. If only the test result is of interest, use _ as SYMBOL, i.e. (_ VALUEFORM), in which case VALUEFORM is evaluated and checked for nil but the result is not bound. An element of VARLIST can also be of the form SYMBOL, in which case the binding of SYMBOL is checked for nil, only. >> >> > + (matched (string-match "\\\\usepackage{fontspec}\n" >> header)) >> >> Why trailing \n? >> Also, what about \usepackage[...]{fontspec}? >> > > I realised later.... And that's part of the second patch in [1] > > I really don't know how to help you move fontspec to main. > The RTT for this is very high and the stop-and-go is very stressful > for me. > > /PA > > [1] https://codeberg.org/paaguti/org-font-control/patches. It does not help when changes are spread thin over multiple places - some on the mailing list, other in other threads, third on codeberg. Unfortunately, I am unable to keep track of everything, so the preferred way is keeping all the related changes in a single dedicated thread. Keep in mind, that I am working on dozens of patches at the same time, so it is critical for me to have all the necessary context together, without having to search all the past discussions every single time I come back to working on the reviews. This fragmentation of the discussions related to your feature branch is part of the reason why reviews are taking more than usual - I have to dedicate larger chunks of time to review your patches as I need to collect relevant data. And larger chunks of free time are scarce considering that I also have high workload on my main job. Could we just address all the comments to this patch (which are mostly minor) in this thread? I'd rather not rely on followup patches fixing some of the comments and make sure that all the problems are addressed in place. IMHO, the changes I asked for this particular patch should not be too complex. -- 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>
