nms-cgi-devel — Developers discussion for NMS CGI programs

Re: [Nms-cgi-devel] patch for nms FormMail

[Craig S. <ca...@ta...>]

(i'm not subscribed to the nms-cgi-devel list.  please cc me on any
replies)

On Thu, Feb 21, 2002 at 04:06:20AM -0800, Joseph Ryan wrote:
> > the attached patch:
> >
> > 1. reads in a list of allowed recipient domains from
> >    /etc/formmail.recipients
> 
> A good idea, but Im not sure if implementing it is a good idea since it
> changes behaivor from the original version

why not?  the script already has an @referers array...my patch just adds
extra elements to the array by reading a text file.  this means you can
let customer service staff add new referers without requiring them to
have any perl programming skills.


> > 4. uses Email::Valid to verify that addresses are valid
> 
> Can't use CPAN modules (see the NMS faq for reasons why) :(

i couldn't find anything in the faq about this.

BTW, my patch for MX & NS record checks use the Net::DNS module from
CPAN.  so does the patch attached to this message.

this new patch modifies the referer checking routine so that the
hostname portion of the HTTP_REFERER variable is checked against
a regexp made up from a list of IP addresses.  if they match then the
referer is accepted.  

e.g. "@ref_ip = qw(203.10.72. 202.137.);" results in a regexp string
$ref_ip which looks like: "^(203\.10\.72\.|202\.137\.)"

as with most of my changes to the NMS FormMail script, this is very
useful on a web server with hundreds of IP based virtual hosts...saves
having to update the script for each new virtual host added.


> Our own email verification is pretty good anyways

fair enough.  i'm not sure if Email::Valid is needed...i had it in (my
severely hacked version of) the old MW FormMail script but the NMS
version already has email verification stuff.

> > 5. use CGI::Carp to log each usage of the script, both successful and
> >    failed.
> 
> We rolled our version of CGI::Carp fatalsToBrowser, so this shouldnt be that
> hard to implement.  However, it does change behaivor from the original...

i like to print debugging messages to the apache error.log, and the
warn() function from CGI::Carp is perfect for that.  if there's another
way to achieve that which fits in with better with NMS FormMail then i'd
be happy to change it to suit.


> > 7. adds Sender:, Reply-To:, and Errors-To: headers to the mail so that
> >    any bounces have a chance of actually being seen by the sender rather
> >    than getting lost in the webserver's unread mailbox.
> >
> 
> I think our version already has this.

it wasn't in the version i downloaded last week.


craig

-- 
craig sanders <ca...@ta...>

Fabricati Diem, PVNC.
 -- motto of the Ankh-Morpork City Watch

Re: [Nms-cgi-devel] Another Script Worthy of our attention

[iain t. <ic...@eh...>]

* Jonathan Stowe (gel...@ge...) [24 Feb 2002 01:04]:
> This one is worrying :

> http://www.worldwidemart.com/scripts/cgi-bin/download.cgi?s=ccver&c=txt&f=cc_ver.pl

I'd recommend you mostly just lift code from Business::CreditCard (on
CPAN).

It's what I did - I sent a version to Dave before this sourceforge thing
got going but it must've disappeared into the aether. Ah well.


cheers,
-- 
iain.                                          <http://eh.org/~koschei/>

RE: [Nms-cgi-devel] Regarding the Search.pl recursion "bug"

[Wizard <wi...@ne...>]

> You can't beat the big guys building an
> equivalent product for the same price; you have to build a MUCH better
> product.

Caveat lector: unless you're Microsoft or Harley-Davidson.
Grant M.

Re: [Nms-cgi-devel] [ nms-cgi-Bugs-521847 ] Test Ignore

[Jonathan S. <gel...@ge...>]

On Sat, 23 Feb 2002 no...@so... wrote:

> Bugs item #521847, was opened at 2002-02-23 10:50
> You can respond by visiting:

There you go  :)


/J\
-- 
Jonathan Stowe                      |
<http://www.gellyfish.com>          |      This space for rent
                                    |

RE: [Nms-cgi-devel] Regarding the Search.pl recursion "bug"

[Wizard <wi...@ne...>]

> Heres the deal:  Search.pl got modified so that it wasn't recursive
> (currently, you have to "make" it become recursive by specifiying
filetypes
> such as "*.html, */*.html, */*/*.html, etc".  The reasoning at the time
was
> to emulate Matt's code.  Personally, I think that it should be made
> recursive (at least guarded it with an $emulate_matts_code).  What is the
> general consensus here?  I'd like to know before I work on "fixing" this
> bug.

NOTE: These comments are about the nms project on the whole, and not just
Search.pl, although this email is what prompts these comments. I apologize
for the length.

I think I have a problem with the whole idea of duplicating code that we
know is marginal, when it really isn't required to perform a plug-in
replacement. I am of the belief that nms should be doing it better, and only
needs to be able to utilize the same resources that Matt's scripts do.

I feel that nms only needs to be able to take over where Matt's scripts left
off, and I don't believe that nms needs to limit itself to only what Matt's
scripts do, nor to try and re-implement marginal functionality when it can
improve upon it.

If you plan to sell non-admins on replacing something that works, it must
be:
    o Simple to install
        * install scripts?
        * configuration scripts?
    o Offer a reason to install
        * Demonstrations of security improvements (if they haven't been
            burned before, chances are that they'll ignore the security
            issue).
        * In-depth explanations of what is wrong with Matt's scripts
        * Perhaps added features such as optional template processing,
            creation and conversion.
    o Offer a long-term benefit over a static script
        * Create applications that can be potentially migrated to an
             enterprise database and shared over multiple installations
             (such as an ISP).
        * Create an API that allows plug-in feature enhancement, such as
             recursive searches, indexing, and pre- and post-manipulation
             of results.

I'll concede that this is a far greater goal than the project had planned,
but if nms expects to catch on, it has to be VISIBLY better than Matt's
scripts, and not just intrinsically better. nms can write a whole website of
scripts that are functionally equivalent to Matt's scripts, but I can almost
guarantee that Matt's will still beat nms hands-down for downloads for the
simple reason of name recognition. You can't beat the big guys building an
equivalent product for the same price; you have to build a MUCH better
product.
my two cents,
Grant M.

[Nms-cgi-devel] Regarding the Search.pl recursion "bug"

[Joseph R. <rya...@os...>]

Heres the deal:  Search.pl got modified so that it wasn't recursive =
(currently, you have to "make" it become recursive by specifiying =
filetypes such as "*.html, */*.html, */*/*.html, etc".  The reasoning at =
the time was to emulate Matt's code.  Personally, I think that it should =
be made recursive (at least guarded it with an $emulate_matts_code).  =
What is the general consensus here?  I'd like to know before I work on =
"fixing" this bug.

Re: [Nms-cgi-devel] Another Script Worthy of our attention

[Joseph R. <rya...@os...>]

Ironically enough, Dave Paris is the author of many of the big Encryption
modules on CPAN (3DES, Blowfish, etc).  Kinda odd that he is connected with
MWS...

----- Original Message -----
From: "Jonathan Stowe" <gel...@ge...>
To: "NMS Developers List" <nms...@li...>
Sent: Saturday, February 23, 2002 5:44 AM
Subject: [Nms-cgi-devel] Another Script Worthy of our attention


> This one is worrying :
>
>
>
http://www.worldwidemart.com/scripts/cgi-bin/download.cgi?s=ccver&c=txt&f=cc
_ver.pl
>
>
> /J\
> --
> Jonathan Stowe                      |
> <http://www.gellyfish.com>          |      This space for rent
>                                     |
>
>
> _______________________________________________
> Nms-cgi-devel mailing list
> Nms...@li...
> https://lists.sourceforge.net/lists/listinfo/nms-cgi-devel
>

[Nms-cgi-devel] [ nms-cgi-Bugs-521847 ] Test Ignore

[<no...@so...>]

Bugs item #521847, was opened at 2002-02-23 10:50
You can respond by visiting: 
http://sourceforge.net/tracker/?func=detail&atid=425769&aid=521847&group_id=39625

Category: None
Group: None
Status: Open
Resolution: None
Priority: 5
Submitted By: Jonathan Stowe (gellyfish)
Assigned to: Nobody/Anonymous (nobody)
Summary: Test Ignore

Initial Comment:
This is a test please ignore - I will delete it when I
have found out what I want :)

----------------------------------------------------------------------

You can respond by visiting: 
http://sourceforge.net/tracker/?func=detail&atid=425769&aid=521847&group_id=39625

Re: [Nms-cgi-devel] Another Script Worthy of our attention

[Jonathan S. <gel...@ge...>]

On 23 Feb 2002, Mike Jarvis wrote:

> On Sat, 2002-02-23 at 08:44, Jonathan Stowe wrote:
> > This one is worrying :
> >
>
> I was planing on taking a look at this one, but if somebody else gets to
> it first, I won't be crushed.  It's amazing how little free time you
> sometimes have, even when you've got CFT.
>

Nah, go for it - I have even created the new module in the cvs for you ;-}

/J\
-- 
Jonathan Stowe                      |
<http://www.gellyfish.com>          |      This space for rent
                                    |

Re: [Nms-cgi-devel] search/nms

[Jonathan S. <gel...@ge...>]

On Sat, 23 Feb 2002, Dave Cross wrote:

> On Fri, Feb 22, 2002 at 12:59:13PM -0500, Alex BATKO (ab...@mi...) wrote:
>
> > 04) On January 27th, I submitted a patch to add a missing semicolon (whose
> >     absence was yielding a compilation error).  How can someone update the
> >     CVS tree with a program they haven't even compiled to ensure it's
> >     syntactically correct ?
>
> I don't know. I hope the person responsible is suitably chastised. With the
> amount of publicity that nms is getting (and I have more things planned)
> we just can't afford the scripts to be broken.
>

Well it was me that made that particular commit and yes it was a stupid
case of making a small change in a hurry which I could have avoided.
However in general I would prefer to reserve the right to intentionally
check in code that I *know* is broken - after all the stuff in the CVS
is by its very nature in development and there may be very good reasons
for committing stuff that is not complete.  There are projects with a way
higher profile than NMS (Parrot an Mono spring to mind) where there is a
fairly good chance that at any given checkout you are going to get
something that doesn't work or even compile at all, this is the nature of
a development project; people who don't realize what they are getting out
of the CVS probably shouldn't be checking it out in the first place.

I would suggest that we probably should be keeping the released files up
to date and making it clearer that the stuff in CVS is really only
suitable for developers, rather than getting upset about something that is
bound to happen in a project of this nature.

/J\
-- 
Jonathan Stowe                      |
<http://www.gellyfish.com>          |      This space for rent
                                    |

Re: [Nms-cgi-devel] search/nms

[Dave C. <da...@da...>]

