Re: PATCH: adding auto-detection for file types

I have a couple of comments below...

On Sun, Nov 30, 2008 at 7:02 PM, Mark Stosberg <[email protected]> wrote:

>
> HTML::Template currently supports 4 formats for the input file, with three
> different APIs
> to specify it:
>
>  First:
>   new( filename   => $f, ... );
>   new( scalarref  => $f, ... );
>   new( arrayref   => $f, ... );
>   new( filehandle => $f, ... );
>
>  Second:
>   new_file($f, ... );
>   new_scalar_ref($f, ... );
>   new_array_ref($f, ... );
>   new_filehandle($f, ... );
>
>  Third:
>   new(type => 'filename'  , source => $f, ...);
>   new(type => 'scalarref' , source => $f, ...);
>   new(type => 'arrayref'  , source => $f, ...);
>   new(type => 'filehandle', source => $f, ...);
>
> ###
>
> I find these more verbose and confusing than it needs to be. Beecause the
> second option provides alternate spellings for 3 out of 4 options, it's
> hard to
> ever get the spelling right without consulting the docs.
>

First, I'd like to see alternatives for the unfortunately misspelled :-)
methods:

new_filename()
new_scalarref()
new_arrayref()

Since those are practically one-liners anyway, a few additions ought not be
a big deal.



>
> I would like to contribute a patch that cleans all this up to this:
>
>  new(source => $filehandle);
>  new(source => $filename);
>  new(source => $scalarref);
>  new(source => $arrayref);
>
> Since we can detect the difference between a string, a scalarref, an
> arrayref
> and a filehandle, we don't need to user to repeat this information for us.
> It
> can "just work". The auto-detection logic would come into play if "source"
> is
> specified, but 'type' is not.
>
> HTML::FillInForm had a similar interface and is a precendent for this kind
> of API overhaul.
> You can see their old API:
>
>
> http://search.cpan.org/~tjmather/HTML-FillInForm-2.00/lib/HTML/FillInForm.pm#Old_syntax<http://search.cpan.org/%7Etjmather/HTML-FillInForm-2.00/lib/HTML/FillInForm.pm#Old_syntax>
>
> And the new API:
>
>
> http://search.cpan.org/~tjmather/HTML-FillInForm-2.00/lib/HTML/FillInForm.pm#SYNOPSIS<http://search.cpan.org/%7Etjmather/HTML-FillInForm-2.00/lib/HTML/FillInForm.pm#SYNOPSIS>
>
> The attach patch includes working code and tests which head in this
> direction,
> but it is not complete. (The current patch auto-detects the first arg,
> instead of
> the value for 'source').
>
> The final patch would update the docs to highlight the simpler new syntax
> and
> de-emphasize the other interfaces (The same approach used by
> HTML::FillInForm).
> This would simplify things for new users, while users migrating from older
> versions would still find the reference docs for the older APIs.
>
> Before proceeding further, I wanted to get some feedback about heading in
> this
> direction.
>

I think this is an okay idea.  But I'm leery of the technique of shoehorning
alternative syntax over existing syntax the way your patch seems to imply.

Instead, I would look at this spot in the code:

  # handle the "type", "source" parameter format (does anyone use it?)
  if (exists($options->{type})) {
    exists($options->{source}) or croak("HTML::Template->new() called
with 'type' parameter set, but no 'source'!");
    ($options->{type} eq 'filename' or $options->{type} eq 'scalarref' or
     $options->{type} eq 'arrayref' or $options->{type} eq 'filehandle') or
       croak("HTML::Template->new() : type parameter must be set to
'filename', 'arrayref', 'scalarref' or 'filehandle'!");

    $options->{$options->{type}} = $options->{source};
    delete $options->{type};
    delete $options->{source};
  }

It looks like this is the only place that needs to change.

Finally, at least one other package, HTML::Template::Compiled, may
also benefit from these API changes.

Regards,

Brad

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/

_______________________________________________
Html-template-users mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/html-template-users