Better type error for portgen(1)

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

Better type error for portgen(1)

Kurt Mosiejczuk-9
Currently, if you give an unknown port type to portgen it dies with
"unknown module type\n". This doesn't reflect what the rest of the code
or the manpage uses to refer to the first argument: "type". So, correct
that, along with outputting the offending input.

While here, tell the user what the valid choices are.

--Kurt

Index: portgen
===================================================================
RCS file: /cvs/ports/infrastructure/bin/portgen,v
retrieving revision 1.1
diff -u -p -r1.1 portgen
--- portgen 18 Jan 2016 19:01:02 -0000 1.1
+++ portgen 7 Jul 2019 21:41:23 -0000
@@ -45,7 +45,7 @@ if ( $type eq 'p5' ) {
 } elsif ( $type eq 'ruby' ) {
  $o = OpenBSD::PortGen::Port::Ruby->new();
 } else {
- die "unknown module type\n";
+ die "unknown type: " . $type . "\nknown types: p5, py, ruby\n";
 }
 
 my @new_ports = $o->port($module);

Reply | Threaded
Open this post in threaded view
|

Re: Better type error for portgen(1)

Andrew Hewus Fresh
On Sun, Jul 07, 2019 at 05:44:28PM -0400, Kurt Mosiejczuk wrote:
> Currently, if you give an unknown port type to portgen it dies with
> "unknown module type\n". This doesn't reflect what the rest of the code
> or the manpage uses to refer to the first argument: "type". So, correct
> that, along with outputting the offending input.
>
> 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.


>
> --Kurt
>
> Index: portgen
> ===================================================================
> RCS file: /cvs/ports/infrastructure/bin/portgen,v
> retrieving revision 1.1
> diff -u -p -r1.1 portgen
> --- portgen 18 Jan 2016 19:01:02 -0000 1.1
> +++ portgen 7 Jul 2019 21:41:23 -0000
> @@ -45,7 +45,7 @@ if ( $type eq 'p5' ) {
>  } elsif ( $type eq 'ruby' ) {
>   $o = OpenBSD::PortGen::Port::Ruby->new();
>  } else {
> - die "unknown module type\n";
> + die "unknown type: " . $type . "\nknown types: p5, py, ruby\n";
>  }
>  
>  my @new_ports = $o->port($module);
>

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);

Reply | Threaded
Open this post in threaded view
|

Re: Better type error for portgen(1)

Kurt Mosiejczuk-9
> > 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@

--Kurt

Reply | Threaded
Open this post in threaded view
|

Re: Better type error for portgen(1)

Andrew Hewus Fresh
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 | Threaded
Open this post in threaded view
|

Re: Better type error for portgen(1)

Kurt Mosiejczuk-9
On Sun, Jul 07, 2019 at 06:22:18PM -0700, Andrew Hewus Fresh wrote:

> > 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.

That's going more advanced with the perl than I know. That is a lot
harder for *me* to follow anyway. The other thing I did like about the
old structure is I was considering proposing aliases for the modules.
The modules are (mostly?) named after where we are pulling the port
from, but we are using the prefix of the port to specify each one.  The
other portgen-like tool I used I would specify pypi, so I had been
looking at a patch to add "pypi" as an option for python ports and
"cpan" and/or "perl" as an option for the CPAN module. Doing it this way
you'll end up muddying the usage and/or error messages.

I personally would vote for your first proposal on this.

--Kurt

> 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;
>