On Fri, Feb 22, 2002 at 12:59:13PM -0500, Alex BATKO (ab...@mi...) wrote:
> 
> This is a general statement of concern about the state of nms project, and
> more specifically about search/* (because that's all i've looked at in any
> detail).
> 
> I've been following this list since the end of January and I only started
> to look at the source code yesterday.  I would like to present the
> following comments as I feel they might be useful.
 
Alex,

Thanks for your comments. I really welcome this kind of constructive
criticism as it keeps us focussed :)
 
> 01) The tarballs on the main nms webpage http://nms-cgi.sourceforge.net/
>     are very old, and the revision dates are not specified.

My fault. The versions are now up to date as of lunchtime today. I know that
I need to be a little more proactive when it comes to releasing versions.
If you put something good in CVS and it needs to be released, please feel
free to prod me.

As for the timestamps, can everyone take a look at 

<http://nms-cgi.sourceforge.net/nms-cgi/inex.shtml>

The only reason I haven't replaced index.html is because I was getting some
very bad response times when I was testing it earlier.

> 02) The search.pl program doesn't work.  I downloaded search from MSA and
>     it worked without ANY modifications.  I can't say this for the nms
>     version - infact i'm still trying to get it to work.


I _think_ this is fixed in the latest version, but I can't be sure. Which is
bad.

Some one suggested that we could put demos of the scripts on the web page,
that would certainly mean that they get tested. I'll look at doing this next
week.

> 03) The program is poorly designed.  Just to give a simple example of poor 
>     design, i'll cite one major problem:  In search.pl, one of the first
>     functional things that happens is a call to the subroutine
>     start_of_html.  That subroutine pumps out the header, and begins the
>     html to the point where the search results are ready to be displayed.
>     When 'die' is called, a message is sent to the subroutine
>     fatalsToBrowser, which begins by reprinting the header manually (not
>     using 'print header'), and it prints out a new html page.  So the
>     result is a web page within a half started webpage.  Nasty !!!

Yep. That's bad and needs to be fixed.

> 04) On January 27th, I submitted a patch to add a missing semicolon (whose
>     absence was yielding a compilation error).  How can someone update the
>     CVS tree with a program they haven't even compiled to ensure it's
>     syntactically correct ?

I don't know. I hope the person responsible is suitably chastised. With the
amount of publicity that nms is getting (and I have more things planned)
we just can't afford the scripts to be broken.

> 05) Poor programming technique:  multiple return points in one subroutine.
>     A subroutine should be written in such a way that there is only one
>     return statement (and it is the last statement of the subroutine).
>     In this respect the do_search subroutine is a really great example
>     of this poor technique.

Like others who have responded, I don't see this as a problem. 

IMO, redesigning subroutines to ensure that they only have one exit point
often makes them harder to understand.

> 06) Poor commenting:  When the code is publicly available (and one of your
>     intentions is for people to actually LOOK at your code), it should be
>     well commented.  Each subroutine should have a description of its
>     purpose, what the arguments are, and what the return value is.
> 
>     Major chunks of code within a subroutine should also have a comment
>     outlining what will be done (unless it is extremely obvious).
> 
>     The idea is that one shouldn't have to run through a given program in
>     his/her head, just to understand what is the purpose of a particular
>     chunk of code (let alone what is the purpose of a particular subroutine).

Well, a good piece of code shouldn't need comments in order to explain what
it's doing - but given that these scripts are aimed at beginners I take your 
point. We do need better comments, but getting the code working right is
more important so I'm not too disappointed if the developers have been
concentrating on that instead of writing comments.

> 07) No set/accepted coding style (rules), leads to INCONSISTENT code.
> 
>     This topic often brings forth "holy wars", but it must be surfaced at
>     one point or another (when multiple authors are involved).  nms has no
>     such guidelines.  If you're running linux, and you have the kernel
>     source installed, take a look at /usr/src/Documentation/CodingStyle
>     for an example of what i'm talking about.
> 
>     In the nms code, some people organize code by 3 spaces, some by 4, some
>     by tabs, etc.  The suggested spacing for Perl is 4 spaces, and in all
>     public code NEVER USE TABS (because different programs interpret TABS
>     differently).
> 
>     Other inconsistencies are things like
> 
>         if($variable) {
>         if( $variable ) {
>         if ( $variable ) {

Again, I can't really get excited about that. People _do_ use different
coding styles and that's fine by me. If someone started to dictate to me
what code style I sohlud use then I'd stop writing code for them :)

> 08) search/README doesn't respect the 80 character ISO/ANSI standard for
>     line widths.  So lines wrap around (that's very UGLY).

Yep. That need to be fixed. And I'd prefer 72 columns.

> 09) search/README's format is ugly, and it's badly written.  I couldn't
>     understand what the heck $baseurl was.  Laugh if you want, but the
>     point is that the default value in search.pl isn't much of a url,
>     so this lead to my confusion.  I had to look at MSA search/README
>     where it's explained properly (with an example).

Given that it was written in a huge hurry, I'm not too bothered. Would be 
nice if it was cleaned up tho.

> 10) search/README's NOTICE is totally outdated.  The COPYRIGHT refers
>     to 'Countdown Version 1.00'.  The URL says that the nms website
>     http://nms-cgi.sourceforge.net/ has 'the most up to date version
>     of this script', when in fact it's nearly a month old.

The first problem is a typo. Yes it needs to be fixed.

The second problem is the same for all of the scripts. We most recent 
officially released versions _will_ be in the nms website. If versions
aren't being released quickly enough, then that's another problem which
we've already mentioned above.

> 11) Noone has taken the time to respond to the project bug reports
>     http://sourceforge.net/tracker/?group_id=39625&atid=425769
>     By respond, i not only mean look into the problems, but also to
>     reply to the people who have taken the time to make a bug submission.
>     (the one exception that i thought was notable was that the most recent
>     version of search.pl addresses one of the bugs, but noone closed the
>     bug tricket).

This is true. I didn't even realise that people were using the trackers until
it was pointed out to me a few days ago. gellyfish is now acting as the
administrator for the trackers so this shouldn't be a problem in the future
(but please let us know if it is!)

> 12) search.html refers to /css/nms.css.  That's CRAZY !!!  The first
>     error I ran into is "File Not Found":  "The requested URL
>     /css/nms.css was not found on this server".  The project has been
>     around for more then a month, and noone has tested this ????

Well no-one has tested it on a server that doesn't have a stylesheet 
installed perhaps.

This has been discussed here recently and we're working on fixes.

> I hope that we can have a genuine discussion about these issues,
> and come up with a methodical game plan for search/*, and the other
> scripts.  It is not wise to start patching minature problems, when
> there is a major underlying flaw (See 03, above).

I hope that we _do_ have discussion on these points. Much of what you've
mentioned _is_ very important. No sure that I totally agree with you about
which are the most important tho' :)

> I can write a coding standard proposal, and we can all talk about whether
> it's acceptable and complete.

I, for one, am not in favour of a coding standards document any longer than:

1/ Programs must produce no errors under "use strict" and "-Tw".
2/ Variables and subroutines must have meaningful names.

But I'm quite prepared to be outvoted on this :)

> I hope that this motivates discussion and improvements within nms.

<aol />

Dave...

-- 

  Drugs are just bad m'kay

Re: [Nms-cgi-devel] Another Script Worthy of our attention

[Mike J. <my...@er...>]

On Sat, 2002-02-23 at 08:44, Jonathan Stowe wrote:
> This one is worrying :
> 
> 
> http://www.worldwidemart.com/scripts/cgi-bin/download.cgi?s=ccver&c=txt&f=cc_ver.pl

I was planing on taking a look at this one, but if somebody else gets to
it first, I won't be crushed.  It's amazing how little free time you
sometimes have, even when you've got CFT. 


-- 
mike
"Tact is just not saying true stuff" - Cordelia on BtVS

[Nms-cgi-devel] Another Script Worthy of our attention

[Jonathan S. <gel...@ge...>]

This one is worrying :


http://www.worldwidemart.com/scripts/cgi-bin/download.cgi?s=ccver&c=txt&f=cc_ver.pl


/J\
-- 
Jonathan Stowe                      |
<http://www.gellyfish.com>          |      This space for rent
                                    |

Re: [Nms-cgi-devel] search/nms

[Jonathan S. <gel...@ge...>]

On Fri, 22 Feb 2002, Alex BATKO wrote:
>
> 11) Noone has taken the time to respond to the project bug reports
>     http://sourceforge.net/tracker/?group_id=39625&atid=425769
>     By respond, i not only mean look into the problems, but also to
>     reply to the people who have taken the time to make a bug submission.
>     (the one exception that i thought was notable was that the most recent
>     version of search.pl addresses one of the bugs, but noone closed the
>     bug tricket).
>

Firstly any NMS developer can access the various trackers in Sourceforge
and I would exhort everyone to look at this stuff once in a while when
they have time,  I have just altered the tracker configuration so that it
will mail this list when new tracker items are added so hopefully we will
be able to make a timely response.

Secondly I have updated the outstanding items and I think there are only
three remaining open items and only one of these might actually need close
attention - if someone want's to look at :


https://sourceforge.net/tracker/index.php?func=detail&aid=514487&group_id=39625&atid=425769

I have assigned this to Joe but if someone else has more time available
then feel free to take it up and I will assign it to them instead :)

Finally, and this is a more general point, it has to be borne in mind that
this is an Open Source volunteer project: no-one is compelled to be doing
this.  Most of the NMS developers have to do real work to earn a living
and I'm sure a number of us are also involved in other Open Source
projects: in an ideal world sure we would like to have everything working
like clockwork but this just isn't going to happen in a project like this.

/J\
-- 
Jonathan Stowe                      |
<http://www.gellyfish.com>          |      This space for rent
                                    |

Re: [Nms-cgi-devel] search/nms

[Joseph R. <rya...@os...>]

----- Original Message -----
From: "Alex BATKO" <ab...@mi...>
To: <nms...@li...>
Sent: Friday, February 22, 2002 9:59 AM
Subject: [Nms-cgi-devel] search/nms


>
> This is a general statement of concern about the state of nms project, and
> more specifically about search/* (because that's all i've looked at in any
> detail).
>
> I've been following this list since the end of January and I only started
> to look at the source code yesterday.  I would like to present the
> following comments as I feel they might be useful.

> 02) The search.pl program doesn't work.  I downloaded search from MSA and
>     it worked without ANY modifications.  I can't say this for the nms
>     version - infact i'm still trying to get it to work.
>

Well, it works different than what we would like.  I personally think the
search itself should be recursive, which is a huge step up from Matt's
design.  I think the only reason why Matt uses the "*/.html, */*/.html"
method is that he didn't know how to make it recursive.  Perhaps we should
remove this behavior entirely and make the search recursive...
>
> 03) The program is poorly designed.  Just to give a simple example of poor
>     design, i'll cite one major problem:  In search.pl, one of the first
>     functional things that happens is a call to the subroutine
>     start_of_html.  That subroutine pumps out the header, and begins the
>     html to the point where the search results are ready to be displayed.
>     When 'die' is called, a message is sent to the subroutine
>     fatalsToBrowser, which begins by reprinting the header manually (not
>     using 'print header'), and it prints out a new html page.  So the
>     result is a web page within a half started webpage.  Nasty !!!
>

Good point, I don't think anyone thought about that.  Should be a simple
fix...

>
> 05) Poor programming technique:  multiple return points in one subroutine.
>     A subroutine should be written in such a way that there is only one
>     return statement (and it is the last statement of the subroutine).
>     In this respect the do_search subroutine is a really great example
>     of this poor technique.
>

You must not understand how File::Find works.  File::Find's find function is
recursive, and if it returns undef (which is what a return in void context
does), it is equivalant to saying "Move on to the next file".  It would be
VERY painful to code without using this technique.  At any rate, I hardly
think that multiple returns are a bad construct - Subroutines should be
constructed so that they flow a logical way - and if the damn thing should
stop at some point, it should stop, and not dance around so that there is
only 1 return statement (Not to mention that CPU time is saved by not
running unnecessary code).

>
> 06) Poor commenting:  When the code is publicly available (and one of your
>     intentions is for people to actually LOOK at your code), it should be
>     well commented.  Each subroutine should have a description of its
>     purpose, what the arguments are, and what the return value is.
>
>     Major chunks of code within a subroutine should also have a comment
>     outlining what will be done (unless it is extremely obvious).
>
>     The idea is that one shouldn't have to run through a given program in
>     his/her head, just to understand what is the purpose of a particular
>     chunk of code (let alone what is the purpose of a particular
subroutine).
>

True, commenting is something we do need to work on.  However, since the nms
scripts are still under development, this isn't the most urgent thing that
needs to be done.  Perhaps someone with some spare time could do some
commenting?

>
> 07) No set/accepted coding style (rules), leads to INCONSISTENT code.
>
>     This topic often brings forth "holy wars", but it must be surfaced at
>     one point or another (when multiple authors are involved).  nms has no
>     such guidelines.  If you're running linux, and you have the kernel
>     source installed, take a look at /usr/src/Documentation/CodingStyle
>     for an example of what i'm talking about.
>
>     In the nms code, some people organize code by 3 spaces, some by 4,
some
>     by tabs, etc.  The suggested spacing for Perl is 4 spaces, and in all
>     public code NEVER USE TABS (because different programs interpret TABS
>     differently).
>

This has come up before; Although I think it is ridiculous to expect people
to write code the same way, the least that could be done is run code under
tidy to ensure consistant formatting.

>     Other inconsistencies are things like
>
>         if($variable) {
>         if( $variable ) {
>         if ( $variable ) {
>

Does so minute as spacing really matter?

>
> 09) search/README's format is ugly, and it's badly written.  I couldn't
>     understand what the heck $baseurl was.  Laugh if you want, but the
>     point is that the default value in search.pl isn't much of a url,
>     so this lead to my confusion.  I had to look at MSA search/README
>     where it's explained properly (with an example).
>

I agree, the readmes do need work, but they were written rather hastily.  At
first, we used Matt Wright's readmes - until Dave started recieving legal
threats from him.  So, we had to remove the MW readmes and replace them with
our own.

>
>
> 11) Noone has taken the time to respond to the project bug reports
>     http://sourceforge.net/tracker/?group_id=39625&atid=425769
>     By respond, i not only mean look into the problems, but also to
>     reply to the people who have taken the time to make a bug submission.
>     (the one exception that i thought was notable was that the most recent
>     version of search.pl addresses one of the bugs, but noone closed the
>     bug tricket).
>

If it is any consolation, the bugs have been fixed within hours of the bug
reports... :(

>
> 12) search.html refers to /css/nms.css.  That's CRAZY !!!  The first
>     error I ran into is "File Not Found":  "The requested URL
>     /css/nms.css was not found on this server".  The project has been
>     around for more then a month, and noone has tested this ????
>

If you have been following the rest of the mailing list, there already is
discussion about stylesheets.

>
> I hope that we can have a genuine discussion about these issues,
> and come up with a methodical game plan for search/*, and the other
> scripts.  It is not wise to start patching minature problems, when
> there is a major underlying flaw (See 03, above).
>
> I can write a coding standard proposal, and we can all talk about whether
> it's acceptable and complete.
>
> I hope that this motivates discussion and improvements within nms.
>
> _______________________________________________
> Nms-cgi-devel mailing list
> Nms...@li...
> https://lists.sourceforge.net/lists/listinfo/nms-cgi-devel
>

Re: [Nms-cgi-devel] search/nms

[Jonathan S. <gel...@ge...>]

On Fri, 22 Feb 2002, Chris Devers wrote:
> On Fri, 22 Feb 2002, Alex BATKO wrote:
> > 05) Poor programming technique:  multiple return points in one subroutine.
> >     A subroutine should be written in such a way that there is only one
> >     return statement (and it is the last statement of the subroutine).
> >     In this respect the do_search subroutine is a really great example
> >     of this poor technique.
>
> Ok this I totally reject. I don't see the benefit of adding in all the
> necessary plumbing to force all functions to have one 'in' and one 'out'.
> If you encounter a termination condition at the top of the function, you
> shouldn't be forced to go to the bottom of that function just to escape.
> In this I place a higher precedence on "goto considered harmful" than
> "multiple exits considered harmful".
>
> Why is this considered to be such a great design criteria, anyway? When I
> was first learning to progam in C++, being forced to do this was a great
> source of bugs for me, and it didn't really make anything I wrote all that
> much clearer in the first place. No one told us that we could just escape
> out of a function, avoid all that buggy scaffolding code that's needed to
> support single-exit, *and* make execution flow IMO much clearer.
>
> Unless you can offer a pretty good justification for this, I don't see any
> reason to rewrite everything so that it adheres to this rule.
>

Yeah, if you speak up on this list with a suggestion you had better be
prepared to implement it ;-}

Earlier in my career I worked with a lot of COBOL programmers and they
would have been of a similar position to Alex - I *can* program COBOL if
pressed but I would never describe myself as a COBOL programmer :)  But
this is fine: you simply collected all the failures and have one exit
point with the appropriate exit code.

I'm not sure that the body of Perl thought will go along with this but if
you think that this is where you want to go feel free to implement it :)

/J\
-- 
Jonathan Stowe                      |
<http://www.gellyfish.com>          |      This space for rent
                                    |

RE: [Nms-cgi-devel] search/nms

[Paul R. <pa...@ro...>]

I haven't worked with the search script, so I'll comment on some of the
other stuff:

> 05) Poor programming technique:  multiple return points in one subroutine.
>     A subroutine should be written in such a way that there is only one
>     return statement (and it is the last statement of the subroutine).
>     In this respect the do_search subroutine is a really great example
>     of this poor technique.
>

What was that you said about "holy wars"?

There are those of us (OK, me) who would argue that, in many cases, this
particular rule can lead to convoluted conditional logic that might
otherwise be expressed quite simply and cleanly.

We could argue this for days, but the truth is there's really no consensus
on this one, and it would be a shame to expend too much energy batting it
back and forth.


> 07) No set/accepted coding style (rules), leads to INCONSISTENT code.
>
> I can write a coding standard proposal, and we can all talk about whether
> it's acceptable and complete.
>

Rather than that, perhaps we should adopt an existing, somewhat-widely-used
standard.  We'll never come up with one that will please everyone, so we
might as well pick one that's good enough and learn to live with it.  How
'bout the standard perlstyle
(http://www.perldoc.com/perl5.6.1/pod/perlstyle.html)?  I'll happily use
that, even though some of the details are *not* the choices I'd make at
all...

-paul

Re: [Nms-cgi-devel] search/nms

[Chris D. <cd...@ts...>]

On Fri, 22 Feb 2002, Alex BATKO wrote:

> This is a general statement of concern about the state of nms project,
> and more specifically about search/* (because that's all i've looked at
> in any detail). 

...which is not to put people on the defensive or anything... ;)
 
> 04) On January 27th, I submitted a patch to add a missing semicolon (whose
>     absence was yielding a compilation error).  How can someone update the
>     CVS tree with a program they haven't even compiled to ensure it's
>     syntactically correct ?

<aol />

I submitted a bug to the sourceforge site, regarding POSIX date formatting
in iirc the guestbook script. As it turned out after more testing, the
problem is with Mac OS X, and not NMS, so I filed another bug report with
Apple. Still though, it would have been nice to have some acknowledgement
that a bug was submitted... 
 
> 05) Poor programming technique:  multiple return points in one subroutine.
>     A subroutine should be written in such a way that there is only one
>     return statement (and it is the last statement of the subroutine).
>     In this respect the do_search subroutine is a really great example
>     of this poor technique.

Ok this I totally reject. I don't see the benefit of adding in all the
necessary plumbing to force all functions to have one 'in' and one 'out'. 
If you encounter a termination condition at the top of the function, you
shouldn't be forced to go to the bottom of that function just to escape. 
In this I place a higher precedence on "goto considered harmful" than
"multiple exits considered harmful". 

Why is this considered to be such a great design criteria, anyway? When I
was first learning to progam in C++, being forced to do this was a great
source of bugs for me, and it didn't really make anything I wrote all that
much clearer in the first place. No one told us that we could just escape
out of a function, avoid all that buggy scaffolding code that's needed to
support single-exit, *and* make execution flow IMO much clearer. 

Unless you can offer a pretty good justification for this, I don't see any
reason to rewrite everything so that it adheres to this rule. 


Not being a NMS developer, I won't respond to your other points, but a lot
of them just seem pedantic & unhelpful to me. Yes, the code could be more
consistent, and maybe someone ought to be Kwalitee Czar enforcing this
sort of rule, but a lot of these qualms don't seem terribly significant.

Ansi standard line lengths, for example? Sheesh... 



-- 
Chris Devers                           chd...@ne...
Apache / mod_perl / http://homepage.mac.com/chdevers/resume/

[Nms-cgi-devel] search/nms

[Alex B. <ab...@mi...>]

This is a general statement of concern about the state of nms project, and
more specifically about search/* (because that's all i've looked at in any
detail).

I've been following this list since the end of January and I only started
to look at the source code yesterday.  I would like to present the
following comments as I feel they might be useful.


01) The tarballs on the main nms webpage http://nms-cgi.sourceforge.net/
    are very old, and the revision dates are not specified.


02) The search.pl program doesn't work.  I downloaded search from MSA and
    it worked without ANY modifications.  I can't say this for the nms
    version - infact i'm still trying to get it to work.


03) The program is poorly designed.  Just to give a simple example of poor 
    design, i'll cite one major problem:  In search.pl, one of the first
    functional things that happens is a call to the subroutine
    start_of_html.  That subroutine pumps out the header, and begins the
    html to the point where the search results are ready to be displayed.
    When 'die' is called, a message is sent to the subroutine
    fatalsToBrowser, which begins by reprinting the header manually (not
    using 'print header'), and it prints out a new html page.  So the
    result is a web page within a half started webpage.  Nasty !!!


04) On January 27th, I submitted a patch to add a missing semicolon (whose
    absence was yielding a compilation error).  How can someone update the
    CVS tree with a program they haven't even compiled to ensure it's
    syntactically correct ?


05) Poor programming technique:  multiple return points in one subroutine.
    A subroutine should be written in such a way that there is only one
    return statement (and it is the last statement of the subroutine).
    In this respect the do_search subroutine is a really great example
    of this poor technique.


06) Poor commenting:  When the code is publicly available (and one of your
    intentions is for people to actually LOOK at your code), it should be
    well commented.  Each subroutine should have a description of its
    purpose, what the arguments are, and what the return value is.

    Major chunks of code within a subroutine should also have a comment
    outlining what will be done (unless it is extremely obvious).

    The idea is that one shouldn't have to run through a given program in
    his/her head, just to understand what is the purpose of a particular
    chunk of code (let alone what is the purpose of a particular subroutine).


07) No set/accepted coding style (rules), leads to INCONSISTENT code.

    This topic often brings forth "holy wars", but it must be surfaced at
    one point or another (when multiple authors are involved).  nms has no
    such guidelines.  If you're running linux, and you have the kernel
    source installed, take a look at /usr/src/Documentation/CodingStyle
    for an example of what i'm talking about.

    In the nms code, some people organize code by 3 spaces, some by 4, some
    by tabs, etc.  The suggested spacing for Perl is 4 spaces, and in all
    public code NEVER USE TABS (because different programs interpret TABS
    differently).

    Other inconsistencies are things like

        if($variable) {
        if( $variable ) {
        if ( $variable ) {


08) search/README doesn't respect the 80 character ISO/ANSI standard for
    line widths.  So lines wrap around (that's very UGLY).


09) search/README's format is ugly, and it's badly written.  I couldn't
    understand what the heck $baseurl was.  Laugh if you want, but the
    point is that the default value in search.pl isn't much of a url,
    so this lead to my confusion.  I had to look at MSA search/README
    where it's explained properly (with an example).


10) search/README's NOTICE is totally outdated.  The COPYRIGHT refers
    to 'Countdown Version 1.00'.  The URL says that the nms website
    http://nms-cgi.sourceforge.net/ has 'the most up to date version
    of this script', when in fact it's nearly a month old.


11) Noone has taken the time to respond to the project bug reports
    http://sourceforge.net/tracker/?group_id=39625&atid=425769
    By respond, i not only mean look into the problems, but also to
    reply to the people who have taken the time to make a bug submission.
    (the one exception that i thought was notable was that the most recent
    version of search.pl addresses one of the bugs, but noone closed the
    bug tricket).


12) search.html refers to /css/nms.css.  That's CRAZY !!!  The first
    error I ran into is "File Not Found":  "The requested URL
    /css/nms.css was not found on this server".  The project has been
    around for more then a month, and noone has tested this ????


I hope that we can have a genuine discussion about these issues,
and come up with a methodical game plan for search/*, and the other
scripts.  It is not wise to start patching minature problems, when
there is a major underlying flaw (See 03, above).

I can write a coding standard proposal, and we can all talk about whether
it's acceptable and complete.

I hope that this motivates discussion and improvements within nms.

RE: [Nms-cgi-devel] Just a suggestion...

[Paul R. <pa...@ro...>]

Agreed -- but it is a requirement of ours, and given that we're aiming at
novices, it might help if we can give them something concrete to ask their
admins about.

Might be as simple as their needing to change the shebang to
"#!/usr/bin/perl5 ..."

-paul


-----Original Message-----
From: nms...@li...
[mailto:nms...@li...]On Behalf Of Jonathan
Stowe
Sent: Thursday, February 21, 2002 12:29 PM
To: Paul Roub
Cc: NMS Developers List
Subject: RE: [Nms-cgi-devel] Just a suggestion...


On Thu, 21 Feb 2002, Paul Roub wrote:

> This brings up another point -- should we be doing a:
>
> 	require 5.004_04;
>
> in each script?  Alternatively, should we check $PERL_VERSION and die more
> gracefully?
>

If anyone is on a web server with less than that then this will be least
of their problems ;-}

/J\
> -paul
>
>
> -----Original Message-----
> From: nms...@li...
> [mailto:nms...@li...]On Behalf Of Jonathan
> Stowe
> Sent: Thursday, February 21, 2002 10:57 AM
> To: NMS Developers List
> Subject: RE: [Nms-cgi-devel] Just a suggestion...
>
>
> On Thu, 21 Feb 2002, Wizard wrote:
>
> > > Not that I would like to discourage anyone from creating a new storage
> > > mechanism that could be used on green field sites (bearing in mind
this
> > > can't use any non-Core modules so XML or DBI is out :()
> >
> > If anyone has a list of what modules were distributed with 5.004_04, it
> > would help a lot. The earliest version available on perl.com is
5.005_03.
> >
>
> http://www.cpan.org/src/5.0/maint/perl5.004_04.tar.gz
>
>
> > Additionally, some requirement of the minimum version of CGI.pm required
> to
> > run these nms scripts would probably be in order, and the scripts should
> > test for it (CGI->version()) and be tested against it. The 5.005_03
> version
> > on perl.com comes with 2.46, but I have no idea if that's what
originally
> > came with it. As a note, CGI.pm versions prior to 2.36 "suffered a year
> 2000
> > problem in the handling of cookies" and who knows what else.
>
>
> The version that came with 5.004.04 is 2.36
>
> /J\
> --
> Jonathan Stowe                      |
> <http://www.gellyfish.com>          |      This space for rent
>                                     |
>
>
> _______________________________________________
> Nms-cgi-devel mailing list
> Nms...@li...
> https://lists.sourceforge.net/lists/listinfo/nms-cgi-devel
>
>
> _______________________________________________
> Nms-cgi-devel mailing list
> Nms...@li...
> https://lists.sourceforge.net/lists/listinfo/nms-cgi-devel
>

--
Jonathan Stowe                      |
<http://www.gellyfish.com>          |      This space for rent
                                    |


_______________________________________________
Nms-cgi-devel mailing list
Nms...@li...
https://lists.sourceforge.net/lists/listinfo/nms-cgi-devel

Re: [Nms-cgi-devel] Re: [Nms-cgi-support] Re: FormMail

[Joseph R. <rya...@os...>]

----- Original Message -----
From: "Jonathan Stowe" <gel...@ge...>
To: "NMS Developers List" <nms...@li...>
Sent: Wednesday, February 20, 2002 11:59 PM
Subject: [Nms-cgi-devel] Re: [Nms-cgi-support] Re: FormMail


> On Tue, 19 Feb 2002, Dave Cross wrote:
> >
> > Note to nms developers: Should we look at a change so that setting
$style to
> > a false value, turns off all stylesheet code?
> >
>
> It shouldn't really be necessary because as you say if the stylesheet
> isn't there then the page will simply get rendered without the benefit of
> any styles.

Not if the user is using Netscape 4.x, which unfortunately still consumes 5%
of the browser market.
In that case, the user will be treated to a 404 page stating that nms.css
cannot be found.

> However if the user were to examine their error log they are
> possibly going to be alarmed by the number of 404s for the 'nms.css' or
> whatever is the default.  I think we should distribute a stylesheet with
> each individual program as a sort of gentle hint - in the fullness of time
> it might be nice to get together a gallery of CSS for use with NMS.  I'm
> hedging on the actual question because the code might become a little
> ugly - I'll make the change to FormMail.pl to see what you guys think.

I already did it to Simple Search awhile ago ;)
It was only a 1 line change... or do you mean to set $style equal to 0 if
the file could not be opened, despite what the user set?

RE: [Nms-cgi-devel] Just a suggestion...

[Jonathan S. <gel...@ge...>]

On Thu, 21 Feb 2002, Paul Roub wrote:

> This brings up another point -- should we be doing a:
>
> 	require 5.004_04;
>
> in each script?  Alternatively, should we check $PERL_VERSION and die more
> gracefully?
>

If anyone is on a web server with less than that then this will be least
of their problems ;-}

/J\
> -paul
>
>
> -----Original Message-----
> From: nms...@li...
> [mailto:nms...@li...]On Behalf Of Jonathan
> Stowe
> Sent: Thursday, February 21, 2002 10:57 AM
> To: NMS Developers List
> Subject: RE: [Nms-cgi-devel] Just a suggestion...
>
>
> On Thu, 21 Feb 2002, Wizard wrote:
>
> > > Not that I would like to discourage anyone from creating a new storage
> > > mechanism that could be used on green field sites (bearing in mind this
> > > can't use any non-Core modules so XML or DBI is out :()
> >
> > If anyone has a list of what modules were distributed with 5.004_04, it
> > would help a lot. The earliest version available on perl.com is 5.005_03.
> >
>
> http://www.cpan.org/src/5.0/maint/perl5.004_04.tar.gz
>
>
> > Additionally, some requirement of the minimum version of CGI.pm required
> to
> > run these nms scripts would probably be in order, and the scripts should
> > test for it (CGI->version()) and be tested against it. The 5.005_03
> version
> > on perl.com comes with 2.46, but I have no idea if that's what originally
> > came with it. As a note, CGI.pm versions prior to 2.36 "suffered a year
> 2000
> > problem in the handling of cookies" and who knows what else.
>
>
> The version that came with 5.004.04 is 2.36
>
> /J\
> --
> Jonathan Stowe                      |
> <http://www.gellyfish.com>          |      This space for rent
>                                     |
>
>
> _______________________________________________
> Nms-cgi-devel mailing list
> Nms...@li...
> https://lists.sourceforge.net/lists/listinfo/nms-cgi-devel
>
>
> _______________________________________________
> Nms-cgi-devel mailing list
> Nms...@li...
> https://lists.sourceforge.net/lists/listinfo/nms-cgi-devel
>

-- 
Jonathan Stowe                      |
<http://www.gellyfish.com>          |      This space for rent
                                    |

RE: [Nms-cgi-devel] Just a suggestion...

[Paul R. <pa...@ro...>]

This brings up another point -- should we be doing a:

	require 5.004_04;

in each script?  Alternatively, should we check $PERL_VERSION and die more
gracefully?

-paul


-----Original Message-----
From: nms...@li...
[mailto:nms...@li...]On Behalf Of Jonathan
Stowe
Sent: Thursday, February 21, 2002 10:57 AM
To: NMS Developers List
Subject: RE: [Nms-cgi-devel] Just a suggestion...


On Thu, 21 Feb 2002, Wizard wrote:

> > Not that I would like to discourage anyone from creating a new storage
> > mechanism that could be used on green field sites (bearing in mind this
> > can't use any non-Core modules so XML or DBI is out :()
>
> If anyone has a list of what modules were distributed with 5.004_04, it
> would help a lot. The earliest version available on perl.com is 5.005_03.
>

http://www.cpan.org/src/5.0/maint/perl5.004_04.tar.gz


> Additionally, some requirement of the minimum version of CGI.pm required
to
> run these nms scripts would probably be in order, and the scripts should
> test for it (CGI->version()) and be tested against it. The 5.005_03
version
> on perl.com comes with 2.46, but I have no idea if that's what originally
> came with it. As a note, CGI.pm versions prior to 2.36 "suffered a year
2000
> problem in the handling of cookies" and who knows what else.


The version that came with 5.004.04 is 2.36

/J\
--
Jonathan Stowe                      |
<http://www.gellyfish.com>          |      This space for rent
                                    |


_______________________________________________
Nms-cgi-devel mailing list
Nms...@li...
https://lists.sourceforge.net/lists/listinfo/nms-cgi-devel

RE: [Nms-cgi-devel] Just a suggestion...

[Jonathan S. <gel...@ge...>]

On Thu, 21 Feb 2002, Wizard wrote:

> > Not that I would like to discourage anyone from creating a new storage
> > mechanism that could be used on green field sites (bearing in mind this
> > can't use any non-Core modules so XML or DBI is out :()
>
> If anyone has a list of what modules were distributed with 5.004_04, it
> would help a lot. The earliest version available on perl.com is 5.005_03.
>

http://www.cpan.org/src/5.0/maint/perl5.004_04.tar.gz


> Additionally, some requirement of the minimum version of CGI.pm required to
> run these nms scripts would probably be in order, and the scripts should
> test for it (CGI->version()) and be tested against it. The 5.005_03 version
> on perl.com comes with 2.46, but I have no idea if that's what originally
> came with it. As a note, CGI.pm versions prior to 2.36 "suffered a year 2000
> problem in the handling of cookies" and who knows what else.


The version that came with 5.004.04 is 2.36

/J\
-- 
Jonathan Stowe                      |
<http://www.gellyfish.com>          |      This space for rent
                                    |

Re: [Nms-cgi-devel] Re: [Nms-cgi-support] Re: FormMail

[Chris D. <cd...@ts...>]

On Thu, 21 Feb 2002, Jonathan Stowe wrote:

> I think we should distribute a stylesheet with
> each individual program as a sort of gentle hint

FYI, when setting up FormMail for a friend's site last week, I grabbed
nms.css from the SourceForge site and used it more or less as-is, if 
only to use it as a placeholder for exactly this reason. Having a sample
gallery wouldn't be such a bad idea, but setting a cgi parameter that
allows the user to use arbitrary stylesheets might be even better. 


--
Chris Devers

"Okay, Gene... so, -1 x -1 should equal what?" "A South American!"    
[....] "no human can understand the Timecube" and Gene responded
 without missing a beat "Yeah.  I'm not human."