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

Re: RFC: [PATCH] Make "lintian --info --color=auto" work again



On Sat, 2008-11-29 at 17:00 +0100, Frank Lichtenheld wrote:
> While I don't doubt that your patch accomplishes it goal, I wonder
> whether it would be better to fix this "correctly" now. By this
> I mean:
> 
> 1) The extended description should be available in the $tag_info
> 2) The list of already issued tags should probably be a object
>    attribute instead of a module attribute.
> 3) I'm unsure whether the flag show_description shouldn't be an
>    object attribute of Lintian::Output instead of a module
>    attribute of Tags.

Thanks for the feedback. I've reimplemented the changes bearing your
suggestions in mind, and attached an updated patch.

Descriptions are now stored along with the rest of the tag data read by
the frontend. I considered doing it directly in Tags.pm but it seemed
cleaner not to have add_tag() modify the tag data directly. The
formatting work itself is all abstracted in Read_taginfo.pm so it can be
reused elsewhere.

The Output module has gained two attributes indicating which tags have
been issued by the module, and whether descriptions should be displayed.

Regards,

Adam
diff --git a/debian/changelog b/debian/changelog
index fea113e..1c1ac9d 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -12,15 +12,26 @@ lintian (2.1.1) unstable; urgency=low
       better match executions of the command rather than the use of its name
       in pattern matching expressions.  (Closes: #499847)
 
+  * frontend/lintian:
+    + [ADB] Don't call lintian-info when --info is specified.  Set a new
+      flag on the Lintian::Output module instead.
+    + [ADB] Add the description to the list of information stored about each
+      tag.
+
   * lib/Lintian/Output.pm:
     + [ADB] Make sure the default list of colours is initialised before
       attempting to use it.  Thanks, gregor herrmann.  (Closes: #507241)
+    + [ADB] Maintain a list of tags which have been issued.
+    + [ADB] Optionally include the description of a tag when displaying it.
   * lib/Lintian/Output/XML.pm:
     + [ADB] Add a missing angle bracket to the closing "tag" tag, so that
       the XML is well-formed.
 
   * lib/Lab.pm:
     + [FL] Fix breakage of populate_with_dist() I introduced in 2.1.0.
+  * lib/Read_taginfo.pm:
+    + [ADB] Move the work of formatting a tag's description to a separate
+      function so that it can be called from multiple locations.
   * lib/Tags.pm:
     + [ADB] Don't output a "Processing binary package foo.changes" message.
       Thanks Sandro Tosi for pointing the issue out.
diff --git a/frontend/lintian b/frontend/lintian
index fe9a823..66beaba 100755
--- a/frontend/lintian
+++ b/frontend/lintian
@@ -29,7 +29,6 @@ use FileHandle;
 # }}}
 
 # {{{ Global Variables
-my $lintian_info_cmd = 'lintian-info'; #Command to run for ?
 my $LINTIAN_VERSION = "<VERSION>";	#External Version number
 my $BANNER = "Lintian v$LINTIAN_VERSION"; #Version Banner - text form
 my $LAB_FORMAT = 8;		#Lab format Version Number
@@ -473,10 +472,6 @@ if (defined $LINTIAN_ROOT) {
     unless ($LINTIAN_ROOT =~ m,^/,) {
 	$LINTIAN_ROOT = "$cwd/$LINTIAN_ROOT";
     }
-    # see if it has a frontend directory
-    if (-d "$LINTIAN_ROOT/frontend") {
-        $lintian_info_cmd = "$LINTIAN_ROOT/frontend/lintian-info";
-    }
 } else {
     $LINTIAN_ROOT = '/usr/share/lintian';
 }
@@ -626,6 +621,7 @@ require Lab;
 
 require Util;
 require Read_pkglists;
+require Read_taginfo;
 
 import Util;
 
@@ -663,6 +659,7 @@ $Lintian::Output::GLOBAL->verbose($verbose);
 $Lintian::Output::GLOBAL->debug($debug);
 $Lintian::Output::GLOBAL->quiet($quiet);
 $Lintian::Output::GLOBAL->color($color);
+$Lintian::Output::GLOBAL->showdescription($lintian_info);
 
 # Print Debug banner, now that we're finished determining
 # the values and have Lintian::Output available
@@ -680,7 +677,8 @@ debug_msg(1,
 
 my @l_secs = read_dpkg_control("$LINTIAN_ROOT/checks/lintian.desc");
 shift(@l_secs);
-map { $_->{'script'} = 'lintian'; Tags::add_tag($_) } @l_secs;
+map { $_->{'script'} = 'lintian'; $_->{'description'} =
+  format_tag_description($_, 3); Tags::add_tag($_) } @l_secs;
 
 $Tags::show_experimental = $display_experimentaltags;
 $Tags::show_overrides = $show_overrides;
@@ -747,29 +745,6 @@ $LINTIAN_LAB = $LAB->{dir};
 
 # }}}
 
-# {{{ Setup the lintian-info pipe
-
-# pipe output through lintian-info?
-# note: if any E:/W: lines are added above this point, this block needs to
-#       be moved up
-if ($lintian_info) {
-    open(OUTPUT_PIPE, '|-', $lintian_info_cmd) or fail("cannot open output pipe to $lintian_info_cmd: $!");
-    $Lintian::Output::GLOBAL->stdout(\*OUTPUT_PIPE);
-}
-
-# Close the OUTPUT_PIPE in an END block so that we can ensure that
-# lintian-info exits before we do.  Otherwise, we may get output from
-# lintian-info after lintian exits, which can confuse the shell output.
-END {
-    if ($lintian_info) {
-	my $status = $?;
-	close OUTPUT_PIPE;
-	$Lintian::Output::GLOBAL->stdout(\*STDOUT);
-	$? = $status;
-    }
-}
-# }}}
-
 # {{{ Compile list of files to process
 
 $schedule = new Lintian::Schedule(verbose => $verbose);
@@ -1176,6 +1151,7 @@ for my $f (readdir CHECKDIR) {
 	$p->{'requested-tags'} = 0;
 	foreach my $tag (@secs) {
 	    $tag->{'script'} = $script;
+	    $tag->{'description'} = format_tag_description($tag, 3);
 	    Tags::add_tag($tag);
 	    $p->{'requested-tags'}++ if Tags::display_tag($tag);
 	}
diff --git a/lib/Lintian/Output.pm b/lib/Lintian/Output.pm
index 02beb03..1ec7ecf 100644
--- a/lib/Lintian/Output.pm
+++ b/lib/Lintian/Output.pm
@@ -103,7 +103,8 @@ I/O handle to use for warnings.  Defaults to C<\*STDERR>.
 =cut
 
 use base qw(Class::Accessor Exporter);
-Lintian::Output->mk_accessors(qw(verbose debug quiet color colors stdout stderr));
+Lintian::Output->mk_accessors(qw(verbose debug quiet color colors stdout
+    stderr showdescription issuedtags));
 
 our @EXPORT = ();
 our %EXPORT_TAGS = ( messages => [qw(msg v_msg warning debug_msg delimiter)],
@@ -126,6 +127,7 @@ sub new {
     $self->stdout(\*STDOUT);
     $self->stderr(\*STDERR);
     $self->colors({%default_colors});
+    $self->issuedtags({});
 
     return $self;
 }
@@ -209,6 +211,19 @@ sub delimiter {
     return $self->_delimiter;
 }
 
+=item C<issued_tag($tag_name)>
+
+Indicate that the named tag has been issued.  Returns a boolean value
+indicating whether the tag had previously been issued by the object.
+
+=cut
+
+sub issued_tag {
+    my ($self, $tag_name) = _global_or_object(@_);
+
+    return $self->issuedtags->{$tag_name}++ ? 1 : 0;
+}
+
 =item C<string($lead, @args)>
 
 TODO: Is this part of the public interface?
@@ -269,6 +284,11 @@ sub print_tag {
     }
 
     $self->_print('', "$code: $pkg_info->{pkg}$type", "$tag$extra");
+    if (!$self->issued_tag($tag_info->{tag}) and $self->showdescription) {
+	$self->_print('', 'N', '');
+	$self->_print('', 'N', split("\n", $tag_info->{description}));
+	$self->_print('', 'N', '');
+    }
 }
 
 =item C<print_start_pkg($pkg_info)>
diff --git a/lib/Read_taginfo.pm b/lib/Read_taginfo.pm
index dbcd464..cade0ec 100644
--- a/lib/Read_taginfo.pm
+++ b/lib/Read_taginfo.pm
@@ -41,8 +41,6 @@ sub read_tag_info {
     my %tag_info;
     if (defined $type && $type eq 'html') {
 	$dtml_convert = \&dtml_to_html;
-    } else {
-	$dtml_convert = \&dtml_to_text;
     }
 
  #   $debug = 2;
@@ -54,35 +52,51 @@ sub read_tag_info {
 
 	for (my $i=1; $i<=$#secs; $i++) {
 	    (my $tag = $secs[$i]->{'tag'}) or fail("error in description file $f: section $i does not have a `Tag:'");
-
-	    my @foo = split_paragraphs($secs[$i]->{'info'});
-	    if ($secs[$i]->{'ref'}) {
-		push(@foo,"");
-		push(@foo,format_ref($secs[$i]->{'ref'}));
-	    }
-
-	    if ($secs[$i]->{'severity'} and $secs[$i]->{'certainty'}) {
-		push(@foo, "");
-		push(@foo, "Severity: $secs[$i]->{'severity'}; " .
-		           "Certainty: $secs[$i]->{'certainty'}");
-	    }
-
-	    if ($secs[$i]->{'experimental'}) {
-		push(@foo,"");
-		push(@foo,"Please note that this tag is marked Experimental, which "
-		     . "means that the code that generates it is not as well tested "
-		     . "as the rest of Lintian, and might still give surprising "
-		     . "results.  Feel free to ignore Experimental tags that do not "
-		     . "seem to make sense, though of course bug reports are always "
-		     . "welcomed.");
-	    }
-
-	    $tag_info{$tag} = join("\n",&$dtml_convert(@foo));
+	    $tag_info{$tag} = format_tag_description($secs[$i], 0, $dtml_convert);
 	}
     }
+
     return \%tag_info;
 }
 
+sub format_tag_description {
+    my $tag=shift;
+    my $indent=shift;
+    my $dtml_convert=shift;
+
+    if (not defined $dtml_convert) {
+	$dtml_convert = \&dtml_to_text;
+    }
+
+    my @foo = split_paragraphs($tag->{'info'});
+    if ($tag->{'ref'}) {
+	push(@foo,"");
+	push(@foo,format_ref($tag->{'ref'}));
+    }
+
+    if ($tag->{'severity'} and $tag->{'certainty'}) {
+	push(@foo, "");
+	push(@foo, "Severity: $tag->{'severity'}; " .
+	           "Certainty: $tag->{'certainty'}");
+    }
+
+    if ($tag->{'experimental'}) {
+	push(@foo,"");
+	push(@foo,"Please note that this tag is marked Experimental, which "
+	     . "means that the code that generates it is not as well tested "
+	     . "as the rest of Lintian, and might still give surprising "
+	     . "results.  Feel free to ignore Experimental tags that do not "
+	     . "seem to make sense, though of course bug reports are always "
+	     . "welcomed.");
+    }
+
+    if ($indent) {
+	return wrap_paragraphs(" " x $indent, join("\n",&$dtml_convert(@foo)));
+    } else {
+	return join("\n",&$dtml_convert(@foo));
+    }
+}
+
 sub manual_ref {
     my ($man, $sub) = @_;
     my $numbered = ($sub =~ /[A-Z\d\.]+/) ? 1 : 0;

Reply to: