Re: [PATCH 1/5] gitweb: Add a comment explaining the meaning of $/
On Wed, 2013-12-04 at 12:28 -0800, Junio C Hamano wrote: Martin Langhoff martin.langh...@gmail.com writes: On Wed, Dec 4, 2013 at 10:46 AM, Krzesimir Nowak krzesi...@endocode.com wrote: On Wed, 2013-12-04 at 16:11 +0100, Jakub Narębski wrote: On Wed, Dec 4, 2013 at 2:42 PM, Krzesimir Nowak krzesi...@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 - It's classic Perl. Perhaps you'd want to use English; and call it $INPUT_RECORD_SEPARATOR in a patch titled Make things readable to non-Perl natives. Hmm, but do we want to see use English there in the first place? Nevermind, I'm going to drop that patch. -- Krzesimir Nowak Software Developer Endocode AG krzesi...@endocode.com -- Endocode AG, Johannisstraße 20, 10117 Berlin i...@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 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/5] gitweb: Add a comment explaining the meaning of $/
So future reader will know what does it mean without running perldoc
perlvar.
Signed-off-by: Krzesimir Nowak krzesi...@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;
open my $fh, -|, git_cmd(), config, '-z', '-l',
--
1.8.3.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/5] gitweb: Add a comment explaining the meaning of $/
On Wed, 2013-12-04 at 16:11 +0100, Jakub Narębski wrote:
On Wed, Dec 4, 2013 at 2:42 PM, Krzesimir Nowak krzesi...@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 krzesi...@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
krzesi...@endocode.com
--
Endocode AG, Johannisstraße 20, 10117 Berlin
i...@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
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/5] gitweb: Add a comment explaining the meaning of $/
On Wed, Dec 4, 2013 at 4:46 PM, Krzesimir Nowak krzesi...@endocode.com wrote: On Wed, 2013-12-04 at 16:11 +0100, Jakub Narębski wrote: On Wed, Dec 4, 2013 at 2:42 PM, Krzesimir Nowak krzesi...@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. I think it depends on what kind of Perl code one is used to. It is not as obscure as $; and similar to $|, I think. In this case I guess it would be more readable to use the following code instead: $fh-input_record_separator (\0); That would be a good change to replace local $/ = \0; open my $fh, -|, git_cmd(), ..., '-z', ... with open my $fh, -|, git_cmd(), ..., '-z', ... $fh-input_record_separator (\0); (not forgetting about use IO::Handle, which module is core Perl module); Anyway, this change (or proposed change) does not, IMHO, belong in this series. -- Jakub Narebski -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/5] gitweb: Add a comment explaining the meaning of $/
On Wed, Dec 4, 2013 at 6:34 PM, Jakub Narębski jna...@gmail.com wrote: On Wed, Dec 4, 2013 at 4:46 PM, Krzesimir Nowak krzesi...@endocode.com wrote: On Wed, 2013-12-04 at 16:11 +0100, Jakub Narębski wrote: On Wed, Dec 4, 2013 at 2:42 PM, Krzesimir Nowak krzesi...@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. I think it depends on what kind of Perl code one is used to. It is not as obscure as $; and similar to $|, I think. In this case I guess it would be more readable to use the following code instead: $fh-input_record_separator (\0); That would be a good change to replace local $/ = \0; open my $fh, -|, git_cmd(), ..., '-z', ... with open my $fh, -|, git_cmd(), ..., '-z', ... $fh-input_record_separator (\0); (not forgetting about use IO::Handle, which module is core Perl module); Actually it is replacing local $/ = \0; with IO::Handle-input_record_separator(\0); see http://p3rl.org/IO::Handle -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/5] gitweb: Add a comment explaining the meaning of $/
Martin Langhoff martin.langh...@gmail.com writes: On Wed, Dec 4, 2013 at 10:46 AM, Krzesimir Nowak krzesi...@endocode.com wrote: On Wed, 2013-12-04 at 16:11 +0100, Jakub Narębski wrote: On Wed, Dec 4, 2013 at 2:42 PM, Krzesimir Nowak krzesi...@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 - It's classic Perl. Perhaps you'd want to use English; and call it $INPUT_RECORD_SEPARATOR in a patch titled Make things readable to non-Perl natives. Hmm, but do we want to see use English there in the first place? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
