Hi answers inline...
On Fri, 7 Nov 2025 at 09:12, Ihor Radchenko <[email protected]> wrote: > "Pedro A. Aranda" <[email protected]> writes: > > >>> I notice that you are using tabs for indentation. Our convention (and > >>> settings in .dir-locals.el) disable indent-tabs-mode. > >> > >> Did you miss this comment? > > > > I thought not... Still there? I have (indent-tabs-mode -1) > > in the emacs-lisp-mode-hook in my init. Will investigate... > > To be fair, our files are a mix of spaces and tabs, but Emacs devel > convention is to only ever touch those when making actual changes > (i.e. no whitespace-only commits). > OK, I'll try my best. At least I fixed my emacs config... > > >>> >This option is not used anywhere. > >>> > >>> Part of our discussions... commented out > >> > >> Which discussion are you referring to? > > > > :-) This thread and the previous ones are somehow long. This variable > > was used by a function that eventually disappeared in the process of > > giving this functionality its (current) form. > > Then, why do we need to retain it as comment? > Right, deleted when I commit. > That's going to take time... Unless I find a way to refactor variables > > in emacs-lisp-mode. > > A simple M-x query-replace usually works just fine. > In more complex cases, you can make use of \_< and \_> regexp constructs > and > M-x query-replace-regexp. > Will start after this commit. >>> Including fontspec seems sometimes redundant, but I'm still not 100% > fine > >>> with not including it because of tracinglostchars. > >> > >> Does \tracinglostchars have anything to do with fontspec package? It > >> seems to be a TeX primitive: > https://tug.org/utilities/plain/cseq.html#tracinglostchars-rp > > > > Was not part of the generated code before, right. But it improves the > > information provided by the log. > > Then, I do not understand what you mean by > >>> ... I'm still not 100% fine with not including it because of > tracinglostchars > That I'll keep it FTMB. This is not cast in stone and people may come up with better solutions. Currently, it does what I need, which is to provide explicit enough messages (we may want to detect and use in the future). > > > >>> (let ((test-copy test-list)) > >>> (message "Inside let, before pop -> %s" test-copy) > >>> (pop test-copy) > >>> (message "Inside let, after pop -> %s" test-copy)) > >>> (message "Outside let -> %s" test-list) > >> > >> You are right, but that only works because how the code is written > >> now. To avoid future regression I'd prefer to not use pop when not > >> necessary. In this particular case, a simple (car ...) and (cdr ...) > >> will work just fine, AFAIU. We should generally be careful with > >> functions that modify things by side effect. Bugs arising from their > >> misuse are often hard to debug. > > > > How long has (pop) been around? > > What do you mean 'how the code is written now'? > > In short, I have seen bugs introduced when code around calls to pop or > other function with side effect changes. It is not that your code is not > correct. It is my worry about maintainability of that code. > So, I generally prefer avoiding modifying staff by side effect unless > strictly necessary. > I have changed that under protest. `pop' has been around in my programmer's life since I used the Intel 8008 and it is part of a 101 in programming. >> As a general stylistic rule, we should use simpler constructs when we > >> can. Some people have trouble reading cl-loop constructs, and we should > >> only use it when it clearly benefits the code. IMHO inherited from > >> Nicolas. > > > > In this case, I doubt cl-loop is that more difficult to understand that > > (dolist)? Mixing cl-loop and dolist is more confusing for me. > > On emacs-devel, there is a noticeable group of Elisp hackers who often > complain about cl-lib and its idiomatic constructs like cl-loop (which > they do not want to bother learning) > So, cl-lib is often considered less readable and people generally prefer > avoiding it, unless it brings clear benefit to the code (some people > just avoid cl-lib. full stop) > And people don't know what pop does ... ;-) >>> > (defun org-latex--lualatex-fontspec-config (info) > >>> > "Returns the font prelude when we are on Lua/XeLaTeX and > >>> > we are using neither bale nor polyglossia" > >>> > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > >>> > >>> To stress that it is a fontspec-only. Anyhow, changed... > >> > >> I did not mean that it needs to be changed. Just highlighted that the > >> commentary contradicted the docstring. > > > > But it's better now, right? > > Yes. > OK > >>> >> (when fallback-alist > >>> >> (unless (equal compiler "lualatex") > >>> >> (setq fallback-alist nil) > >>> >> (warn "Fallback fonts only work with lualatex!"))) > >>> > > >>> >AFAIU, this warning will be displayed every single time latex > export is > >>> >using xelatex, if there are fallback fonts in the fontspec-config. > Is it > >>> >really desired? Should be there an option to provide separate > fontspec > >>> >configuration to xelatex? > >>> > >>> Well, I'd rather keep it. If someone doesn't fully understand this and > >>> keeps trying to use xelatex with fallbacks, he should be warned each > >>> single time. > >> > >> It is fine to keep the warning, but what should be done for someone who > >> uses both lualatex and xelatex? The configuration is shared. > > > > It will only appear when (s)he is not using lualatex. pdflatex and > > xelatex would generate errors. > > Let me elaborate. > Consider a user who sometimes want to use lualatex and sometimes > xelatex. Since xelatex does not support fallabacks, the user may want to > have a completely different font config for xelatex (more generic, less > satisfying font), but maintain a separate config for lualatex (better > font + fallback for rare strange symbols). > This would not be possible on your branch. > The way I cope with this is I have a root .dir-locals.el in my $HOME/Documents with the most probable configuration (lualatex+fontspec) and when I need to go beyond that I place a document-specific .dir-locals.el, where I would put everything xelatex "needs and loves". >> But why should we? There is org-latex-packages-alist where users can add > >> default packages. xpinyin might (or might not) be a nice default package > >> to load, by it has little to do with babel/polyglossia/font > configuration. > > > > Not so sure about this. Imagine someone comes up with a config that we > > can reasonably fit in this... Unfortunately, I haven't had time to > > study Japanese. Maybe when I retire... > > If someone comes up with ideas how to improve font config, we can add > whatever idea is presented then. I still do not understand why is it > necessary to keep reference to xpinyin in the code? > > Also, how does pinyin (Chinese transliteration) have anything to do with > Japanese language? > 1. s/Japanese/Chinese/g (sorry) 2. I have seen it in most examples for xelatex with fontspec and CJK fonts and since I don't speak it, I can't judge whether it can be omitted or not. So better keep it. > > >> Cleaned up docstring and the code: > >> > >> (defun org-latex--pdflatex-ldf (lang) > >> "Return ldf code for LANG from :babel property in > `org-latex-language-alist'. > > ... > > Will be more difficult to detect regressions/debug in the future. I'd > > leave it like it is now. It documents the logic to arrive at the ldf > better. > > Ok. > > >>> (when multi-lang > >>> (insert "%% \\usepackage[utf8]{inputenc}\n") > >> > >> Why do you need to insert a commented-out usepackage call? > > > > Helps me when looking at the result in a LaTeX export buffer. > > Could you elaborate? > When I have problems, I export to a .tex file and then edit from there until I debug/cleanup. I may want to revert to pdflatex and there I would uncomment this line. >> (insert (format "\n\\defaultfontfeatures{\n %s\n}}" > >> (mapconcat (pcase-lambda (`(,feature . ,value)) > (concat feature "=" value)) > >> current-default-features ",\n "))) > >> > >> (The same for `org-latex--lualatex-fontspec-config') > > > > I can't decode your proposal as quickly as with cl-loop, sorry. > > :) Ok. I am in pcase camp, you are in cl-loop. > This one is not critical. > > > To start a conversation on this: > > What is the overhead of cl-loop in a process which ends up in a buffer > > output or a file or a file that is then compiled by LaTeX? > > I would worry about overhead if this was part of a function that > > needs to be executed often. But here, I find it easier to read. > > You do not need to worry about overheads here. Not for preamble that is > computed once. Also, cl-loop does not have much overhead. > > >>> (let ((lang-tag lang)) > >>> ;; (message "new font family: (%s . %s)" lang props) > >>> (if-let* ((lang-alist (assoc lang > org-latex-language-alist)) > >>> (lang-plist (cdr lang-alist))) > >>> (setq lang-tag (plist-get lang-plist > :polyglossia))) > >>> ;; (message "polyglossia language name is %s" > lang-tag) > >>> (insert (format "\n\\newfontfamily{\\%sfont%s}%s{%s}" > >> > >> Should the above comments be removed? > > > > Do they harm? They will help me remember what I did a couple months from > > now. > > No harm, except that they distract reading the code (for me). > I'd prefer proper comments that will not only remind _you_ what you did > but also explain things to others ;) > Maybe the first is a bit more '(cryptic|distracting)' but I think the second isn't. And well, the lines can be uncommented in case we need to debug any specific configuration. I keep saying to me that this code is by no means perfect and we will need to debug. Am I over-cautious? Best, /PA > > -- > 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
