Johannes,
Given other people have answered some points, I'll answer the rest.
Johannes Schindelin wrote:
>>> -- snip --
>>> #!/usr/bin/perl
>> Add -w for warnings, also use strict;
>
> <dumb>What does "use strict;" imply?</dumb>
Three things;
1. variables must be declared with 'my', 'our' or 'use var' before they
are used, to catch typos
2. when subroutine calls are found, they are checked to exist otherwise
they throw a compile-time error
3. force all dereferences to follow real references and not allow symbol
table access (don't worry about that ;-))
>>> sub handle_text {
>> this function acts on globals; make them explicit arguments to the
>> function.
>
> Actually, it resets the global $par. Should I rather make it a class?
Well, just channelling Dijkstra really. Functions should take all their
input as formal arguments rather than globals.
ie
sub handle_text {
my $par = shift;
}
If it really should be a global, it is perhaps best declared up front
with "our" or "use vars". "use strict" will force you to do one of these.
>> also consider making this a "tabular ternary" with the actions in
>> separate functions.
>>
>> ie
>>
>> $result = ( $par =~ /^\. /s ? $conv->do_enum($par) :
>> $par =~ /^\[verse\]/ ? $conv->do_verse($par) :
>> ... )
>
> I do not like that way... is it Perl standard to code like that?
It's in Perl Best Practices, but these are always suggestions and not
hard and fast rules. It just means that you have a big table of regex
-> function that you can quickly check rather than looking at a lot of
spaced out 'elsif's
>>> s/gitlink:([^\[ ]*)\[(\d+)\]/sprintf "%s",
>>> $conv->get_link($1, $2)/ge;
>>> # handle link:
>>> s/link:([^\[ ]*)\[(.+)\]/sprintf "%s",
>>> $conv->get_link($1, $2, 'external')/ge;
>> These REs suffer from LTS (Leaning Toothpick Syndrome). Consider using
>> s{foo}{bar} and adding the 'x' modifier to space out groups.
>
> I guess you mean the forward slash. Alas, that's what I'm used to, and
> I'd rather not change it unless forced to... lest I stop understanding my
> own code!
>
> (Besides, I did not find _any_ example showing why "x" should be useful.)
Before:
s/link:([^\[ ]*)\[(.+)\]/sprintf "%s",
$conv->get_link($1, $2, 'external')/ge;
After:
s{ link: ([^\[\040]*) \[(.+)\] }
{ sprintf "%s", $conv->get_link($1, $2, 'external') }gex;
>>> if ($self->{preamble_shown} == undef) {
>>> print '.\" disable hyphenation' . "\n"
>>> . '.nh' . "\n"
>>> . '.\" disable justification (adjust text to left'
>>> . ' margin only)' . "\n"
>>> . '.ad l' . "\n";
>> Using commas rather than "." will safe you a concat when printing to
>> filehandles, but that's a very small nit to pick :)
>
> Does that also work with older perl? IIRC there was some strange problem
> with my perl when lots of code in git.git was changed to using commata.
That should go back all the way to perl 4, if not earlier. If you're
assigning to a scalar, then you need to use concat. But very minor.
>> Hmm, that regex would not match for <<foo > bar>>, if you care you'd
>> need to write something like <<((?:[^>]+|>[^>])*)>>
>
> I'd rather leave it as is -- this script is not meant to grok all kind of
> sh*t. It is meant to make translating the docs as fast and uncumbersome
> as possible. Which will involve making the documentation more consistent
> (in and of itself something I rather like).
>
> So unless there comes a compelling reason, I'd rather leave it.
Sure, KISS.
> Another thing: if I want to add some documentation, what would be the
> common way to do it? =pod...=cut?
That's right. If you're an emacs user, in cperl-mode with abbrevs on
you type '=head1' and you'll get:
=head1 NAME
scriptname
=head1 SYNOPSIS
=head1 DESCRIPTION
=cut
See perlpod(3) for more!
Sam.
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
