On Sun, Jul 07, 2019 at 08:24:27PM -0400, Kurt Mosiejczuk wrote:
> > > While here, tell the user what the valid choices are.
> 
> > Maybe we can be more informative right off the bat?  I'm not sure I like
> > my wording any better than yours, so maybe a combination, or maybe
> > someone has a better idea.
> 
> > Index: infrastructure/bin/portgen
> > ===================================================================
> > RCS file: /cvs/ports/infrastructure/bin/portgen,v
> > retrieving revision 1.1
> > diff -u -p -r1.1 portgen
> > --- infrastructure/bin/portgen      18 Jan 2016 19:01:02 -0000      1.1
> > +++ infrastructure/bin/portgen      8 Jul 2019 00:05:15 -0000
> > @@ -35,7 +35,8 @@ use OpenBSD::PortGen::Port::Ruby;
> 
> >  my ( $type, $module ) = @ARGV;
> 
> > -die "usage: portgen type module\n" unless $type and $module;
> > +my $usage = "usage: portgen p5|py|ruby module\n";
> > +die $usage unless $type and $module;
> 
> >  my $o;
> >  if ( $type eq 'p5' ) {
> > @@ -45,7 +46,7 @@ if ( $type eq 'p5' ) {
> >  } elsif ( $type eq 'ruby' ) {
> >     $o = OpenBSD::PortGen::Port::Ruby->new();
> >  } else {
> > -   die "unknown module type\n";
> > +   die $usage;
> >  }
> 
> >  my @new_ports = $o->port($module);
> 
> I like yours better. I was thinking a bit too local to the error I was
> looking to improve.
> 
> OK kmos@

I've also got this option, which means you only have to define the
mapping of "type" to "package" when adding new types and not have to add
the `use` and the if statement, instead deciding to use a dispatcher.

It also adjusts the error message to be more like yours, including the
known types on a separate line.  I'm not quite sure what I prefer
though.


Index: infrastructure/bin/portgen
===================================================================
RCS file: /cvs/ports/infrastructure/bin/portgen,v
retrieving revision 1.1
diff -u -p -r1.1 portgen
--- infrastructure/bin/portgen  18 Jan 2016 19:01:02 -0000      1.1
+++ infrastructure/bin/portgen  8 Jul 2019 01:19:40 -0000
@@ -29,24 +29,24 @@ BEGIN {
 
 use lib ( "$portdir/infrastructure/lib", "$FindBin::Bin/../lib" );
 
-use OpenBSD::PortGen::Port::CPAN;
-use OpenBSD::PortGen::Port::PyPI;
-use OpenBSD::PortGen::Port::Ruby;
+my %types = (
+    p5   => 'OpenBSD::PortGen::Port::CPAN',
+    py   => 'OpenBSD::PortGen::Port::PyPI',
+    ruby => 'OpenBSD::PortGen::Port::Ruby',
+);
+my $known = join ', ', sort keys %types;
 
 my ( $type, $module ) = @ARGV;
 
-die "usage: portgen type module\n" unless $type and $module;
+die "usage: portgen type module\nknown types: $known\n"
+    unless $type and $module and $types{$type};
 
-my $o;
-if ( $type eq 'p5' ) {
-       $o = OpenBSD::PortGen::Port::CPAN->new();
-} elsif ( $type eq 'py' ) {
-       $o = OpenBSD::PortGen::Port::PyPI->new();
-} elsif ( $type eq 'ruby' ) {
-       $o = OpenBSD::PortGen::Port::Ruby->new();
-} else {
-       die "unknown module type\n";
+{
+    local $@;
+    eval "require $types{$type}";
+    die if $@;
 }
 
+my $o = $types{$type}->new;
 my @new_ports = $o->port($module);
 say for @new_ports;

Reply via email to