git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 5/5] gitweb: Use block form of map/grep in a few cases more

From
Jakub Narebski <jnareb@gmail.com>
Date
May 10, 2009, 00:40 UTC
Message-ID
<200905100240.37772.jnareb@gmail.com>
In-Reply-To
<200905100203.51744.jnareb@gmail.com>

Use block form of 'grep' i.e. 'grep {BLOCK} LIST' rather than 'grep(EXPR, LIST)' in filter_snapshot_fmts subroutine. This makes code more readable, as expression is rather long, and statement above there is 'map' with very similar expression also in the block form.

Remove unnecessary and misleading parentheses around block form 'map' arguments in quote_command subroutine.

The inner "map" in format_snapshot_links was left alone, as it is not clear whether adding parentheses or changing it into block form would improve readibility and clarity of this code.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
Perl::Critic::Policy::BuiltinFunctions::RequireBlockGrep
  Write grep { $_ =~ /$pattern/ } @list instead of grep /$pattern/, @list.
  The expression forms of grep and map are awkward and hard to read. Use the
  block forms instead.

See also Damian Conway's book "Perl Best Practices", section 8.13. Mapping and Grepping (Always use a block with a map and grep.)

NOTE: In my opinion the expression form when using function-like call to
"grep" or "map" e.g. grep(/$pattern/, @list) is readable enough.  In more
complicated cases (with more complicated expressions, especially with
explicit $_) it might be better to use block form instead, as stated above.
And what do *you* think?
 gitweb/gitweb.perl |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 1cb3a4f..f465666 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -458,8 +458,8 @@ sub filter_snapshot_fmts {
 	@fmts = map {
 		exists $known_snapshot_format_aliases{$_} ?
 		       $known_snapshot_format_aliases{$_} : $_} @fmts;
-	@fmts = grep(exists $known_snapshot_formats{$_}, @fmts);
-
+	@fmts = grep {
+		exists $known_snapshot_formats{$_} } @fmts;
 }
 
 our $GITWEB_CONFIG = $ENV{'GITWEB_CONFIG'} || "++GITWEB_CONFIG++";
@@ -1838,7 +1838,7 @@ sub git_cmd {
 # Try to avoid using this function wherever possible.
 sub quote_command {
 	return join(' ',
-		    map( { my $a = $_; $a =~ s/(['!])/'\\$1'/g; "'$a'" } @_ ));
+		map { my $a = $_; $a =~ s/(['!])/'\\$1'/g; "'$a'" } @_ );
 }
 
 # get HEAD ref of given project as hash
-- 
1.6.3
Previous: Jakub NarebskiNext: Junio C Hamano
Message 11 of 16 in “gitweb: Some code cleanups (up to perlcritic --stern)”
  1. 0/5 gitweb: Some code cleanups (up to perlcritic --stern)Jakub Narebski, May 10, 2009
  2. 1/5 gitweb: Remove function prototypesJakub Narebski, May 10, 2009
  3. Jakub NarebskiMay 10, 2009
  4. 2/5 gitweb: Do not use bareword filehandlesJakub Narebski, May 10, 2009
  5. Petr BaudisMay 10, 2009
  6. Jakub NarebskiMay 10, 2009
  7. 2/5 gitweb: Do not use bareword filehandlesJakub Narebski, May 11, 2009
  8. 3/5 gitweb: Always use three argument form of openJakub Narebski, May 10, 2009
  9. 3/5 gitweb: Always use three argument form of openJakub Narebski, May 11, 2009
  10. 4/5 gitweb: Localize magic variable $/Jakub Narebski, May 10, 2009
  11. 5/5 gitweb: Use block form of map/grep in a few cases moreJakub Narebski, May 10, 2009
  12. Junio C HamanoMay 11, 2009
  13. Jakub NarebskiMay 11, 2009
  14. Junio C HamanoMay 11, 2009
  15. Daniel PittmanMay 11, 2009
  16. Jakub NarebskiMay 11, 2009

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.