On Tue, Feb 19, 2013 at 02:46:02PM +0000, Dave Cross wrote: > Hi, > > However, I think that you could probably use a little more Perl > expertise on your team. I wrote some brief thoughts on your Perl > code on my blog at > http://perlhacks.com/2013/02/texinfo-5-0-in-perl/. > > So I'm offering my assistance to improve the quality (and, perhaps, > the performance) of the Perl used in your project. Does anyone think > that might be useful?
Of course. I am the main (or sole) culprit, so I'll try to answer part of your concerns. The code is nicely partitioned into modules. And many of the modules are really classes. But some of the modules (the ones in the init directory) have no package statement. So the are more like Perl4-style libraries than what we’d recognise as modules. In fact, in the init directory the files are not meant to be packages/modules. The idea is to consider those files as 'customization'. They are in a package, though, you can see in texi2any.pl, l. 358 that they are loaded in the Texinfo::Config namespace. This is indeed not standard, but the idea is to try to let people writting init files just write some perl code without needing to know what packages are. Many (in fact it might be all) of the subroutines in the modules have prototypes. In many cases, that doesn’t do any harm (although Perl prototypes don’t really address the issues that most people assume they address). But many of these subroutines are called as methods. And prototypes have no effect on method calls at all. Even though they are called as method, in general in the code, I still use prototypes to provide for checking in case people do not call the subroutines as methods, but as subroutines, with the first argument being a parser or converter objet. It is occasionnaly done like that in the code too, though not often. I also think that it serves as 'documentation' of which argument is optional. There is rather more use of package variables (instead of lexical variables) than I’d be comfortable using in my code. Agreed, though all the package variables have been set up as package variables because they are indeed used in other modules. Do you see an alternative? I can see no use of CPAN modules. Perhaps there are no CPAN modules that help with this code. But I’d find that surprising in a project of this size. We use Locale::Messages, Unicode::EastAsianWidth, Text::Unidecode For these, internal versions are included, and are installed and used as part of Texinfo (not disturbing the Perl installation at all). It is possible to pass flags to configure to avoid using the internal versions. We wanted to avoid using non core perl modules as much as possible, but if relevant, feel free to point code from CPAN that could be used. For Texinfo/Convert/Paragraph.pm, Texinfo/Convert/Line.pm and Texinfo/Convert/UnFilled.pm (especially for Texinfo/Convert/Paragraph.pm, in fact), that are not really Texinfo spcific, I tried to find modules that do some filling, but none were suitable. Also for the tree handling I looked a bit at existing modules but I wanted to have a kind of double tree both with 'args' and 'contents'. For the required perl version, the idea was to require the earlier version that is compatible with the code at the file level. So, except for the code that uses Encode and requires 5.007_003, the requirement are of prior perl versions. This does not imply that we mistake this 5.6-era code for state of the art Perl... The requirement for texinfo as a whole is obviously 5.007_003 with Encode. -- Pat
