comments inline: On 9/12/18 2:52 PM, Rhonda D'Vine wrote: > The iso 3166-1 information in the iso-codes package seems to be more > up to par than the one that was used from tzdata. > > Signed-off-by: Rhonda D'Vine <rho...@proxmox.com> > --- > country.pl | 18 ++++++++++++------ > debian/control | 1 + > 2 files changed, 13 insertions(+), 6 deletions(-) > > diff --git a/country.pl b/country.pl > index 277fc34..b149b5b 100755 > --- a/country.pl > +++ b/country.pl > @@ -4,6 +4,7 @@ use strict; > use warnings; > > use PVE::Tools; > +use JSON; > > # see also: http://en.wikipedia.org/wiki/Keyboard_layout > # > @@ -14,14 +15,19 @@ use PVE::Tools; > > my $country = {}; > > -my $line; > -open (TMP, "</usr/share/zoneinfo/iso3166.tab"); > -while (defined ($line = <TMP>)) { > - if ($line =~ m/^([A-Z][A-Z])\s+(.*\S)\s*$/) { > - $country->{lc($1)} = $2; > - } > +open (TMP, "</usr/share/iso-codes/json/iso_3166-1.json");
you missed to change the comment (out of context, at start of script): # country codes from: /usr/share/zoneinfo/iso3166.tab > +my $json_data; > +{ > + local $/; > + $json_data = <TMP>; > } > close (TMP); you also change how we read a file (with no mentioning of that in the commit message). And we have helpers for reading/writing files in PVE::Tools, known to be good handling various corner cases. As PVE::Tools is already included I'd suggest doing something like: my $raw = PVE::Tools::file_get_contents("/usr/share/zoneinfo/iso3166.tab", 128*1024); if this needs change anyway. > +$json_data = from_json($json_data); variable use for different stuff, and json_data is a too generic name, one has no idea what could be possible contained in this variable reading only the name... Use something more telling like $iso_codes_raw for the raw file and $iso_codes for the (JSON) parsed hash. And maybe add a $country_codes = $iso_codes->{'3166-1'} variable, it's the only thing in there and that would make code below a bit shorter/easier to read. > + > +foreach my $entry (@{$json_data->{'3166-1'}}) { > + $country->{lc($entry->{'alpha_2'})} = defined $entry->{'common_name'} ? > + $entry->{'common_name'} : $entry->{'name'}; > +} could be also written as: my $country = { map { lc($_->{alpha_2}) => $_->{common_name} // $_->{name} } @{$json_data->{'3166-1'}}; }; but, while map is in general faster on such things, I have no problem with the foreach. But something like the following in the loop body: my $code = lc($entry->{'alpha_2'}); $country->{$code} = $entry->{common_name} // $entry->{name}; is much easier and quicker to read, IMO. > > # we need mappings for X11, console, and kvm vnc > > diff --git a/debian/control b/debian/control > index 60bc8b0..7ab4515 100644 > --- a/debian/control > +++ b/debian/control > @@ -3,6 +3,7 @@ Section: perl > Priority: optional > Maintainer: Proxmox Support Team <supp...@proxmox.com> > Build-Depends: debhelper (>= 9), > + iso-codes, > libpve-common-perl, > librsvg2-bin, > perl (>= 5.10.0-19), > _______________________________________________ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel