Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PATCH] Fix Dumper.pm podcheck error #14871

Closed
p5pRT opened this issue Aug 25, 2015 · 10 comments
Closed

[PATCH] Fix Dumper.pm podcheck error #14871

p5pRT opened this issue Aug 25, 2015 · 10 comments
Labels

Comments

@p5pRT
Copy link

p5pRT commented Aug 25, 2015

Migrated from rt.perl.org#125895 (status was 'rejected')

Searchable as RT125895$

@p5pRT
Copy link
Author

p5pRT commented Aug 25, 2015

From @paulohrpinheiro

This is a bug report for perl from paulohrpinheiro@​gmail.com,
generated with the help of perlbug 1.40 running under perl 5.23.3.


Fixed error in test for Dumper.pm. Apparently the test script to verify the
string "See C<"XXX​::XXX" assumes that it is a link. Escaping the second "​:"
with E<58> (code for "​:") makes this rule does not apply.



Flags​:
  category=library
  severity=wishlist
  Type=Patch
  PatchStatus=HasPatch
  module=Data​::Dumper


Site configuration information for perl 5.23.3​:

Configured by phrp at Mon Aug 24 23​:24​:08 BRT 2015.

Summary of my perl5 (revision 5 version 23 subversion 3) configuration​:
  Derived from​: 6a475a4
  Platform​:
  osname=linux, osvers=3.19.0-26-generic, archname=x86_64-linux
  uname='linux paulaum 3.19.0-26-generic #28-ubuntu smp tue aug 11
14​:16​:32 utc 2015 x86_64 x86_64 x86_64 gnulinux '
  config_args='-des -Dusedevel'
  hint=recommended, useposix=true, d_sigaction=define
  useithreads=undef, usemultiplicity=undef
  use64bitint=define, use64bitall=define, uselongdouble=undef
  usemymalloc=n, bincompat5005=undef
  Compiler​:
  cc='cc', ccflags ='-fwrapv -fno-strict-aliasing -pipe
-fstack-protector-strong -I/usr/local/include -D_LARGEFILE_SOURCE
-D_FILE_OFFSET_BITS=64',
  optimize='-O2',
  cppflags='-fwrapv -fno-strict-aliasing -pipe
-fstack-protector-strong -I/usr/local/include'
  ccversion='', gccversion='4.9.2', gccosandvers=''
  intsize=4, longsize=8, ptrsize=8, doublesize=8, byteorder=12345678,
doublekind=3
  d_longlong=define, longlongsize=8, d_longdbl=define,
longdblsize=16, longdblkind=3
  ivtype='long', ivsize=8, nvtype='double', nvsize=8, Off_t='off_t',
lseeksize=8
  alignbytes=8, prototype=define
  Linker and Libraries​:
  ld='cc', ldflags =' -fstack-protector-strong -L/usr/local/lib'
  libpth=/usr/local/lib
/usr/lib/gcc/x86_64-linux-gnu/4.9/include-fixed
/usr/include/x86_64-linux-gnu /usr/lib /lib/x86_64-linux-gnu /lib/../lib
/usr/lib/x86_64-linux-gnu /usr/lib/../lib /lib
  libs=-lpthread -lnsl -ldl -lm -lcrypt -lutil -lc
  perllibs=-lpthread -lnsl -ldl -lm -lcrypt -lutil -lc
  libc=libc-2.21.so, so=so, useshrplib=false, libperl=libperl.a
  gnulibc_version='2.21'
  Dynamic Linking​:
  dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E'
  cccdlflags='-fPIC', lddlflags='-shared -O2 -L/usr/local/lib
-fstack-protector-strong'

Locally applied patches​:
  uncommitted-changes


@​INC for perl 5.23.3​:
  lib
  /home/phrp/perl5/lib/perl5
  /usr/local/lib/perl5/site_perl/5.23.3/x86_64-linux
  /usr/local/lib/perl5/site_perl/5.23.3
  /usr/local/lib/perl5/5.23.3/x86_64-linux
  /usr/local/lib/perl5/5.23.3
  .


Environment for perl 5.23.3​:
  HOME=/home/phrp
  LANG=pt_BR.UTF-8
  LANGUAGE=pt_BR​:pt​:en
  LD_LIBRARY_PATH (unset)
  LOGDIR (unset)
PATH=/home/phrp/perl5/bin​:/usr/local/sbin​:/usr/local/bin​:/usr/sbin​:/usr/bin​:/sbin​:/bin​:/usr/games​:/usr/local/games
  PERL5LIB=/home/phrp/perl5/lib/perl5
  PERL_BADLANG (unset)
  PERL_LOCAL_LIB_ROOT=/home/phrp/perl5
  PERL_MB_OPT=--install_base "/home/phrp/perl5"
  PERL_MM_OPT=INSTALL_BASE=/home/phrp/perl5
  PERL_POD_PEDANTIC=1
  SHELL=/bin/bash

@p5pRT
Copy link
Author

p5pRT commented Aug 25, 2015

From @paulohrpinheiro

0001-Fixed-pod-trick-in-Dumper.pm.patch
From b8a66286e51ac68439aa2691c55a87f5e7ecd411 Mon Sep 17 00:00:00 2001
From: Paulo Pinheiro <paulohrpinheiro@gmail.com>
Date: Tue, 25 Aug 2015 00:03:40 -0300
Subject: [PATCH] Fixed pod trick in Dumper.pm

---
 dist/Data-Dumper/Dumper.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/dist/Data-Dumper/Dumper.pm b/dist/Data-Dumper/Dumper.pm
index e884298..0d1108f 100644
--- a/dist/Data-Dumper/Dumper.pm
+++ b/dist/Data-Dumper/Dumper.pm
@@ -889,7 +889,7 @@ C<$VAR>I<n> (where I<n> is a numeric suffix), and other duplicate references
 to substructures within C<$VAR>I<n> will be appropriately labeled using arrow
 notation.  You can specify names for individual values to be dumped if you
 use the C<Dump()> method, or you can change the default C<$VAR> prefix to
-something else.  See C<$Data::Dumper::Varname> and C<$Data::Dumper::Terse>
+something else.  See C<$DataE<58>:Dumper::Varname> and C<$Data::Dumper::Terse>
 below.
 
 The default output of self-referential structures can be C<eval>ed, but the
-- 
2.1.4

@p5pRT
Copy link
Author

p5pRT commented Aug 26, 2015

From @tonycoz

On Mon Aug 24 20​:30​:47 2015, paulohrpinheiro@​gmail.com wrote​:

Fixed error in test for Dumper.pm. Apparently the test script to
verify the
string "See C<"XXX​::XXX" assumes that it is a link. Escaping the
second "​:"
with E<58> (code for "​:") makes this rule does not apply.

Which test is that?

I just tested Data​::Dumper from CPAN, which passed, and Data​::Dumper in blead is tested several times daily (including pod checks), and it passes.

Tony

@p5pRT
Copy link
Author

p5pRT commented Aug 26, 2015

The RT System itself - Status changed from 'new' to 'open'

@p5pRT
Copy link
Author

p5pRT commented Aug 26, 2015

From @paulohrpinheiro

Hi!

Em Ter Ago 25 17​:07​:30 2015, tonyc escreveu​:

(...)
Which test is that?

I just tested Data​::Dumper from CPAN, which passed, and Data​::Dumper
in blead is tested several times daily (including pod checks), and it
passes.

Tony

Hi!

In file `t/porting/known_pod_issues.dat` of source tree there are a list of silenced tests. I read it on this page​:

  http​://perl5.git.perl.org/perl.git/blob/HEAD​:/Porting/todo.pod

(section `Fix POD errors in Perl documentation`)

And chose one to present my first contribution.

Paulo

@p5pRT
Copy link
Author

p5pRT commented Aug 26, 2015

From @paulohrpinheiro

Hi!

In file `t/porting/known_pod_issues.dat`of source tree there area list
of the silenced tests. I read it on this page​:

http​://perl5.git.perl.org/perl.git/blob/HEAD​:/Porting/todo.pod

(section `Fix POD errors in Perl documentation`)

And chose one to present my first contribution.

Konsole output

Paulo Henrique Rodrigues Pinheiro
paulohrpinheiro@​gmail.com
http​://www.sysinclout.it

On 25/08/2015 21​:07, Tony Cook via RT wrote​:

On Mon Aug 24 20​:30​:47 2015, paulohrpinheiro@​gmail.com wrote​:

Fixed error in test for Dumper.pm. Apparently the test script to
verify the
string "See C<"XXX​::XXX" assumes that it is a link. Escaping the
second "​:"
with E<58> (code for "​:") makes this rule does not apply.
Which test is that?

I just tested Data​::Dumper from CPAN, which passed, and Data​::Dumper in blead is tested several times daily (including pod checks), and it passes.

Tony

@p5pRT
Copy link
Author

p5pRT commented Aug 27, 2015

From @khwilliamson

On 08/25/2015 06​:44 PM, Paulo Henrique Rodrigues Pinheiro wrote​:

Hi!

In file `t/porting/known_pod_issues.dat`of source tree there area list
of the silenced tests. I read it on this page​:

http​://perl5.git.perl.org/perl.git/blob/HEAD​:/Porting/todo.pod

(section `Fix POD errors in Perl documentation`)

And chose one to present my first contribution.

Thank you for wanting to help out, and I hope to see further
contributions from you.

It turns out that the problem you fixed in Dumper.pm is actually better
fixed in podcheck.t, as that test file, which I maintain (mostly its me,
anyway), could easily test for something like this condition and not
raise a warning.

The change of a colon to an escape sequence would cause the pod to be
less readable than it is now, and really should be avoided. (Most
people would have to look up what 58 means.) It's better to fix the
test or have an exception in the podcheck database than it is to make
the pod itself less readable.

I'll put fixing podcheck.t for this problem on my list of things to do.
  I had not considered this particular issue before, so thank you for
bringing it up.

If you'd like to make some more contributions to pod errors, please
email me off-list, and I can help direct you. I have a bunch of fixes
in the queue waiting for me to get time to apply them, and it would be a
pity if you worked on something that has already been fixed, but not yet
made public.

Thanks again.

Karl Williamson

Konsole output

Paulo Henrique Rodrigues Pinheiro
paulohrpinheiro@​gmail.com
http​://www.sysinclout.it

On 25/08/2015 21​:07, Tony Cook via RT wrote​:

On Mon Aug 24 20​:30​:47 2015,paulohrpinheiro@​gmail.com wrote​:

Fixed error in test for Dumper.pm. Apparently the test script to
verify the
string "See C<"XXX​::XXX" assumes that it is a link. Escaping the
second "​:"
with E<58> (code for "​:") makes this rule does not apply.
Which test is that?

I just tested Data​::Dumper from CPAN, which passed, and Data​::Dumper in blead is tested several times daily (including pod checks), and it passes.

Tony

@p5pRT
Copy link
Author

p5pRT commented Aug 27, 2015

From @paulohrpinheiro

I'm happy to have helped in some way, and I also think that what you
proposed is more accurate and appropriate. Thank you all.

Paulo Henrique Rodrigues Pinheiro
paulohrpinheiro@​gmail.com
http​://www.sysinclout.it

On 27/08/2015 02​:32, Karl Williamson wrote​:

On 08/25/2015 06​:44 PM, Paulo Henrique Rodrigues Pinheiro wrote​:

Hi!

In file `t/porting/known_pod_issues.dat`of source tree there area list
of the silenced tests. I read it on this page​:

http​://perl5.git.perl.org/perl.git/blob/HEAD​:/Porting/todo.pod

(section `Fix POD errors in Perl documentation`)

And chose one to present my first contribution.

Thank you for wanting to help out, and I hope to see further
contributions from you.

It turns out that the problem you fixed in Dumper.pm is actually
better fixed in podcheck.t, as that test file, which I maintain
(mostly its me, anyway), could easily test for something like this
condition and not raise a warning.

The change of a colon to an escape sequence would cause the pod to be
less readable than it is now, and really should be avoided. (Most
people would have to look up what 58 means.) It's better to fix the
test or have an exception in the podcheck database than it is to make
the pod itself less readable.

I'll put fixing podcheck.t for this problem on my list of things to
do. I had not considered this particular issue before, so thank you
for bringing it up.

If you'd like to make some more contributions to pod errors, please
email me off-list, and I can help direct you. I have a bunch of fixes
in the queue waiting for me to get time to apply them, and it would be
a pity if you worked on something that has already been fixed, but not
yet made public.

Thanks again.

Karl Williamson

Konsole output

Paulo Henrique Rodrigues Pinheiro
paulohrpinheiro@​gmail.com
http​://www.sysinclout.it

On 25/08/2015 21​:07, Tony Cook via RT wrote​:

On Mon Aug 24 20​:30​:47 2015,paulohrpinheiro@​gmail.com wrote​:

Fixed error in test for Dumper.pm. Apparently the test script to
verify the
string "See C<"XXX​::XXX" assumes that it is a link. Escaping the
second "​:"
with E<58> (code for "​:") makes this rule does not apply.
Which test is that?

I just tested Data​::Dumper from CPAN, which passed, and Data​::Dumper
in blead is tested several times daily (including pod checks), and
it passes.

Tony

@p5pRT
Copy link
Author

p5pRT commented Sep 1, 2015

From @tonycoz

On Thu Aug 27 05​:18​:03 2015, paulohrpinheiro@​gmail.com wrote​:

I'm happy to have helped in some way, and I also think that what you
proposed is more accurate and appropriate. Thank you all.

Thanks for trying to contribute, as discussed this needs to be fixed in the pod check code, so this patch is inappropriate.

Closing this ticket.

Tony

@p5pRT
Copy link
Author

p5pRT commented Sep 1, 2015

@tonycoz - Status changed from 'open' to 'rejected'

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant