All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jakub Narebski <jnareb@gmail.com>
To: git@vger.kernel.org
Subject: [PATCHv2 1/5] gitweb: Remove function prototypes
Date: Sun, 10 May 2009 02:05:23 +0200	[thread overview]
Message-ID: <200905100205.23733.jnareb@gmail.com> (raw)
In-Reply-To: <200905100203.51744.jnareb@gmail.com>

Use of function prototypes is considered bad practice in Perl.  The
ones used here didn't accomplish anything anyhow, so they've been
removed.

>From perlsub(1):

  [...] the intent of this feature [prototypes] is primarily to let
  you define subroutines that work like built-in functions [...]
  you can generate new syntax with it [...]

We don't want to have subroutines behaving exactly like built-in
functions, we don't want to define new syntax / syntactic sugar, so
prototypes in gitweb are not needed... and they can have unintended
consequences.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
Perl::Critic::Policy::Subroutines::ProhibitSubroutinePrototypes

  Don't write 'sub my_function (@@) {}'.

  Contrary to common belief, subroutine prototypes do not enable
  compile-time checks for proper arguments. Don't use them.

See also Damian Conway's book "Perl Best Practices",
chapter "9.10. Prototypes" (Don't use subroutine prototypes.)


This follows similar patch for git-send-email.perl by Bill Pemberton.
Also "sub S_ISGITLINK($) {" line caused `imenu` in my old cperl-mode
(4.23) in GNU Emacs 21.4.1 to fail, sometimes.

This patch was send with slightly different commit message as
standalone patch earlier. This is the replacement patch, which differs
only in the commit message.

 gitweb/gitweb.perl |   12 +++++-------
 1 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 3f99361..06e9160 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -838,7 +838,7 @@ exit;
 ## ======================================================================
 ## action links
 
-sub href (%) {
+sub href {
 	my %params = @_;
 	# default is to use -absolute url() i.e. $my_uri
 	my $href = $params{-full} ? $my_url : $my_uri;
@@ -1036,7 +1036,7 @@ sub esc_url {
 }
 
 # replace invalid utf8 character with SUBSTITUTION sequence
-sub esc_html ($;%) {
+sub esc_html {
 	my $str = shift;
 	my %opts = @_;
 
@@ -1296,7 +1296,7 @@ use constant {
 };
 
 # submodule/subproject, a commit object reference
-sub S_ISGITLINK($) {
+sub S_ISGITLINK {
 	my $mode = shift;
 
 	return (($mode & S_IFMT) == S_IFGITLINK)
@@ -2615,7 +2615,7 @@ sub parsed_difftree_line {
 }
 
 # parse line of git-ls-tree output
-sub parse_ls_tree_line ($;%) {
+sub parse_ls_tree_line {
 	my $line = shift;
 	my %opts = @_;
 	my %res;
@@ -3213,7 +3213,6 @@ sub git_print_header_div {
 	      "\n</div>\n";
 }
 
-#sub git_print_authorship (\%) {
 sub git_print_authorship {
 	my $co = shift;
 
@@ -3269,8 +3268,7 @@ sub git_print_page_path {
 	print "<br/></div>\n";
 }
 
-# sub git_print_log (\@;%) {
-sub git_print_log ($;%) {
+sub git_print_log {
 	my $log = shift;
 	my %opts = @_;
 
-- 
1.6.3

  reply	other threads:[~2009-05-10  0:42 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-05-10  0:03 [PATCH 0/5] gitweb: Some code cleanups (up to perlcritic --stern) Jakub Narebski
2009-05-10  0:05 ` Jakub Narebski [this message]
2009-05-10  9:05   ` [PATCHv2 1/5] gitweb: Remove function prototypes Jakub Narebski
2009-05-10  0:36 ` [PATCH 2/5] gitweb: Do not use bareword filehandles Jakub Narebski
2009-05-10  7:50   ` Petr Baudis
2009-05-10  9:27     ` Jakub Narebski
2009-05-11  1:21   ` [PATCH v2 " Jakub Narebski
2009-05-10  0:38 ` [PATCH 3/5] gitweb: Always use three argument form of open Jakub Narebski
2009-05-11  1:29   ` [PATCH v2 " Jakub Narebski
2009-05-10  0:39 ` [PATCH 4/5] gitweb: Localize magic variable $/ Jakub Narebski
2009-05-10  0:40 ` [PATCH 5/5] gitweb: Use block form of map/grep in a few cases more Jakub Narebski
2009-05-11  0:47 ` [PATCH 0/5] gitweb: Some code cleanups (up to perlcritic --stern) Junio C Hamano
2009-05-11  1:33   ` Jakub Narebski
2009-05-11  4:21     ` Junio C Hamano
2009-05-11  5:13       ` Daniel Pittman
2009-05-11  7:19       ` Jakub Narebski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=200905100205.23733.jnareb@gmail.com \
    --to=jnareb@gmail.com \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.