All of lore.kernel.org
 help / color / mirror / Atom feed
From: Krzesimir Nowak <krzesimir@endocode.com>
To: "Jakub Narębski" <jnareb@gmail.com>
Cc: git <git@vger.kernel.org>, Junio C Hamano <gitster@pobox.com>,
	Eric Sunshine <sunshine@sunshineco.com>
Subject: Re: [PATCH 1/5] gitweb: Add a comment explaining the meaning of $/
Date: Wed, 04 Dec 2013 16:46:17 +0100	[thread overview]
Message-ID: <1386171977.2173.23.camel@localhost.localdomain> (raw)
In-Reply-To: <CANQwDwdiNHFCChVdkKxkEa+HOKzzhgfzy4d6iZB6ejSfva3Z3A@mail.gmail.com>

On Wed, 2013-12-04 at 16:11 +0100, Jakub Narębski wrote:
> On Wed, Dec 4, 2013 at 2:42 PM, Krzesimir Nowak <krzesimir@endocode.com> wrote:
> 
> > So future reader will know what does it mean without running "perldoc
> > perlvar".
> 
> Hmmm... shouldn't future reader know it anyway?  It is not that cryptic.
> I'd say it is idiomatic Perl.
> 

It's plainly obscure. And I think it is not that often used - I keep
forgetting what that pair of punctuation is actually meaning. In this
case I guess it would be more readable to use the following code
instead:
$fh->input_record_separator ("\0");

> Besides, it is not the only place where we set input record separator to NUL,
> to match corresponding option to git command to separate records with NUL
> (the '-z' option in this case).  Others are git_get_path_by_hash(),
> parse_commit(),
> and parse_commits(), git_tree(), not including places where we set $/ to undef
> to slurp whole file: git_get_link_target(), git_blobdiff() for $format
> == 'plain', etc.

Yes, but I stumbled upon that one when trying to understand how config
parsing works. So I added a comment.

> 
> > Signed-off-by: Krzesimir Nowak <krzesimir@endocode.com>
> > ---
> >  gitweb/gitweb.perl | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> > index 68c77f6..ee61f9e 100755
> > --- a/gitweb/gitweb.perl
> > +++ b/gitweb/gitweb.perl
> > @@ -2629,6 +2629,8 @@ sub git_parse_project_config {
> >         my $section_regexp = shift;
> >         my %config;
> >
> > +       # input record separator, so getline does end on null, not
> > +       # newline
> >         local $/ = "\0";
> 
> It is here because of '-z' option below (to account for values with
> embedded newlines).
> 
> >
> >         open my $fh, "-|", git_cmd(), "config", '-z', '-l',
> > --
> > 1.8.3.1
> >
> 
> 
> 

-- 
Krzesimir Nowak
Software Developer
Endocode AG

krzesimir@endocode.com

------
Endocode AG, Johannisstraße 20, 10117 Berlin
info@endocode.com | www.endocode.com

Vorstandsvorsitzender: Mirko Boehm
Vorstände: Dr. Karl Beecher, Chris Kühl, Sebastian Sucker
Aufsichtsratsvorsitzende: Jennifer Beecher

Registergericht: Amtsgericht Charlottenburg - HRB 150748 B

  reply	other threads:[~2013-12-04 15:46 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-12-04 13:42 [PATCH 0/5] Show extra branch refs in gitweb v6 Krzesimir Nowak
2013-12-04 13:42 ` [PATCH 1/5] gitweb: Add a comment explaining the meaning of $/ Krzesimir Nowak
2013-12-04 15:11   ` Jakub Narębski
2013-12-04 15:46     ` Krzesimir Nowak [this message]
2013-12-04 16:19       ` Martin Langhoff
2013-12-04 20:28         ` Junio C Hamano
2013-12-05  9:16           ` Krzesimir Nowak
2013-12-04 17:34       ` Jakub Narębski
2013-12-04 17:37         ` Jakub Narębski
2013-12-04 13:43 ` [PATCH 2/5] gitweb: Move check-ref-format code into separate function Krzesimir Nowak
2013-12-04 15:56   ` Jakub Narębski
2013-12-05  9:19     ` Krzesimir Nowak
2013-12-04 20:31   ` Junio C Hamano
2013-12-05  9:18     ` Krzesimir Nowak
2013-12-04 13:43 ` [PATCH 3/5] gitweb: Return plain booleans in validation methods Krzesimir Nowak
2013-12-04 16:07   ` Jakub Narębski
2013-12-04 18:11     ` Junio C Hamano
2013-12-05  9:23     ` Krzesimir Nowak
2013-12-05 18:16       ` Junio C Hamano
2013-12-05 19:11         ` Jakub Narębski
2013-12-05 20:01           ` Junio C Hamano
2013-12-04 13:43 ` [PATCH 4/5] gitweb: Add a feature for adding more branch refs Krzesimir Nowak
2013-12-04 18:06   ` Jakub Narębski
2013-12-05 10:00     ` Krzesimir Nowak
2013-12-05 11:40       ` Jakub Narębski
2013-12-10 16:04       ` Krzesimir Nowak
2013-12-10 18:54     ` Junio C Hamano
2013-12-10 19:06       ` Jakub Narębski
2013-12-10 19:44         ` Junio C Hamano
2013-12-04 13:43 ` [PATCH 5/5] gitweb: Denote non-heads, non-remotes branches Krzesimir Nowak
2013-12-04 18:54   ` Jakub Narębski
2013-12-04 20:37 ` [PATCH 0/5] Show extra branch refs in gitweb v6 Junio C Hamano

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=1386171977.2173.23.camel@localhost.localdomain \
    --to=krzesimir@endocode.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jnareb@gmail.com \
    --cc=sunshine@sunshineco.com \
    /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.