Hello Michael
Just a small suggestion. Instead of working with $_ you can directly create a
variable. This way it's a bit more obvious what is going on and one doesn't
provoke strange side-effects if something else is storing stuff in $_.
version 2 is even more on the 'safe side' by explicitely next-ing via a label.
I like labels. Code-wise there's no difference but it's nice if the code says
what it is looping over. (I hope I got it right it's a command. On first
reading this code fragment I thought it was a dependency.)
tschüß
thomas, trying to wrap his head around Commands.pm::order_commands()
patch version1)
--- vanilla/Commands.pm 2012-06-25 17:43:00.000000000 +0200
+++ patched/Commands.pm 2012-06-25 21:52:08.147285563 +0200
@@ -1333,8 +1333,7 @@
}
my $all_matched = 1;
if (defined($FAI::commands{$i}{pre})) {
- foreach (split(/,/, $FAI::commands{$i}{pre})) {
- my $cur = $_;
+ foreach my $cur (split(/,/, $FAI::commands{$i}{pre})) {
next if scalar(grep(m{^$cur$}, @pre_deps));
$all_matched = 0;
last;
patch version2)
--- vanilla/Commands.pm 2012-06-25 17:43:00.000000000 +0200
+++ patched/Commands.pm 2012-06-25 22:02:37.766628161 +0200
@@ -1333,9 +1333,11 @@
}
my $all_matched = 1;
if (defined($FAI::commands{$i}{pre})) {
- foreach (split(/,/, $FAI::commands{$i}{pre})) {
- my $cur = $_;
- next if scalar(grep(m{^$cur$}, @pre_deps));
+ COMMAND:
+ foreach my $cur (split(/,/, $FAI::commands{$i}{pre})) {
+ if scalar(grep(m{^$cur$}, @pre_deps)) {
+ next COMMAND;
+ }
$all_matched = 0;
last;
}