[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: Stanford IDG internal Perl style



On 2013-03-29 18:24, Russ Allbery wrote:
> * Use Getopt::Long::Descriptive for option parsing in any script that
> takes options. Give each option a corresponding single-character option
> unless it's rarely used. When listing the options, give the short
> option first, then |, and then the long option, as in:
> 

I tried to convert lintian-info to Getopt::Long::Descriptive today.
Attached is a functional patch as far as I have tested.
  I wonder why you recommend the use of the "short" option first.  It
seems to me that putting the long option first will be much more
descriptive later.  Example from lintian-info being:

  $opt->mode eq 't' instead of $opt->mode eq 'tags'
  $opt->h instead of $opt->help

Regarding the use of:

Usage: lintian-info ...
Usage: lintian-info --annotate
Usage: lintian-info --tags ...

For some reason, it only treats newlines as newlines if they are not
followed by \t or space.  Possible reasons could be that the string is
"s/\s\s++/ /"'ed, but I haven't checked the code.

>         my ($opt, $usage = describe_options(
>           '$0 $o <args>',
>           [ 'd|delete', 'delete the given object' ],
>           [ 'e|exclude=s', 'exclude any matching names' ],
>           [ 'h|help',      'display help' ],
>         ) or exit 1;

The "or exit 1" appear to be useless.  AFAICT, describe_options does not
return on bad options.  Instead it exists with 255 (I suspect it ends up
calling die via $usage->die).
  The upstream documentation is sadly not very helpful here, so take it
as an "educated guess".

~Niels


diff --git a/frontend/lintian-info b/frontend/lintian-info
index 997e76a..bfceb29 100755
--- a/frontend/lintian-info
+++ b/frontend/lintian-info
@@ -24,6 +24,7 @@ use strict;
 use warnings;
 
 use Getopt::Long;
+use Getopt::Long::Descriptive;
 
 # turn file buffering off:
 $| = 1;
@@ -42,49 +43,51 @@ use Lintian::Internal::FrontendUtil qw(split_tag);
 use Lintian::Profile;
 
 my %already_displayed = ();
-my ($annotate, $tags, $help, $prof);
-my $user_dirs = 1; # Allow user dirs ($HOME/.lintian and /etc/lintian) by default
+my ($annotate, $tags);
 my $profile;
 my @dirs = ();
 
+my @mode_specs = (
+    ['a|annotate', 'display descriptions of ags in Lintian overrides'],
+    ['t|tags', 'display tag descriptions'],
+    ['logfile', 'hidden/default', { 'hidden' => 1 }],
+);
+
+my @option_specs = (
+    ['mode', \@mode_specs, { 'hidden' => 1, 'default' => 'logfile'}],
+    ['profile=s', 'vendor profile to use for determining severities',
+        { 'default' => '{VENDOR}' }],
+    ['include-dir=s@', 'check dir for Lintian support files',
+        { 'default' => [] }],
+    ['user-dirs!', 'whether to include profiles from user directories',
+        { 'default' => 1 }],
+    ['h|help', 'print this help message'],
+);
+
 Getopt::Long::config('bundling', 'no_getopt_compat', 'no_auto_abbrev');
-GetOptions(
-    'annotate|a' => \$annotate,
-    'tags|t' => \$tags,
-    'help|h' => \$help,
-    'profile=s' => \$prof,
-    'user-dirs!' => \$user_dirs,
-    'include-dir=s' => \@dirs,
-) or die("error parsing options\n");
+my ($opt, $usage) = describe_options(
+    usage_header(),
+    @option_specs,
+);
 
 # help
-if ($help) {
-    print <<"EOT";
-Usage: lintian-info [log-file...] ...
-       lintian-info --annotate [overrides ...]
-       lintian-info --tags tag ...
-
-Options:
-    -a, --annotate    display descriptions of tags in Lintian overrides
-    -t, --tags        display tag descriptions
-    --profile X       use vendor profile X to determine severities
-    --include-dir DIR check for Lintian data in DIR
-    --[no-]user-dirs  whether to include profiles from user directories
-EOT
-
+if ($opt->h) {
+    print $usage;
     exit 0;
 }
 
-if ($user_dirs) {
+if ($opt->user_dirs) {
     # If requested, pre-append user dirs
-    unshift @dirs, '/etc/lintian';
-    unshift @dirs, "$ENV{'HOME'}/.lintian" if exists $ENV{'HOME'};
+    push(@dirs, '/etc/lintian');
+    push(@dirs, "$ENV{'HOME'}/.lintian") if exists $ENV{'HOME'};
 }
+push(@dirs, @{ $opt->include_dir });
+
 # Add the Lintian root in the end...
 push @dirs, $ENV{'LINTIAN_ROOT'} if exists $ENV{'LINTIAN_ROOT'};
 push @dirs, '/usr/share/lintian' unless exists $ENV{'LINTIAN_ROOT'};
 
-$profile = Lintian::Profile->new ($prof, \@dirs);
+$profile = Lintian::Profile->new ($opt->profile, \@dirs);
 
 Lintian::Data->set_vendor ($profile);
 
@@ -92,7 +95,7 @@ Lintian::Data->set_vendor ($profile);
 # descriptions for each one.  (We don't currently display the severity,
 # although that would be nice.)
 my $unknown;
-if ($tags) {
+if ($opt->mode eq 't') {
     for my $tag (@ARGV) {
         my $info = $profile->get_tag ($tag, 1);
         if ($info) {
@@ -124,7 +127,7 @@ while (<>) {
     s,</span>,,g;
 
     my $tag;
-    if ($annotate) {
+    if ($opt->mode eq 'a') {
         my $tagdata;
         next unless m/^(?:                         # start optional part
                     (?:\S+)?                       # Optionally starts with package name
@@ -149,6 +152,13 @@ while (<>) {
 
 exit 0;
 
+sub usage_header {
+    return join("\n", 'Usage: %c [log-file...]',
+                'Usage: %c --annotate [overrides ...]',
+                'Usage: %c --tags tag [...]');
+
+}
+
 # Local Variables:
 # indent-tabs-mode: nil
 # cperl-indent-level: 4

Reply to: