Skip to content
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

Closed
p5pRT opened this issue Jul 6, 2010 · 14 comments
Closed

[PATCH] Locale::Maketext One Sided Lexicons #10476

p5pRT opened this issue Jul 6, 2010 · 14 comments

Comments

@p5pRT
Copy link
Collaborator

@p5pRT p5pRT commented Jul 6, 2010

Migrated from rt.perl.org#76402 (status was 'rejected')

Searchable as RT76402$

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Jul 6, 2010

From @toddr

Created by @toddr

This patch with tests provides One Sided Lexicons
 
Setting $Onesided = 1 in your package treats the class's %Lexicon as one sided.
What that means is if the hash's keys and values will be the same (IE your main
Lexicon) you can specify it in the key only and leave the value blank.

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.

Perl Info

Flags:
   category=library
   severity=low
   module=Locale::Maketext

Site configuration information for perl 5.12.1:

Configured by cPanel at Mon Jun  7 11:16:28 CDT 2010.

Summary of my perl5 (revision 5 version 12 subversion 1) configuration:

 Platform:
   osname=linux, osvers=2.6.18-194.3.1.el5, archname=x86_64-linux
   uname='linux rpmb-centos-50-64bit 2.6.18-194.3.1.el5 #1 smp thu may 13 13:08:30 edt 2010 x86_64 x86_64 x86_64 gnulinux '
   config_args='-des -Darchname=x86_64-linux -Dcc=/usr/local/cpanel/bin/gcc -Dcpp=/usr/local/cpanel/bin/gcc -E -DDEBUGGING=none -Doptimize=-Os -Dusemymalloc=y -Duseshrplib=true -Duselargefiles=yes -Duseposix=true -Dhint=recommended -Duseperlio=yes -Dccflags=-I/usr/local/cpanel/include -L/usr/local/cpanel/lib64 -Wl,-rpath -Wl,/usr/local/cpanel/lib64  -Dcppflags=-I/usr/local/cpanel/include -L/usr/local/cpanel/lib64 -Dldflags=-Wl,-rpath -Wl,/usr/local/cpanel/lib64 -L/usr/local/cpanel/lib64 -Dprefix=/usr/local/cpanel -Dsiteprefix=/usr/local/cpanel -Dsitebin=/usr/local/cpanel/bin -Dsitelib=/usr/local/cpanel/lib64/perl5/site_lib -Dprivlib=/usr/local/cpanel/lib64/perl5/5.12.1 -Dotherlibdirs=/var/cpanel/perl5/lib:/usr/local/cpanel/lib64/perl5/cpanel -Dman1dir=/usr/local/cpanel/share/man/man1 -Dman3dir=/usr/local/cpanel/share/man/man3 -Dsiteman1dir=/usr/local/cpanel/share/man/man1 -Dsiteman3dir=/usr/local/cpanel/share/man/man3 -Dcf_by=cPanel -Dmyhostname=localhost -Dperladmin=root@localhost -Dcf_email=support@cpanel.net -Di_dbm=/usr/local/cpanel/include -Di_gdbm=/usr/local/cpanel/include -Di_ndbm=/usr/local/cpanel/include -Ud_dosuid -Uuserelocatableinc -Umad -Uusethreads -Uusemultiplicity -Uusesocks -Uuselongdouble -Ui_db -Aldflags=-L/usr/local/cpanel/lib64 -L/usr/lib64 -L/lib64 -lgdbm -Dlocincpth=/usr/local/cpanel/include /usr/local/include  -Acflags=-fPIC -DPIC -m64 -I/usr/local/cpanel/include -Dlibpth=/usr/local/cpanel/lib64 /usr/local/lib64 /usr/local/lib /lib64 /usr/lib64  -Duse64bitint=yes -Duse64bitall=yes'
   hint=recommended, useposix=true, d_sigaction=define
   useithreads=undef, usemultiplicity=undef
   useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef
   use64bitint=define, use64bitall=define, uselongdouble=undef
   usemymalloc=y, bincompat5005=undef
 Compiler:
   cc='/usr/local/cpanel/bin/gcc', ccflags ='-I/usr/local/cpanel/include -L/usr/local/cpanel/lib64 -Wl,-rpath -Wl,/usr/local/cpanel/lib64 -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/cpanel/include -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64',
   optimize='-Os',
   cppflags='-I/usr/local/cpanel/include -L/usr/local/cpanel/lib64 -I/usr/local/cpanel/include -L/usr/local/cpanel/lib64 -Wl,-rpath -Wl,/usr/local/cpanel/lib64 -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/cpanel/include -I/usr/local/include'
   ccversion='', gccversion='4.5.0', gccosandvers=''
   intsize=4, longsize=8, ptrsize=8, doublesize=8, byteorder=12345678
   d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=16
   ivtype='long', ivsize=8, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8
   alignbytes=8, prototype=define
 Linker and Libraries:
   ld='/usr/local/cpanel/bin/gcc', ldflags ='-Wl,-rpath -Wl,/usr/local/cpanel/lib64 -L/usr/local/cpanel/lib64 -L/usr/local/cpanel/lib64 -L/usr/lib64 -L/lib64 -lgdbm -fstack-protector -L/usr/local/lib'
   libpth=/usr/local/cpanel/lib64 /usr/local/lib64 /usr/local/lib /lib64 /usr/lib64
   libs=-lnsl -lgdbm -ldb -ldl -lm -lcrypt -lutil -lc -lgdbm_compat
   perllibs=-lnsl -ldl -lm -lcrypt -lutil -lc
   libc=/lib/libc-2.5.so, so=so, useshrplib=true, libperl=libperl.so
   gnulibc_version='2.5'
 Dynamic Linking:
   dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E -Wl,-rpath,/usr/local/cpanel/lib64/perl5/5.12.1/x86_64-linux/CORE'
   cccdlflags='-fPIC', lddlflags='-shared -Os -L/usr/local/cpanel/lib64 -L/usr/lib64 -L/lib64 -L/usr/local/lib -fstack-protector'

Locally applied patches:
   cPanel Patches


@INC for perl 5.12.1:
   /usr/local/cpanel/lib64/perl5/site_lib/x86_64-linux
   /usr/local/cpanel/lib64/perl5/site_lib
   /usr/local/cpanel/lib64/perl5/5.12.1/x86_64-linux
   /usr/local/cpanel/lib64/perl5/5.12.1
   /var/cpanel/perl5/lib
   /usr/local/cpanel/lib64/perl5/cpanel
   .


Environment for perl 5.12.1:
   HOME=/home/toddr
   LANG=en_US.UTF-8
   LANGUAGE (unset)
   LD_LIBRARY_PATH (unset)
   LOGDIR (unset)
   PATH=/usr/local/cpanel/bin:/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin:/usr/local/bin:/usr/X11/bin:
   PERL_BADLANG (unset)
   SHELL=/bin/zsh


@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Jul 6, 2010

From @toddr

Inline Patch
diff --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');
@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Jul 8, 2010

From @obra

On Tue 6.Jul'10 at 9​:43​:59 -0700, Todd Rinaldo wrote​:

# New Ticket Created by Todd Rinaldo
# Please include the string​: [perl #76402]
# in the subject line of all future correspondence about this issue.
# <URL​: http​://rt.perl.org/rt3/Ticket/Display.html?id=76402 >

This is a bug report for perl from toddr@​cpanel.net,
generated with the help of perlbug 1.39 running under perl 5.12.1.

-----------------------------------------------------------------
[Please describe your issue here]

This patch with tests provides One Sided Lexicons

Setting $Onesided = 1 in your package treats the class's %Lexicon as one sided.
What that means is if the hash's keys and values will be the same (IE your main
Lexicon) you can specify it in the key only and leave the value blank.

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.

Locale​::Maketext already has functionality to do what you want.
The _AUTO meta-entry in %Lexicon allows fallthrough to use the requested
$key as the $value. And then your base lexicon uses even _less_ memory.

The value of having your translators hand-copy the content of your
default lexicon doesn't feel like much of a win for the added
complexity.

Best,
Jesse

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Jul 8, 2010

The RT System itself - Status changed from 'new' to 'open'

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Jul 15, 2010

From @toddr

On Jul 8, 2010, at 8​:57 AM, Jesse Vincent wrote​:

On Tue 6.Jul'10 at 9​:43​:59 -0700, Todd Rinaldo wrote​:

# <URL​: http​://rt.perl.org/rt3/Ticket/Display.html?id=76402 >

Locale​::Maketext already has functionality to do what you want.
The _AUTO meta-entry in %Lexicon allows fallthrough to use the requested
$key as the $value. And then your base lexicon uses even _less_ memory.

The value of having your translators hand-copy the content of your
default lexicon doesn't feel like much of a win for the added
complexity.

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,
Todd Rinaldo

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Jul 15, 2010

From @obra

On Thu, Jul 15, 2010 at 10​:37​:19AM -0500, Todd Rinaldo wrote​:

On 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.

This just feels like feature creep that might better belong in a
subclass.

And for that matter, why not add an option that works like _AUTO rather
than all the duplicated entries?

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Jul 22, 2010

From @toddr

On Jul 15, 2010, at 12​:54 PM, Jesse Vincent wrote​:

On Thu, Jul 15, 2010 at 10​:37​:19AM -0500, Todd Rinaldo wrote​:

On 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.

This just feels like feature creep that might better belong in a subclass.

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.

And for that matter, why not add an option that works like _AUTO rather
than all the duplicated entries?

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

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Jul 23, 2010

From @obra

The only way to subclass it is a monolithic copy/paste. I don't see that it being added hurts anything existing.

That sounds like something it would be great to fix.

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.

It's certainly _one_ thing that's causing me to hesitate. I'd rather a
special key in the lexicon that works just like the other special key in
the lexicon than something similar that's different. But see above.

Jesse

Todd

--

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Jul 23, 2010

From a.r.ferreira@gmail.com

On Thu, Jul 22, 2010 at 3​:31 PM, Todd Rinaldo <toddr@​cpanel.net> 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 down side is to slow down Locale​::Maketext in the common usage
that applications do with it on the wild.

There are HTML sites that perform thousands of L​::M lookups per page
and any light change can compromise performance in such applications.
And such compromise comes with no benefits for these applications.

Notice that new conditionals like

  if (exists $self->{external_lexicon}) {
  }
  else {
  # previous code
  }

are the kind of thing that concerns me.

And for that matter, why not add an option that works like _AUTO rather
than all the duplicated entries?

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

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Jul 23, 2010

From a.r.ferreira@gmail.com

On Thu, Jul 22, 2010 at 9​:35 PM, Jesse Vincent <jesse@​fsck.com> wrote​:

The only way to subclass it is a monolithic copy/paste. I don't see that it being added hurts anything existing.

That sounds like something it would be great to fix.

I agree whole-heartily with that. By doing the magic necessary to
disentangle the guts of Locale​::Maketext from the original
implementation idiosyncrasies (which are for example the use of simple
Perl hashes and replacing its entries as they get interpreted /
compiled), that would be a promising path for the evolution of the
module without inadvertent effects to what already works with
Locale​::Maketext.

Of course, that's not an easy task. I already suggested to the
tickets' original requestor that forking Locale​::Maketext, approaching
the inability to extend it which is caused by the actual code
structure, and offering such rewrite so it can prove its power / value
(by benchmarks and by being applied to actual code / applications),
that would gives us confidence to think about replacing the original
module.

Sorry for being so conservative, but I really believe that
Locale​::Maketext code should not grow to be more complicate (and
expensive in performance and maintenance efforts) that it already is.

Best,
Adriano

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.

It's certainly _one_ thing that's causing me to hesitate. I'd rather a
special key in the lexicon that works just like the other special key in
the lexicon than something similar that's different. But see above.

Jesse

Todd

--

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Aug 6, 2010

From @toddr

On Jul 23, 2010, at 10​:02 AM, Adriano Ferreira wrote​:

On Thu, Jul 22, 2010 at 9​:35 PM, Jesse Vincent <jesse@​fsck.com> wrote​:

The only way to subclass it is a monolithic copy/paste. I don't see that it being added hurts anything existing.

That sounds like something it would be great to fix.

I agree whole-heartily with that. By doing the magic necessary to
disentangle the guts of Locale​::Maketext from the original
implementation idiosyncrasies (which are for example the use of simple
Perl hashes and replacing its entries as they get interpreted /
compiled), that would be a promising path for the evolution of the
module without inadvertent effects to what already works with
Locale​::Maketext.

Of course, that's not an easy task. I already suggested to the
tickets' original requestor that forking Locale​::Maketext, approaching
the inability to extend it which is caused by the actual code
structure, and offering such rewrite so it can prove its power / value
(by benchmarks and by being applied to actual code / applications),
that would gives us confidence to think about replacing the original
module.

Sorry for being so conservative, but I really believe that
Locale​::Maketext code should not grow to be more complicate (and
expensive in performance and maintenance efforts) that it already is.

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,
Todd

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Aug 6, 2010

From @toddr

I recommend this ticket be closed as won't fix. I'm working on a patch to make this module
more easily subclassed, which I think is a better strategy.

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Sep 25, 2010

From @cpansprout

On Fri Aug 06 09​:55​:53 2010, TODDR wrote​:

I recommend this ticket be closed as won't fix. I'm working on a patch
to make this module
more easily subclassed, which I think is a better strategy.

I’m marking it as rejected.

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Sep 25, 2010

@cpansprout - Status changed from 'open' to 'rejected'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
1 participant
You can’t perform that action at this time.