[gentoo-dev] [PATCH] l10n.eclass: Sort and normalize PLOCALES in l10n_find_plocales_change

[gentoo-dev] [PATCH] l10n.eclass: Sort and normalize PLOCALES in l10n_find_plocales_change

[James Le Cuirot]

[-- Attachment #1: Type: text/plain, Size: 1432 bytes --]

l10n_find_plocales_change assumes that PLOCALES is sorted
alphanumerically with a single space between each entry and no
surrounding whitespace. This is not a bad assumption but it isn't
documented and it's inconvenient in at least one particular case.

MakeMKV uses non-standard locale names and I intend to map these using
an associative array, which is possible as of EAPI 6. This allows me
to do the following, though only with the above change as associative
arrays are not ordered.

declare -A MY_LOCALES
MY_LOCALES=( [zh]=chi [da]=dan … )
PLOCALES="${!MY_LOCALES[@]}"
inherit l10n

src_prepare() {
	PLOCALES="${MY_LOCALES[@]}" l10n_find_plocales_changes …
}

src_install() {
	for locale in $(l10n_get_locales); do
		doins makemkv_${MY_LOCALES[${locale}]}.mo.gz
	done
}
---
 eclass/l10n.eclass | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/eclass/l10n.eclass b/eclass/l10n.eclass
index a7a6a26..26aa864 100644
--- a/eclass/l10n.eclass
+++ b/eclass/l10n.eclass
@@ -86,7 +86,7 @@ l10n_find_plocales_changes() {
 		current+="${x} "
 	done
 	popd >/dev/null
-	if [[ ${PLOCALES} != ${current%[[:space:]]} ]] ; then
+	if [[ $(tr -s "[:space:]" "\n" <<< "${PLOCALES}" | sort | xargs echo) != ${current%[[:space:]]} ]] ; then
 		einfo "There are changes in locales! This ebuild should be updated to:"
 		einfo "PLOCALES=\"${current%[[:space:]]}\""
 	else
-- 
2.7.1

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 951 bytes --]

Re: [gentoo-dev] [PATCH] l10n.eclass: Sort and normalize PLOCALES in l10n_find_plocales_change

[Davide Pesavento]

On Sat, May 7, 2016 at 10:13 PM, James Le Cuirot <chewi@gentoo.org> wrote:
> l10n_find_plocales_change assumes that PLOCALES is sorted
> alphanumerically with a single space between each entry and no
> surrounding whitespace. This is not a bad assumption but it isn't
> documented and it's inconvenient in at least one particular case.
>
> MakeMKV uses non-standard locale names and I intend to map these using
> an associative array, which is possible as of EAPI 6. This allows me
> to do the following, though only with the above change as associative
> arrays are not ordered.
>
> declare -A MY_LOCALES
> MY_LOCALES=( [zh]=chi [da]=dan … )
> PLOCALES="${!MY_LOCALES[@]}"
> inherit l10n
>
> src_prepare() {
>         PLOCALES="${MY_LOCALES[@]}" l10n_find_plocales_changes …
> }
>
> src_install() {
>         for locale in $(l10n_get_locales); do
>                 doins makemkv_${MY_LOCALES[${locale}]}.mo.gz
>         done
> }
> ---
>  eclass/l10n.eclass | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/eclass/l10n.eclass b/eclass/l10n.eclass
> index a7a6a26..26aa864 100644
> --- a/eclass/l10n.eclass
> +++ b/eclass/l10n.eclass
> @@ -86,7 +86,7 @@ l10n_find_plocales_changes() {
>                 current+="${x} "
>         done
>         popd >/dev/null
> -       if [[ ${PLOCALES} != ${current%[[:space:]]} ]] ; then
> +       if [[ $(tr -s "[:space:]" "\n" <<< "${PLOCALES}" | sort | xargs echo) != ${current%[[:space:]]} ]] ; then
>                 einfo "There are changes in locales! This ebuild should be updated to:"
>                 einfo "PLOCALES=\"${current%[[:space:]]}\""
>         else
> --
> 2.7.1

Does this also fix bug 513242?

Thanks,
Davide

Re: [gentoo-dev] [PATCH] l10n.eclass: Sort and normalize PLOCALES in l10n_find_plocales_change

[James Le Cuirot]

[-- Attachment #1: Type: text/plain, Size: 665 bytes --]

On Sat, 7 May 2016 23:13:11 +0200
Davide Pesavento <pesa@gentoo.org> wrote:

> On Sat, May 7, 2016 at 10:13 PM, James Le Cuirot <chewi@gentoo.org>
> wrote:
> > l10n_find_plocales_change assumes that PLOCALES is sorted
> > alphanumerically with a single space between each entry and no
> > surrounding whitespace. This is not a bad assumption but it isn't
> > documented and it's inconvenient in at least one particular case.
> 
> Does this also fix bug 513242?

Bonus! I didn't think to check for any existing bug reports but I've
tested this now and indeed it does. :) Thumbs up from you then?

-- 
James Le Cuirot (chewi)
Gentoo Linux Developer

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 951 bytes --]

Re: [gentoo-dev] [PATCH] l10n.eclass: Sort and normalize PLOCALES in l10n_find_plocales_change

[Davide Pesavento]

On Sat, May 7, 2016 at 11:23 PM, James Le Cuirot <chewi@gentoo.org> wrote:
> On Sat, 7 May 2016 23:13:11 +0200
> Davide Pesavento <pesa@gentoo.org> wrote:
>
>> On Sat, May 7, 2016 at 10:13 PM, James Le Cuirot <chewi@gentoo.org>
>> wrote:
>> > l10n_find_plocales_change assumes that PLOCALES is sorted
>> > alphanumerically with a single space between each entry and no
>> > surrounding whitespace. This is not a bad assumption but it isn't
>> > documented and it's inconvenient in at least one particular case.
>>
>> Does this also fix bug 513242?
>
> Bonus! I didn't think to check for any existing bug reports but I've
> tested this now and indeed it does. :) Thumbs up from you then?
>

Yep, +1 from me, although I was wondering whether there was a
cleaner/simpler way of writing that function...

Re: [gentoo-dev] [PATCH] l10n.eclass: Sort and normalize PLOCALES in l10n_find_plocales_change

[Michael Orlitzky]

On 05/07/2016 04:13 PM, James Le Cuirot wrote:
>
> +	if [[ $(tr -s "[:space:]" "\n" <<< "${PLOCALES}" | sort | xargs echo) != ${current%[[:space:]]} ]] ; then

The stuff on the left-hand side just sorts a space-separated list,
right? It might be time to split that into another function. It looks a
lot less insane when it's,

  if [[ $(l10n_sort_spaces "${PLOCALES}") != ${current%[[:space:]]} ]]

Then the  function would just be

  # @FUNCTION: l10n_sort_spaces
  # @DESCRIPTION:
  # Takes a space-separated list of strings as an argument and sorts
  # them. The output is again space-separated. This is accomplished
  # by first replacing the spaces with newlines so that "sort" can
  # be used, and then collecting the output back onto one line.
  function l10n_sort_spaces() {
    tr -s "[:space:]" "\n" <<< "${1}" | sort | xargs echo
  }

You should also document why you're doing that nearby, since like you
said, the eclass assumes that PLOCALES is sorted. Someone will wonder
why you're doing it again.

Re: [gentoo-dev] [PATCH] l10n.eclass: Sort and normalize PLOCALES in l10n_find_plocales_change

[James Le Cuirot]

[-- Attachment #1: Type: text/plain, Size: 1392 bytes --]

On Sun, 8 May 2016 11:20:34 -0400
Michael Orlitzky <mjo@gentoo.org> wrote:

> On 05/07/2016 04:13 PM, James Le Cuirot wrote:
> >
> > +	if [[ $(tr -s "[:space:]" "\n" <<< "${PLOCALES}" | sort |
> > xargs echo) != ${current%[[:space:]]} ]] ; then  
> 
> The stuff on the left-hand side just sorts a space-separated list,
> right? It might be time to split that into another function. It looks
> a lot less insane when it's,
> 
>   if [[ $(l10n_sort_spaces "${PLOCALES}") != ${current%[[:space:]]} ]]
> 
> Then the  function would just be
> 
>   # @FUNCTION: l10n_sort_spaces
>   # @DESCRIPTION:
>   # Takes a space-separated list of strings as an argument and sorts
>   # them. The output is again space-separated. This is accomplished
>   # by first replacing the spaces with newlines so that "sort" can
>   # be used, and then collecting the output back onto one line.
>   function l10n_sort_spaces() {
>     tr -s "[:space:]" "\n" <<< "${1}" | sort | xargs echo
>   }
> 
> You should also document why you're doing that nearby, since like you
> said, the eclass assumes that PLOCALES is sorted. Someone will wonder
> why you're doing it again.

I feel an extra function for something we're only doing once is a bit
redundant. I was happy to add a short explanation though so I did that.
Merged it now.

-- 
James Le Cuirot (chewi)
Gentoo Linux Developer

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 951 bytes --]