emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
From: Ihor Radchenko <yantar92@gmail.com>
To: "Juan Manuel Macías" <maciaschain@posteo.net>
Cc: orgmode <emacs-orgmode@gnu.org>
Subject: Re: [PATCH] ox-latex.el: Unify in one single list Babel and Polyglossia languages alists
Date: Sun, 17 Jul 2022 17:55:24 +0800	[thread overview]
Message-ID: <87bkto83n7.fsf@localhost> (raw)
In-Reply-To: <87lesvlvos.fsf@posteo.net>

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


  parent reply	other threads:[~2022-07-17  9:54 UTC|newest]

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-03 15:28 [PATCH] ox-latex.el: Unify in one single list Babel and Polyglossia languages alists Juan Manuel Macías
2022-07-10  9:25 ` Ihor Radchenko
2022-07-14 12:34   ` Juan Manuel Macías
2022-07-14 15:12     ` Max Nikulin
2022-07-14 15:53       ` Juan Manuel Macías
2022-07-14 18:17         ` Juan Manuel Macías
2022-07-15 12:18           ` Max Nikulin
2022-07-15 14:36             ` Juan Manuel Macías
2022-07-17  9:55     ` Ihor Radchenko [this message]
2022-07-17 14:48       ` Juan Manuel Macías
2022-07-18  6:44         ` Ihor Radchenko
2022-07-18 10:32           ` Juan Manuel Macías
2022-07-18 11:01             ` Juan Manuel Macías
2022-07-18 15:37             ` Max Nikulin
2022-07-18 16:21               ` Juan Manuel Macías
2022-07-19 15:01       ` Juan Manuel Macías
2022-07-19 17:01         ` Max Nikulin
2022-07-19 19:31           ` Juan Manuel Macías
2022-07-20 16:12             ` Max Nikulin
2022-07-20 21:30               ` Juan Manuel Macías
2022-07-21 14:36                 ` Max Nikulin
2022-07-21 15:39                   ` Juan Manuel Macías
2022-07-22 12:16                     ` Max Nikulin
2022-07-22 12:49                       ` Juan Manuel Macías
2022-07-22 14:07                         ` Juan Manuel Macías
2022-07-23 15:19                           ` Max Nikulin
2022-07-23 17:15                             ` Improvements in the default LaTeX preamble (was: [PATCH] ox-latex.el: Unify in one single list Babel and Polyglossia languages alists) Juan Manuel Macías
2022-07-24 12:06                               ` Improvements in the default LaTeX preamble: templates? " Juan Manuel Macías
2022-07-25  9:31                                 ` Ihor Radchenko
2022-07-25 10:45                                   ` Improvements in the default LaTeX preamble: templates? Juan Manuel Macías
2022-07-23  5:01         ` [PATCH] ox-latex.el: Unify in one single list Babel and Polyglossia languages alists Ihor Radchenko
2022-07-23 13:44           ` BUG " Kai von Fintel
2022-07-23 13:59             ` Ihor Radchenko
2022-07-23 14:07               ` Kai von Fintel
2022-07-23 14:22                 ` Ihor Radchenko
2022-07-23 14:39                   ` Kai von Fintel
2022-07-23 14:50                     ` Ihor Radchenko
2022-07-23 15:53                       ` Juan Manuel Macías
2022-07-24  7:15                         ` Ihor Radchenko
2022-07-24 11:29                           ` Juan Manuel Macías
2022-07-26 11:58                             ` Ihor Radchenko
2022-07-26 16:19                               ` Juan Manuel Macías
2022-07-28 12:36                                 ` Ihor Radchenko
2022-07-23 14:53                     ` Juan Manuel Macías
2022-07-23 14:11           ` Juan Manuel Macías
2022-07-23 14:25             ` Ihor Radchenko
2022-07-23 15:29           ` Max Nikulin
2022-07-24  7:23             ` Ihor Radchenko
2022-07-10 10:51 ` Max Nikulin
2022-07-15 15:38   ` Juan Manuel Macías

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.orgmode.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87bkto83n7.fsf@localhost \
    --to=yantar92@gmail.com \
    --cc=emacs-orgmode@gnu.org \
    --cc=maciaschain@posteo.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).