All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sam Vilain <sam@vilain.net>
To: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Cc: Git Mailing List <git@vger.kernel.org>
Subject: Re: WIP: asciidoc replacement
Date: Thu, 04 Oct 2007 17:13:41 +1300	[thread overview]
Message-ID: <47046875.1050605@vilain.net> (raw)
In-Reply-To: <Pine.LNX.4.64.0710030506360.28395@racer.site>

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.

  parent reply	other threads:[~2007-10-04  4:14 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-10-03  0:42 WIP: asciidoc replacement Johannes Schindelin
2007-10-03  1:56 ` Sam Vilain
2007-10-03  4:23   ` Johannes Schindelin
2007-10-03  4:51     ` Jeff King
2007-10-03 13:55     ` J. Bruce Fields
2007-10-04  4:13     ` Sam Vilain [this message]
2007-10-04 12:41       ` Johannes Schindelin
2007-10-03  6:40   ` Wincent Colaiuta
2007-10-03  4:48 ` Junio C Hamano
2007-10-03  6:34   ` Wincent Colaiuta
2007-10-03  8:12     ` David Kastrup
2007-10-03 10:05       ` Wincent Colaiuta
2007-10-03 10:25         ` David Kastrup
2007-10-03 10:52           ` Sam Ravnborg
2007-10-03 13:47           ` J. Bruce Fields
2007-10-03 14:01             ` David Kastrup
2007-10-03 10:57         ` Junio C Hamano
2007-10-03 17:46           ` Sam Ravnborg
2007-10-03 18:57             ` Johannes Schindelin
2007-10-03 19:21               ` Sam Ravnborg
2007-10-04  6:55           ` Martin Langhoff
2007-10-04 20:58             ` David Kastrup
2007-10-04 22:49               ` Martin Langhoff
2007-10-03 11:50   ` [msysGit] " Johannes Schindelin
2007-10-03 12:02     ` David Kastrup

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=47046875.1050605@vilain.net \
    --to=sam@vilain.net \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.