New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[PATCH] Locale::Maketext One Sided Lexicons #10476
Comments
From @toddrCreated by @toddrThis patch with tests provides One Sided Lexicons The advantages are a smaller file, less prone to mistyping or mispasting, and most Perl Info
|
From @toddrInline Patchdiff --git a/dist/Locale-Maketext/lib/Locale/Maketext.pm b/dist/Locale-Maketext/lib/Locale/Maketext.pm
index 7a10ffb..dae6b5c 100644
--- a/dist/Locale-Maketext/lib/Locale/Maketext.pm
+++ b/dist/Locale-Maketext/lib/Locale/Maketext.pm
@@ -25,6 +25,7 @@ $USE_LITERALS = 1 unless defined $USE_LITERALS;
# a hint for compiling bracket-notation things.
my %isa_scan = ();
+my %isa_ones = ();
###########################################################################
@@ -186,18 +187,29 @@ sub maketext {
# Look up the value:
my $value;
+ my $ns = ref($handle) || $handle;
if (exists $handle->{'_external_lex_cache'}{$phrase}) {
DEBUG and warn "* Using external lex cache version of \"$phrase\"\n";
$value = $handle->{'_external_lex_cache'}{$phrase};
}
else {
foreach my $h_r (
- @{ $isa_scan{ref($handle) || $handle} || $handle->_lex_refs }
+ @{ $isa_scan{$ns} || $handle->_lex_refs }
) {
DEBUG and warn "* Looking up \"$phrase\" in $h_r\n";
if(exists $h_r->{$phrase}) {
DEBUG and warn " Found \"$phrase\" in $h_r\n";
unless(ref($value = $h_r->{$phrase})) {
+ # begin $Onesided
+ if ( !defined $value || $value eq '' ) {
+ DEBUG and warn " value is undef or ''";
+ if ( $isa_ones{"$h_r"} ) {
+ DEBUG and warn " $ns ($h_r) is Onesided and \"$phrase\" entry is undef or ''\n";
+ $value = $phrase;
+ }
+ }
+ # end $Onesided
+
# Nonref means it's not yet compiled. Compile and replace.
if ($handle->{'use_external_lex_cache'}) {
$value = $handle->{'_external_lex_cache'}{$phrase} = $handle->_compile($value);
@@ -452,6 +464,7 @@ sub _lex_refs { # report the lexicon references for this handle's class
if( defined( *{$class . '::Lexicon'}{'HASH'} )) {
push @lex_refs, *{$class . '::Lexicon'}{'HASH'};
+ $isa_ones{"$lex_refs[-1]"} = defined ${$class . '::Onesided'} && ${$class . '::Onesided'} ? 1 : 0;
DEBUG and warn '%' . $class . '::Lexicon contains ',
scalar(keys %{$class . '::Lexicon'}), " entries\n";
}
diff --git a/dist/Locale-Maketext/lib/Locale/Maketext.pod b/dist/Locale-Maketext/lib/Locale/Maketext.pod
index 14b47c8..92703d6 100644
--- a/dist/Locale-Maketext/lib/Locale/Maketext.pod
+++ b/dist/Locale-Maketext/lib/Locale/Maketext.pod
@@ -956,6 +956,34 @@ All you need to do is turn on caching outside of the lexicon hash itself like so
And then instead of storing the compiled value in the lexicon hash it will store it in $lh->{'_external_lex_cache'}
+=head1 ONE SIDED LEXICONS
+
+Setting the package variable $Onesided to a true value treats the class's %Lexicon as one sided. What that means is if the hash's keys and values will be the same (e.g. your main Lexicon or known-but-as-yet-untranslated strings) you can specify it in the key only and leave the value blank.
+
+So instead of a Lexicon entry like this:
+
+ package YourProjClass::en_us;
+ our %Lexicon = {
+ q{Pease porridge hot} => q{Pease porridge hot},
+ q{Pease porridge cold} => q{Pease porridge cold},
+ q{Some like it hot} => q{Some like it hot},
+ ...
+ }
+
+You just do:
+
+ package YourProjClass::en_us;
+ our $Onesided = 1;
+ our %Lexicon = {
+ q{Pease porridge hot} => '',
+ q{Pease porridge cold} => '',
+ q{Some like it hot} => '',
+ ...
+ }
+
+The advantages are a smaller file, less prone to mistyping or mispasting, and
+most important of all someone translating it can simply copy it into their module and enter their translation instead of having to remove the value first.
+
=head1 CONTROLLING LOOKUP FAILURE
If you call $lh->maketext(I<key>, ...parameters...),
diff --git a/dist/Locale-Maketext/t/91_onesided_lexicons.t b/dist/Locale-Maketext/t/91_onesided_lexicons.t
new file mode 100644
index 0000000..bd933bf
--- /dev/null
+++ b/dist/Locale-Maketext/t/91_onesided_lexicons.t
@@ -0,0 +1,58 @@
+use Test::More tests => 12;
+
+BEGIN {
+ use_ok('Locale::Maketext');
+};
+
+{
+ package TestApp::Localize;
+ our @ISA = ('Locale::Maketext');
+ our %Lexicon = ('One Side' => 'I am not one sides');
+}
+{
+ package TestApp::Localize::en;
+ our @ISA = ('TestApp::Localize');
+}
+{
+ package TestApp::Localize::i_oneside;
+ our @ISA = ('TestApp::Localize');
+ our $Onesided = 1;
+ our %Lexicon = (
+ 'One Side' => '',
+ );
+}
+
+my $norm = TestApp::Localize->get_handle('en');
+is($norm->maketext('One Side'), 'I am not one sides', 'non $Onesided returns proper value');
+
+my $oneside = TestApp::Localize->get_handle('i_oneside');
+is($TestApp::Localize::i_oneside::Lexicon{'One Side'}, '', '$Onesided untouched initially');
+is($oneside->maketext('One Side'), 'One Side', 'Once used $Onesided returns proper value');
+is(ref $TestApp::Localize::i_oneside::Lexicon{'One Side'}, 'SCALAR', 'Once used $Onesided does lexicon (sanity check that it is not just falling back)');
+is(${$TestApp::Localize::i_oneside::Lexicon{'One Side'}}, 'One Side', 'ref holds corect string');
+
+{
+ package TestAppX::Localize;
+ our @ISA = ('Locale::Maketext');
+ our $Onesided = 1;
+ our %Lexicon = ('One Side' => '');
+}
+{
+ package TestAppX::Localize::en;
+ our @ISA = ('TestAppX::Localize');
+}
+{
+ package TestAppX::Localize::i_oneside;
+ our @ISA = ('TestAppX::Localize');
+ our %Lexicon = ();
+}
+
+my $normx = TestAppX::Localize->get_handle('en');
+is($TestAppX::Localize::Lexicon{'One Side'}, '', 'OS parent : $Onesided untouched initially');
+is($normx->maketext('One Side'), 'One Side', 'OS parent : Once used $Onesided returns proper value');
+is($normx->maketext('One Side'), 'One Side', 'OS parent : parent $Onesided, parent returns proper value');
+
+my $onesidex = TestAppX::Localize->get_handle('i_oneside');
+is($onesidex->maketext('One Side'), 'One Side', 'OS parent : parent $Onesidedm, child returns proper value');
+is(ref $TestAppX::Localize::Lexicon{'One Side'}, 'SCALAR', 'OS parent : Once used $Onesided does lexicon (sanity check that it is not just falling back)');
+is(${$TestAppX::Localize::Lexicon{'One Side'}}, 'One Side', 'OS parent : ref holds corect string'); |
From @obraOn Tue 6.Jul'10 at 9:43:59 -0700, Todd Rinaldo wrote:
Locale::Maketext already has functionality to do what you want. The value of having your translators hand-copy the content of your Best, |
The RT System itself - Status changed from 'new' to 'open' |
From @toddrOn Jul 8, 2010, at 8:57 AM, Jesse Vincent wrote:
Jesse, Onesided-ness is a complementary feature to _AUTO, not a replacement. When you have code that uses _AUTO, Locale fall back happens. The only way to prevent this is to inject a key with an equivalent value to prevent fallback. Sometimes unexpected defaulting might happen. This isn't something you'd use all the time, but in the cases where you do, onesided-ness is more "fat-finger safe" than duping key and value pairs. Having key/value pairs is also useful when you have _AUTO off. In those cases, the only way have MT not break is to put in a key with an equivalent value. Again, you're more "fat-finger safe" by using one sidedness in those cases. The added complexity only happens once when the statement is compiled, so it shouldn't be that expensive from a code path standpoint. Thanks, |
From @obraOn Thu, Jul 15, 2010 at 10:37:19AM -0500, Todd Rinaldo wrote:
This just feels like feature creep that might better belong in a And for that matter, why not add an option that works like _AUTO rather |
From @toddrOn Jul 15, 2010, at 12:54 PM, Jesse Vincent wrote:
The only way to subclass it is a monolithic copy/paste. I don't see that it being added hurts anything existing. Even if it is feature creep, what's the down side of that? Honestly I see it as a part of L:M or something else that has to re-implement or duplicate the whole method. That would be a code fork, which I'm trying to avoid here.
The goal was to avoid polluting the lexicon namespace with another special variable. Since each lexicon is it's own package, it made more sense to put it outside the lexicon. If that's really what's causing you to hesitate, I have no issues providing an amended patch which uses the lexicon instead to look for the _ONE_SIDED flag. Todd |
From @obra
That sounds like something it would be great to fix.
It's certainly _one_ thing that's causing me to hesitate. I'd rather a Jesse
-- |
From a.r.ferreira@gmail.comOn Thu, Jul 22, 2010 at 3:31 PM, Todd Rinaldo <toddr@cpanel.net> wrote:
The down side is to slow down Locale::Maketext in the common usage There are HTML sites that perform thousands of L::M lookups per page Notice that new conditionals like if (exists $self->{external_lexicon}) { are the kind of thing that concerns me.
|
From a.r.ferreira@gmail.comOn Thu, Jul 22, 2010 at 9:35 PM, Jesse Vincent <jesse@fsck.com> wrote:
I agree whole-heartily with that. By doing the magic necessary to Of course, that's not an easy task. I already suggested to the Sorry for being so conservative, but I really believe that Best,
|
From @toddrOn Jul 23, 2010, at 10:02 AM, Adriano Ferreira wrote:
Thanks everyone for your advice on this one. It's always surprising what'll spark a thread. I agree with your points. The last thing I want is to slow this critical module down. Look for a patch from me in a month or 2 on re-factoring L:M to make it simpler to subclass. Thanks, |
From @toddrI recommend this ticket be closed as won't fix. I'm working on a patch to make this module |
From @cpansproutOn Fri Aug 06 09:55:53 2010, TODDR wrote:
I’m marking it as rejected. |
@cpansprout - Status changed from 'open' to 'rejected' |
Migrated from rt.perl.org#76402 (status was 'rejected')
Searchable as RT76402$
The text was updated successfully, but these errors were encountered: