[gentoo-perl] g-cpan patch

[gentoo-perl] g-cpan patch

[dams]

[-- Attachment #1: Type: text/plain, Size: 374 bytes --]

Hi

While looking at the g-cpan code, I have coded some stuff, here is the patch. Remarks welcome. If you are pleased with it, I'll commit it.

Features/changes :

- recoded the small options verification
- implemented a Fake CPAN::Frontend, to catch the output of the CPAN search
- added a small spin animation to the cpan search
- wrote the cpan search result output

dams

[-- Attachment #2: g-cpan_patch.diff --]
[-- Type: application/octet-stream, Size: 3990 bytes --]

Index: bin/g-cpan
===================================================================
--- bin/g-cpan	(revision 98)
+++ bin/g-cpan	(working copy)
@@ -44,9 +44,13 @@
 use Getopt::Long;
 Getopt::Long::Configure("bundling");
 
-# Init all options (has to be done to perform the 'sum test' later)
-my ($debug, $verbose, $search, $install, $upgrade, $generate, $list, $pretend, $ask, $cpan_reload, $buildpkg, $buildpkgonly, $log) =
-  (0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0);
+# Declare options
+# First, the main switches
+my @main_switches = \my ($search, $list, $install, $generate, $ask, $buildpkg, $buildpkgonly);
+# Then, additional switches
+my @additional_switches = \my ($upgrade, $pretend);
+# Then, the normal options
+my ($debug, $verbose, $cpan_reload, $log);
 
 # Set colors here so we can use them at will anywhere :)
 my $green = color("bold green");
@@ -118,20 +122,20 @@
 # Array that will contain the portage friendly version of the values passed to install
 my %really_install;
 
-# Output error if more than one switch is activated
+# Output error if more than one main switch is activated
 #
-if ($search + $list + $install + $generate + $ask + $buildpkg + $buildpkgonly > 1)
+
+if ( (grep { defined $$_ } @main_switches) > 1)
 {
     print_err($prog, "You can't combine actions with each other.\n");
     print_out("${white}Please consult ${cyan}$prog ${green}--help${reset} or ${cyan}man $prog${reset} for more information\n\n");
     exit();
 }
 
-if ($search + $list + $install + $generate + $upgrade + $pretend + $ask + $buildpkg + $buildpkgonly == 0)
+if ( ! grep { defined $$_ } @main_switches, @additional_switches )
 {
     print_err($prog, "You haven't told $prog what to do.\n");
     print_out("${white}Please consult ${cyan}$prog ${green}--help${reset} or ${cyan}man $prog${reset} for more information\n\n");
-
     exit();
 }
 
@@ -306,8 +310,7 @@
 # Set portage_categories to our defined list of perl_dirs
 $gcpan_run->{portage_categories} = \@perl_dirs;
 
-# Taking care of Searches. This has to be improved a lot, since it uses a call to
-# CPAN Shell to do the job, thus making it impossible to have a clean output..
+# Taking care of Searches.
 if ($search)
 {
     foreach my $expr (@ARGV)
@@ -324,17 +327,51 @@
         {
             print_info($prog, "No ebuild exists, pulling up CPAN listings for $expr");
 
-        }
-
         # Assume they gave us module-name instead of module::name
         # which is bad, because CPAN can't convert it ;p
 
-        print_ok($prog, "Searching for $expr on CPAN") if ($verbose);
-        unless (CPAN::Shell->i("/$expr/"))
-        {
-            $expr =~ s/-/::/g;
+            print_ok($prog, "Searching for $expr on CPAN") if ($verbose);
+
+	    my @search_results;
+
+            # Let's define a CPAN::Frontend to use our printing methods
+	    {
+	        package Gentoo::CPAN::FakeFrontend;
+                my $spin = 0;
+		sub myprint {
+		    my ($self, $text) = @_;
+		    print "\r".('/', '-', '\\', '|')[$spin++%4];
+		    # if there is only one result, the string is different
+		    $text =~ /Module id = (.*)/ and push(@search_results, $1), return;
+		    # in the case of multiple result
+		    if ($text =~ /Module\s+(\S+)/) {
+		        foreach (split(/\n/, $text)) {
+		            $_ =~ /Module\s+(\S+)/ or next;
+			    push @search_results, $1;
+		        }
+		    }
+		}
+	    }
+		    
+	    $CPAN::Frontend = "Gentoo::CPAN::FakeFrontend";
+		
             CPAN::Shell->i("/$expr/");
-        }
+            if (!@search_results) {
+                $expr =~ s/-/::/g;
+                CPAN::Shell->i("/$expr/");
+            }
+
+            # remove the spin
+            print "\r \r";
+
+	    # now, @search_results should contain the matching modules strings, if any
+            if (@search_results) {
+	        print_info('g-cpan', @search_results . " results found : @search_results");
+	    } else {
+	        print_warn('no result found.');
+	    }
+
+	}
     }
 
     exit;

Re: [gentoo-perl] g-cpan patch

[Christian Hartmann]

> - recoded the small options verification

Looks good.

> - implemented a Fake CPAN::Frontend, to catch the output of the CPAN search

Eek! That's an awful hack. That's not how code looks like I want to maintain. 
E.g. the 'package Gentoo::CPAN::FakeFrontend' part.

> - added a small spin animation to the cpan search

I think I don't like spinners but I'm actually not really sure about that one. 
Spinners just cause the process to take longer than it would take anyways 
without a spinner. (I'm pretty much against everything these days so don't 
take me too seroius on this one. ;)) Nevertheless I put that in a module 
called Gentoo::Portage::UI::Spinner so it can be reused this way e.g.:

my $spinner=Gentoo::Portage::UI::Spinner->new();
for (my $i=0;$i<5;$i++)
{
	$spinner->spin();
	sleep(1);
}
$spinner->reset();

Yeah.. overhead.. but reusable and more easy to read.

Anyways. The whole Gentoo::Portage stuff needs to be ripped out (of g-cpan, 
up2date-ng, mgtool, demerge, perl-info and some of my other endless and 
nameless tools/hacks) and put together nicely in a neat and slick module. I'm 
currently working on it. - Ok, ok.. I'm short of time but anyways.. - If it's 
all set and done I'll commit it to our gentoo-perl svn, so that we can work 
on it together. One thing I'd like to see established until then is one 
documented and persistent coding style. Furthermore all functions/code should  
be documented and a complete api-documentation about Gentoo::Portage should 
be available. We need a todo-list, a feature-list and a roadmap. - Big 
words.. ;)

I'm pretty serious about it. The reason I'm writing this is that I don't want 
to have it turning into another big and ugly hack. I want a clean and well 
documented api to access and manipulate portage using perl that all our 
little tools can use.

For now I'm asking you not to commit anything until I've got that 
Gentoo::Portage thing ready. - BUT please don't stop hacking on it and coming 
up with ideas and stuff. :)

I just want to ensure that all our efforts go in the same direction.

-- 
Christian Hartmann
http://www.gentoo.org/~ian/

PGP Key:
http://pgp.mit.edu:11371/pks/lookup?op=get&search=0x2154E5EE692A4865
Key fingerprint = 4544 EC0C BAE4 216F 5981  7F95 2154 E5EE 692A 4865

-- 
gentoo-perl@gentoo.org mailing list

Re: [gentoo-perl] g-cpan patch

[Chris White]

[-- Attachment #1: Type: text/plain, Size: 597 bytes --]

On Saturday 09 September 2006 16:19, dams wrote:
> Hi
>
> While looking at the g-cpan code, I have coded some stuff, here is the
> patch. Remarks welcome. If you are pleased with it, I'll commit it.
>
> Features/changes :
>
 * unimportant stuff*
> - added a small spin animation to the cpan search

You've got my vote!

>
> dams

/me runs ;p

-- 
Chris White
Gentoo Developer aka:
ChrisWhite		cpw
ChrisWhite|Work	WhiteChocolate
VanillaWhite		Whitey
WhiteLight		WhiteCheese
WhiteSugar		WhiteButter
WhiteWall		WhiteLemon
WhiteApple		WhiteBlanket
WhiteEnergy		WhiteWhite

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [gentoo-perl] g-cpan patch

[dams]

On Mon, 11 Sep 2006 11:34:23 +0200
Christian Hartmann <ian@gentoo.org> wrote:

> > - recoded the small options verification
> 
> Looks good.
> 
> > - implemented a Fake CPAN::Frontend, to catch the output of the CPAN search
> 
> Eek! That's an awful hack. That's not how code looks like I want to maintain. 
> E.g. the 'package Gentoo::CPAN::FakeFrontend' part.

It doesn't look like a hack for me. Before coding that, I've looked carefully at the CPAN code, and it's obvious (at least for me) that the author thought about people willing to implement their own Frontend to CPAN.pm, for integration purpose. Now, our search should actually use something more backend-ish, but I didn't want to dig too much in the cpan code.

Anyway, my opinion is that creating a frontend for cpan is the good way(tm), when you want to interact with high level features of CPAN.pm.

> 
> > - added a small spin animation to the cpan search
> 
> I think I don't like spinners but I'm actually not really sure about that one.

Actually, I also usually dislike spinners. That's the stuff that need a way to be tunred off. But I thiought it was a good thing to let the user know that something is being done.
 
> Spinners just cause the process to take longer than it would take anyways 
> without a spinner. (I'm pretty much against everything these days so don't 
> take me too seroius on this one. ;)) Nevertheless I put that in a module 
> called Gentoo::Portage::UI::Spinner so it can be reused this way e.g.:
> 
> my $spinner=Gentoo::Portage::UI::Spinner->new();
> for (my $i=0;$i<5;$i++)
> {
> 	$spinner->spin();
> 	sleep(1);
> }
> $spinner->reset();
> 
> Yeah.. overhead.. but reusable and more easy to read.


No, I'm all for it ! I would have coded UI::Spinner, but I wasn't sure you would accept the spinner concept in the first place.

> 
> Anyways. The whole Gentoo::Portage stuff needs to be ripped out (of g-cpan, 
> up2date-ng, mgtool, demerge, perl-info and some of my other endless and 
> nameless tools/hacks) and put together nicely in a neat and slick module. I'm 
> currently working on it. - Ok, ok.. I'm short of time but anyways.. -

Agreed. I think the author of ufed is also willing to do that. And I have a lot of code to handle USE flags and cascading profiles in profuse and flagedit. I should probably take it from flagedit and put it in Gentoo::Portage. (btw, use flagedit, it's a killer-app :)


> If it's 
> all set and done I'll commit it to our gentoo-perl svn, so that we can work 
> on it together. One thing I'd like to see established until then is one 
> documented and persistent coding style. Furthermore all functions/code should  
> be documented and a complete api-documentation about Gentoo::Portage should 
> be available. We need a todo-list, a feature-list and a roadmap. - Big 
> words.. ;)
> 
> I'm pretty serious about it. The reason I'm writing this is that I don't want 
> to have it turning into another big and ugly hack. I want a clean and well 
> documented api to access and manipulate portage using perl that all our 
> little tools can use.

I agree. I code perl for 7 years now, and I have some experience about coding style and documentation. However, my experience and opinion are probably not the same as yours, so I don't want to oblige/force anything.

Here is a small rules I use :

- put { at the end of the line
- put a space between command (if foreach while ...) and the first parenthesis
- don't put a space between methods and their args
- put space around operators
- no space after the first ( and before the last )
- 4 space indentation (no tabs)

example of the above :
if ($foo = 1) {
    do { $something }
    method($param1)
}

now VERY important : 
- never use more ( ) than needed, see 'man perlop' and learn that by eart
- always use '&&' and '||' for BOOLEAN tests. Use 'and' and 'or' for CONDITIONAL tests

This needs explanation :

Correct :  open(FILE, "/tmp/foo") or die "damned!";
in this case the 'or' is like in english :  "please open the file, or else, die."

Correct : if ($foo == 3 && $bar > 2) { ... } #this is a BOOLEAN test

Wrong :  open(FILE, "/tmp/foo") || die; # it's not a BOOLEAN test

Wrong if ($foo == 3 and $bar > 3) {}

Correct : 
foreach (1..10) { 
    is_ok($_) && looks_good($_) or next;
    $_ < 5 and warn "hm, number is < 5, continuing\n";
}

good handling of 'and', 'or', '&&', '||' allows to be more expressive, and to reduce useless ()


The rest of the list of rules :
- never use 'unless', never use 'until'
- never the suffix 'if'. insted of 
do_something() if $ok    write
$ok and do_something().
The order of "condition", then "action" is the same as   if($ok) { do_something() }
- remove useless ';' (before a } )
- private things must start with a underscore : sub _privatemethod { }  or   $self->{_privateattr}

There are some others, but I cannot remember them yet, I have to code to remember :)


> 
> For now I'm asking you not to commit anything until I've got that 
> Gentoo::Portage thing ready. - BUT please don't stop hacking on it and coming 
> up with ideas and stuff. :)

I wouldn't recommand this decision. When new people come and propose a patch and commit, it's usually not good to ask them to stop working/commiting : you'll demotivate them quickly. Better do a branch, and remerge. But let us some sandbox at least.

> 
> I just want to ensure that all our efforts go in the same direction.


I'm willing to refactore and document the whole code, once we have decided rules and todos. Oh, about documentation, please please please, don't go into "dowegen stuff", or "huge header for every methods (even one liners)". I would recommend :

- use POD, use all the features of it.
- Document INLINE, not a but pod at the end of the file (use =cut to ... cut your pod :)
- be nice with the coder : have mandatory to document every single piece of pulic API, but don't be too strict with public methods.



I'm going to have 3 heavy weeks, but then I'll be able to code a lot. During that time, I would be happy to share ideas/remarks, especially about coding style and todos.

I'll talk about new features we thought about with cab, in a next mail :)


cu
-- 
gentoo-perl@gentoo.org mailing list