[patch 1/3] check-accelerator-conflicts.pl v1.5 suggests better and is more verbose.

check-accelerator-conflicts.pl v1.5 suggests better and is more verbose.

The --msgid-fallback mode still reports conflicts between msgid
and msgstr strings, but the resulting suggestions can now include
accelerators reserved by msgids; thus the translator won't be
lured into avoiding accelerators that she'll be likely to replace
anyway.  Therefore, --msgid-fallback is now the default and can
be turned off with the new option --no-msgid-fallback.

After checking each file, check-accelerator-conflicts.pl displays
a one-line summary of how many accelerators it checked and how
many conflicts it found.  This is primarily meant to warn people
by displaying zeroes if if gather-accelerator-contexts.pl has not
been run.

Locale::PO::dump doesn't output an msgid if there isn't one.

diff --git a/po/perl/check-accelerator-conflicts.pl b/po/perl/check-accelerator-conflicts.pl
index b72bd75..dea809e 100755
--- a/po/perl/check-accelerator-conflicts.pl
+++ b/po/perl/check-accelerator-conflicts.pl
@@ -7,7 +7,7 @@
 use Getopt::Long qw(GetOptions :config bundling gnu_compat);
 use autouse 'Pod::Usage' => qw(pod2usage);
 
-my $VERSION = "1.4";
+my $VERSION = "1.5";
 
 sub show_version
 {
@@ -21,8 +21,9 @@ sub show_version
 my $Opt_accelerator_tag;
 
 # True if, for missing or fuzzy translations, the msgid string should
-# be checked instead of msgstr.  Set with the --msgid-fallback option.
-my $Opt_msgid_fallback;
+# be checked instead of msgstr.  Set with the --msgid-fallback and
+# --no-msgid-fallback options.
+my $Opt_msgid_fallback = 1;
 
 sub acceleration_arrays_eq ($$)
 {
@@ -37,9 +38,43 @@ sub acceleration_arrays_eq ($$)
 
 sub check_po_file ($)
 {
+    # The name of the PO file to be checked.
     my($po_file_name) = @_;
+
+    # A nested hash that lists the accelerators and their uses.
+    # $accelerators{$accelerator}{$ctxname}{ACCELERATIONS}[$i]{LINENO}
+    # 1. In %accelerators, the keys are one-character strings,
+    #    and the values are hash references.
+    # 2. In %{$accelerators{$accelerator}}, the keys are names
+    #    of contexts in which the accelerator is used, and the
+    #    values are references to "crossing" hashes.
+    # 3. %{$accelerators{$accelerator}{$ctxname}} is a "crossing" hash,
+    #    so named because it describes how an accelerator and a context
+    #    cross each other.  It has the following elements:
+    #    (ACCELERATIONS => [see point 4 below],
+    #     REPORTED => (1 if this is a conflict and has been reported),
+    #     AVOID => (1 if this accelerator should not be suggested in this
+    #               context))
+    # 4. @{$accelerators{$accelerator}{$ctxname}{ACCELERATIONS}} is a list of
+    #    references to "acceleration" hashes.  If the same acceleration occurs
+    #    in multiple contexts, then the references are to the same hash.
+    # 5. %{$accelerators{$accelerator}{ctxname}{ACCELERATIONS}[$i]} is an
+    #    "acceleration" hash.  It has the following structure:
+    #    (PO => (the Locale::PO object),
+    #     CTXNAMES => [read-only list of names of contexts in which the
+    #                  accelerator is used],
+    #     ACCELERATOR => (a one-character string),
+    #     LINENO => (line number in the PO file),
+    #     STRING => (the msgid or msgstr string that defines the accelerator;
+    #                unquoted as much as possible),
+    #     EXPLAIN => (a string to be displayed if a conflict is found))
     my %accelerators;
-    my $warnings = 0;
+
+    # How many entries had checkable accelerators.
+    my $checkable_count = 0;
+
+    # How many conflicts have been found so far.
+    my $conflict_count = 0;
 
     {
 	my $pos = Locale::PO->load_file_asarray($po_file_name)
@@ -76,10 +111,12 @@ sub check_po_file ($)
  	    	}
  	    }
 
+	    $checkable_count++ if @accelerations;
 	    foreach my $acceleration (@accelerations) {
-		foreach my $ctxname (@ctxnames) {
-		    push(@{$accelerators{uc $acceleration->{ACCELERATOR}}{$ctxname}},
-			 $acceleration);
+		my $accelerator = uc($acceleration->{ACCELERATOR});
+		foreach my $crossing (@{$accelerators{$accelerator}}{@ctxnames}) {
+		    push @{$crossing->{ACCELERATIONS}}, $acceleration;
+		    $crossing->{AVOID} = 1 if $acceleration->{EXPLAIN} =~ /^msgstr/;
 		}
 	    }
 	}
@@ -87,16 +124,16 @@ sub check_po_file ($)
 
     foreach my $accelerator (sort keys %accelerators) {
 	my $ctxhash = $accelerators{$accelerator};
-	foreach my $outer_ctxname (sort keys %$ctxhash) {
-	    # Cannot use "foreach my $accelerations" directly, because
-	    # $accelerations would then become an alias and change to 0 below.
-	    my $accelerations = $ctxhash->{$outer_ctxname};
-	    if (ref($accelerations) eq "ARRAY" && @$accelerations > 1) {
+	foreach my $crossing (@{$ctxhash}{sort keys %$ctxhash}) {
+	    my $accelerations = $crossing->{ACCELERATIONS};
+	    if ($accelerations && @$accelerations > 1 && !$crossing->{REPORTED}) {
 		my @ctxnames_in_conflict;
-		foreach my $ctxname (sort keys %$ctxhash) {
-		    if (acceleration_arrays_eq($ctxhash->{$ctxname}, $accelerations)) {
-			push @ctxnames_in_conflict, $ctxname;
-			$ctxhash->{$ctxname} = 0;
+		foreach my $inner_ctxname (keys %$ctxhash) {
+		    my $inner_crossing = $ctxhash->{$inner_ctxname};
+		    if (acceleration_arrays_eq($inner_crossing->{ACCELERATIONS},
+					       $accelerations)) {
+			push @ctxnames_in_conflict, $inner_ctxname;
+			$inner_crossing->{REPORTED} = 1;
 		    }
 		}
 		my $ctxnames_in_conflict = join(", ", map(qq("$_"), @ctxnames_in_conflict));
@@ -120,7 +157,7 @@ sub check_po_file ($)
 		    SUGGESTION: foreach my $char (split(//, $suggestions)) {
 			foreach my $ctxname (@{$acceleration->{CTXNAMES}}) {
 			    $suggestions =~ s/\Q$char\E//, next SUGGESTION
-				if exists $accelerators{uc($char)}{$ctxname};
+				if $accelerators{uc($char)}{$ctxname}{AVOID};
 			}
 		    }
 
@@ -131,12 +168,18 @@ sub check_po_file ($)
 		    else {
 			warn "$po_file_name:$lineno: suggestions: $suggestions\n";
 		    }
-		}
-		$warnings++;
+		} # foreach $acceleration in conflict
+		$conflict_count++;
 	    } # if found a conflict 
 	} # foreach context known for $accelerator
     } # foreach $accelerator
-    return $warnings ? 1 : 0;
+
+    print "$po_file_name: "
+	. ($checkable_count == 1 ? "Checked 1 entry" : "Checked $checkable_count entries")
+	. ", "
+	. ($conflict_count == 1 ? "found 1 accelerator conflict" : "found $conflict_count accelerator conflicts")
+	. ".\n";
+    return $conflict_count ? 1 : 0;
 }
 
 GetOptions("accelerator-tag=s" => sub {
@@ -147,7 +190,7 @@ GetOptions("accelerator-tag=s" => sub {
 		   if length($value) != 1;
 	       $Opt_accelerator_tag = $value;
 	   },
-	   "msgid-fallback" => \$Opt_msgid_fallback,
+	   "msgid-fallback!" => \$Opt_msgid_fallback,
 	   "help" => sub { pod2usage({-verbose => 1, -exitval => 0}) },
 	   "version" => \&show_version)
     or exit 2;
@@ -212,15 +255,20 @@
 
 =item B<--msgid-fallback>
 
-If the C<msgstr> is empty or the entry is fuzzy, check the C<msgid>
-instead.  Without this option, B<check-accelerator-conflicts.pl>
-completely ignores such entries.
-
-This option also causes B<check-accelerator-conflicts.pl> not to
-suggest accelerators that would conflict with a C<msgid> that was thus
-checked.  Following these suggestions may lead to bad choices for
-accelerators, because the conflicting C<msgid> will eventually be
-shadowed by a C<msgstr> that may use a different accelerator.
+=item B<--no-msgid-fallback>
+
+Select how to check entries where the C<msgstr> is missing or fuzzy.
+The default is B<--msgid-fallback>, which makes
+B<check-accelerator-conflicts.pl> use the C<msgid> instead, and report
+any conflicts between C<msgid> and C<msgstr> strings.  The alternative
+is B<--no-msgid-fallback>, which makes B<check-accelerator-conflicts.pl>
+completely ignore such entries.
+
+Regardless of these options, B<check-accelerator-conflicts.pl> will
+suggest accelerators that would conflict with ones defined in C<msgid>
+strings.  Those strings will be eventually shadowed by C<msgstr>
+strings, so their accelerators should not affect which accelerators
+the translator chooses for C<msgstr> strings.
 
 =back
 
diff --git a/po/perl/Locale/PO.pm b/po/perl/Locale/PO.pm
index c141630..d3d7864 100644
--- a/po/perl/Locale/PO.pm
+++ b/po/perl/Locale/PO.pm
@@ -308,11 +308,13 @@ sub dump {
     $self->_update_flagstr();
     $dump .= $self->_dump_multi_comment( $self->{'_flagstr'}, "#, " )
       if defined( $self->{'_flagstr'} );
-    $dump .= "msgid " . $self->_normalize_str( $self->msgid );
+    $dump .= "msgid " . $self->_normalize_str( $self->msgid )
+      if $self->msgid;
     $dump .= "msgid_plural " . $self->_normalize_str( $self->msgid_plural )
       if $self->msgid_plural;
 
-    $dump .= "msgstr " . $self->_normalize_str( $self->msgstr ) if $self->msgstr;
+    $dump .= "msgstr " . $self->_normalize_str( $self->msgstr )
+	if $self->msgstr;
 
     if ( my $msgstr_n = $self->msgstr_n ) {
         $dump .= "msgstr[$_] " . $self->_normalize_str( $$msgstr_n{$_} )
@@ -1017,7 +1019,7 @@
 Documented that C<msgstr> normally returns C<undef> if there are plurals.
 Documented the new methods C<msgid_begin_lineno> and C<msgstr_begin_lineno>.
 
-=item Z<>2006-02-28  Kalle Olavi Niemitalo  <[email protected]>
+=item Z<>2006-02-18  Kalle Olavi Niemitalo  <[email protected]>
 
 Locale::PO now preserves unrecognized flags, although there is still no documented way to access them.  It also preserves the order of flags, if no flags are modified.  Replaced the C<fuzzy>, C<c_format>, and C<php_format> fields with C<_flaghash>, and renamed the C<_flag> field to C<_flagstr>.
 Flag-setting functions silently map unsupported values (e.g. 42) to supported ones (e.g. 1), which they also return.
@@ -1027,6 +1029,10 @@
 POD changes:
 Unlisted the bugs that have now been fixed.
 
+=item Z<>2006-02-19  Kalle Olavi Niemitalo  <[email protected]>
+
+The C<dump> method doesn't output an C<msgid> if there isn't one. 
+
 =back
 
 =head1 COPYRIGHT AND LICENSE

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.2 (GNU/Linux)

iD8DBQBEEw/pHm9IGt60eMgRAjQfAJoD7DHqAL3FoATzPX8q7iL7dHVCyQCdE1Vi
q+SlsuO5eTkYZaJmcURGZWU=
=r4Wm
-----END PGP SIGNATURE-----