Re: [PATCH] ox-latex.el: Unify in one single list Babel and Polyglossia languages alists

[Ihor Radchenko <yantar92@gmail.com>]

Juan Manuel Macías <maciaschain@posteo.net> writes:

> I attach the new version of the patch with both variables declared
> obsolete.

Thanks!

We usually declare obsolete variables in org-compat.el.

> If everything is ok, I can add what is necessary to NEWS and to the Org Manual.

I have some minor comments. You can address them and then go ahead with
NEWS and manual.

For Max's comment, using plist/alist would make things more clear
code-wise for future developers. I always find it annoying when I need
to go back and forth checking which element should be second or third or
forth in the list. Especially if the variable is used all around the
codebase. Though this particular case is not such -
`org-latex-language-alist' is used just in few places.

> +(make-obsolete-variable 'org-latex-babel-language-alist
> +                        "set `org-latex-language-alist' instead." "9.6")
> +
> +(make-obsolete-variable 'org-latex-polyglossia-language-alist
> +                        "set `org-latex-language-alist' instead." "9.6")
> +

As I mentioned earlier, please move the obsoletion statements to org-compat.

> -  "Alist between language code and corresponding Polyglossia option.")
> +  "Alist between language code and corresponding Babel/Polyglossia option.
> +
> +For the names of the languages, the Babel nomenclature is
> +preferred to that of Polyglossia, in those cases where both
> +coincide.
> +
> +The alist supports three types of members:
> +
> +- Members with two elements: CODE BABEL/POLYGLOSSIA OPTION.
> +
> +- Members with three elements: CODE BABEL/POLYGLOSSIA OPTION
> +ASTERISK (the presence of the asterisk indicates that this
> +language is not loaded in Babel using the old method of ldf
> +files but using ini files. If Babel is loaded in an Org
> +document with these languages, the \"AUTO \" argument is just
> +removed, to avoid compilation errors).

Two spaces between sentences please, as in
https://orgmode.org/worg/org-contribute.html

>  	;; If LANGUAGE is already loaded, return header without AUTO.
>  	;; Otherwise, replace AUTO with language or append language if
>  	;; AUTO is not present.
>  	(replace-match
>  	 (mapconcat (lambda (option) (if (equal "AUTO" option) language option))
>  		    (cond ((member language options) (delete "AUTO" options))
> +			  ((let ((l (assoc language-code org-latex-language-alist)))
> +			     (and (consp l) (= (length l) 3))) (delete "AUTO" options))

A comment explaining why "3" would be useful.

> -			      (if (and (consp l) (= (length l) 3))
> -				  (format "[variant=%s]" (nth 2 l))
> +			      (if (and (consp l) (= (length l) 4))
> +				  (format "[variant=%s]" (nth 3 l))
>  				"")
> -			      (nth 1 l))))
> +			      (if (and (consp l) (= (length l) 4))
> +				  (nth 2 l)
> +				(nth 1 l)))))

Again, comment explaining all these 2,3,4 would help.

Best,
Ihor