public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed
From: Junio C Hamano <junkio@cox•net>
To: Jakub Narebski <jnareb@gmail•com>
Cc: git@vger•kernel.org
Subject: Re: [RFD] gitweb: href() function to generate URLs for CGI
Date: Tue, 22 Aug 2006 00:09:31 -0700	[thread overview]
Message-ID: <7vlkphqmac.fsf@assigned-by-dhcp.cox.net> (raw)
In-Reply-To: <eccujr$90h$1@sea.gmane.org> (Jakub Narebski's message of "Mon, 21 Aug 2006 20:38:45 +0200")

Jakub Narebski <jnareb@gmail•com> writes:

> This doesn't work as expected. Map works on _every_ element of list; if
> expression doesn't return anything it just puts undef I think. For example
> while
>
>         print join(";", 
>                 map { 
>                         "a/$_" if ($_ eq lc($_)) 
>                 } ("a", "b", "C", "d")),
>                 "\n";'
>
> returns "a/a;a/b;;a/d" (notice the doubled ';'), the correct way would be to

... say this, than using extra grep {}.

        print join(";", 
                map { 
                        ($_ eq lc($_)) ? "a/$_" : ()
                } ("a", "b", "C", "d")),
                "\n";

> What about my proposed solution? Don't sort, use sorted keys instead, i.e.
> something like (after unmangling whitespace)
>
> sub href(%) {
>         my @mapping = ( project => "p",
>                         action => "a",
>                         hash => "h",
>                         hash_parent => "hp",
>                         hash_base => "hb",
>                         file_name => "f",
>                         file_parent => "fp",
>                         page => "pg",
>                         searchtext => "s",
>                         );
>         my %mapping;
>         my @mapping_keys;                        
>         for (my $i = 0; $i < @mapping; $i += 2) {
>                 my ($k, $v) = ($mapping[$i], $mapping[$i+1]);
>                 $mapping{$k} = $v;
>                 push @mapping_keys, $k;
>         }

Now this loop got an expensive way to say %mapping = @mapping
plus assigning @mapping_keys, so I would do only the push part
in the loop for readability while changing the name of the
variable a bit more meaningful.

	my %mapping = @mapping;
        my @valid_params;
	for (my $i = 0; $i < @mapping; $i += 2) {
	        push @valid_params, $mapping[$i];
	}

But aside from that, I very much prefer your version that loops
over what _we_ define (i.e. @valid_params), rather than what the
user happens to throw at us (i.e. keys %params).

  reply	other threads:[~2006-08-22  7:09 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-08-21 15:39 [RFD] gitweb: href() function to generate URLs for CGI Jakub Narebski
2006-08-21 15:52 ` Jakub Narebski
2006-08-21 18:22 ` Junio C Hamano
2006-08-21 18:38   ` Jakub Narebski
2006-08-22  7:09     ` Junio C Hamano [this message]
2006-08-22  8:18       ` Jakub Narebski
2006-08-22  8:55         ` Junio C Hamano
2006-08-22  9:34           ` Jakub Narebski
2006-08-22 10:47           ` Jakub Narebski
2006-08-22 22:26             ` Junio C Hamano
2006-08-22 22:39               ` Jakub Narebski
2006-08-22 18:35           ` Randal L. Schwartz
2006-08-22 19:55             ` Jakub Narebski
2006-08-22 20:01               ` Jakub Narebski
2006-08-22 22:21                 ` Junio C Hamano
2006-08-22 17:05 ` [PATCH 1/2] gitweb: Drop the href() params which keys are not in %mapping Jakub Narebski
2006-08-22 17:05 ` [PATCH 2/2] gitweb: Sort CGI parameters returned by href() Jakub Narebski
  -- strict thread matches above, loose matches on Subject: below --
2006-08-21 15:21 [RFD] gitweb: href() function to generate URLs for CGI 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=7vlkphqmac.fsf@assigned-by-dhcp.cox.net \
    --to=junkio@cox$(echo .)net \
    --cc=git@vger$(echo .)kernel.org \
    --cc=jnareb@gmail$(echo .)com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox