Skip Menu |
Report information
Id: 120174
Status: resolved
Priority: 0/
Queue: perl5

Owner: Nobody
Requestors: smylers [at] stripey.com
Cc:
AdminCc:

Operating System: Linux
PatchStatus: (no value)
Severity: low
Type: core
Perl Version: 5.19.5
Fixed In: (no value)

Attachments
0001-Add-tests-for-p-and-x-commands-without-subsequent-wh.patch
0001-RT-120174-regression-in-single-letter-debugger-comma.patch
0002-perl-120174-Debugger-cmds-not-requiring-spaces.patch
120174-0001-Add-tests-for-p-and-x-commands-without-subsequent-wh.patch
120174.68cd360812.1799399c45.perl5db.pl.diff.gz



Subject: Debugger command regression: Now requires more spaces
Date: Thu, 10 Oct 2013 12:13:51 +0100 (BST)
To: perlbug [...] perl.org
From: smylers [...] stripey.com
Download (untitled) / with headers
text/plain 3.9k
This is a bug report for perl from smylers@stripey.com, generated with the help of perlbug 1.39 running under perl 5.19.5. ----------------------------------------------------------------- In the Perl debugger commands written like the following used to work, but now throw errors: p@ARGV x@ARGV x\@ARGV x\%INC They still work if you type a space between the command letter and the punctuation character which starts its argument, but they used to work without the space too. It only seems to be the single-letter debugger commands that are affected. print@ARGV and so on still works without spaces. The error message in blead is: DB<72> x\@ARGV Backslash found where operator expected at (eval 8)[lib/perl5db.pl:732] line 2, near "x\" at (eval 8)[lib/perl5db.pl:732] line 2. eval 'no strict; ($@, $!, $^E, $,, $/, $\\, $^W) = @DB::saved;package main; $^D = $^D | $DB::db_stop; x\\@ARGV; ' called at lib/perl5db.pl line 732 DB::eval called at lib/perl5db.pl line 3091 DB::DB called at -e line 1 syntax error at (eval 8)[lib/perl5db.pl:732] line 2, near "x\" This is it working in v5.14.3: DB<72> x\@ARGV 0 ARRAY(0x2267d18) empty array I haven't tried to narrow it down any further, mainly cos I don't know how to come up with a non-interactive way of demonstrating the change. ----------------------------------------------------------------- --- Flags: category=core severity=low --- Site configuration information for perl 5.19.5: Configured by smylers at Wed Oct 9 17:34:57 BST 2013. Summary of my perl5 (revision 5 version 19 subversion 5) configuration: Commit id: 01582e5ce2e8b00ce08a55eb4a588e811e479912 Platform: osname=linux, osvers=3.8.0-32-generic, archname=x86_64-linux uname='linux fozzie 3.8.0-32-generic #47-ubuntu smp tue oct 1 22:35:23 utc 2013 x86_64 x86_64 x86_64 gnulinux ' config_args='-des -Dusedevel' hint=recommended, useposix=true, d_sigaction=define useithreads=undef, usemultiplicity=undef useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef use64bitint=define, use64bitall=define, uselongdouble=undef usemymalloc=n, bincompat5005=undef Compiler: cc='cc', ccflags ='-fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64', optimize='-O2', cppflags='-fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include' ccversion='', gccversion='4.7.3', gccosandvers='' intsize=4, longsize=8, ptrsize=8, doublesize=8, byteorder=12345678 d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=16 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 -L/usr/local/lib' libpth=/usr/local/lib /lib/x86_64-linux-gnu /lib/../lib /usr/lib/x86_64-linux-gnu /usr/lib/../lib /lib /usr/lib libs=-lnsl -ldl -lm -lcrypt -lutil -lc perllibs=-lnsl -ldl -lm -lcrypt -lutil -lc libc=, so=so, useshrplib=false, libperl=libperl.a gnulibc_version='2.17' 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' --- @INC for perl 5.19.5: lib /home/smylers/lib/perl5/site_perl /home/smylers/lib/perl5 /usr/local/lib/perl5/site_perl/5.19.5/x86_64-linux /usr/local/lib/perl5/site_perl/5.19.5 /usr/local/lib/perl5/5.19.5/x86_64-linux /usr/local/lib/perl5/5.19.5 . --- Environment for perl 5.19.5: HOME=/home/smylers LANG=en_GB.utf8 LANGUAGE=en_GB:en LC_COLLATE=C LD_LIBRARY_PATH (unset) LOGDIR (unset) PATH=/home/smylers/bin:/usr/local/sbin:/usr/local/bin:/sbin:/bin:/usr/sbin:/usr/bin:/usr/X11R6/bin:/usr/games PERL5LIB=/home/smylers/lib/perl5/site_perl:/home/smylers/lib/perl5 PERL_BADLANG (unset) PERL_CPANM_OPT=--sudo --prompt SHELL=/bin/bash
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 3.2k
On Thu, 10 Oct 2013 11:14:24 GMT, smylers@stripey.com wrote: Show quoted text
> > This is a bug report for perl from smylers@stripey.com, > generated with the help of perlbug 1.39 running under perl 5.19.5. > > > ----------------------------------------------------------------- > > In the Perl debugger commands written like the following used to work, > but now throw errors: > > p@ARGV > x@ARGV > x\@ARGV > x\%INC > > They still work if you type a space between the command letter and the > punctuation character which starts its argument, but they used to work > without the space too. > > It only seems to be the single-letter debugger commands that are > affected. print@ARGV and so on still works without spaces. > > The error message in blead is: > > DB<72> x\@ARGV > Backslash found where operator expected at (eval > 8)[lib/perl5db.pl:732] line 2, near "x\" > at (eval 8)[lib/perl5db.pl:732] line 2. > eval 'no strict; ($@, $!, $^E, $,, $/, $\\, $^W) = @DB::saved;package > main; $^D = $^D | $DB::db_stop; > x\\@ARGV; > ' called at lib/perl5db.pl line 732 > DB::eval called at lib/perl5db.pl line 3091 > DB::DB called at -e line 1 > syntax error at (eval 8)[lib/perl5db.pl:732] line 2, near "x\" > > This is it working in v5.14.3: > > DB<72> x\@ARGV > 0 ARRAY(0x2267d18) > empty array > > I haven't tried to narrow it down any further, mainly cos I don't know > how to come up with a non-interactive way of demonstrating the change. >
I have confirmed the regression. It is still present in blead (5.27.3). However, we should first ask: Is this a bug? That is, was the fact that we once could use the 'p' and 'x' commands in the debugger without a subsequent wordspace intentional or accidental? Note: I'm not saying this should not be fixed. I simply think the question should be faced. Use of perlbrew to get various versions of perl shows roughly when the regression occurred: ##### good perlbrew use perl-5.16.3 Loading DB routines from perl5db.pl version 1.37 bad perlbrew use perl-5.18.4 Loading DB routines from perl5db.pl version 1.39_11 ##### Which means that the problem occurred roughly between these two commits: ##### commit 68cd360812f9eaa2d34c45c501e2fef87c44ccde Author: Jesse Vincent <jesse@bestpractical.com> Date: Tue Apr 24 19:02:34 2012 -0400 -$VERSION = '1.36'; +$VERSION = '1.37'; commit 1799399c45baa7e5af45db33567d2a2de629d5e0 Author: Ricardo Signes <rjbs@cpan.org> AuthorDate: Fri Jun 7 11:51:45 2013 -0400 Commit: Ricardo Signes <rjbs@cpan.org> CommitDate: Fri Jun 7 11:51:45 2013 -0400 version bumps and perldelta for debugger depth -$VERSION = '1.39_10'; +$VERSION = '1.40'; ##### There was a lot of work being done on the debugger during this period. The diff is large enough that I'm attaching it gzipped first. I don't know how to bisect a debugger problem. But I was able to create a branch based on perl-5.16.3 (jkeenan/120174-starting-from-5.16.3) in which I wrote some tests for the problem with passed. Those same tests, when added to a branch based on blead (5.27.3) (jkeenan/120174-starting-from-5.27.3), fail. Hence, the tests -- which, I concede, are rough, as I'm not familiar with the tests in lib/perl5db.t -- can be used as regression tests once the problem is corrected (assuming we want to correct it). Thank you very much. -- James E Keenan (jkeenan@cpan.org)
Subject: 120174-0001-Add-tests-for-p-and-x-commands-without-subsequent-wh.patch
From 9cbf7940aed97cb73313c28dc2ba2fd85cc19458 Mon Sep 17 00:00:00 2001 From: James E Keenan <jkeenan@cpan.org> Date: Sat, 2 Sep 2017 22:28:20 -0400 Subject: [PATCH] Add tests for 'p' and 'x' commands without subsequent whitespace. Tests pass on perl-5.16.3 but should fail (until source code is corrected) on subsequent versions. For: RT #120174 --- lib/perl5db.t | 86 ++++++++++++++++++++++++++++++++++++++++++++++++- lib/perl5db/t/rt-120174 | 4 +++ 2 files changed, 89 insertions(+), 1 deletion(-) create mode 100644 lib/perl5db/t/rt-120174 diff --git a/lib/perl5db.t b/lib/perl5db.t index a2dccc6..3d432ad 100644 --- a/lib/perl5db.t +++ b/lib/perl5db.t @@ -31,7 +31,7 @@ BEGIN { $ENV{PERL_RL} = 'Perl'; # Suppress system Term::ReadLine::Gnu } -plan(123); +plan(127); my $rc_filename = '.perldb'; @@ -2817,6 +2817,90 @@ SKIP: ); } +{ + # perl 5 RT #120174 - 'p' command + my $wrapper = DebugWrap->new( + { + cmds => + [ + 'b 2', + 'c', + 'p@abc', + 'q', + ], + prog => '../lib/perl5db/t/rt-120174', + } + ); + + $wrapper->contents_like( + qr/1234/, + q/RT 120174: p command can be invoked without space after 'p'/, + ); +} + +{ + # perl 5 RT #120174 - 'x' command on array + my $wrapper = DebugWrap->new( + { + cmds => + [ + 'b 2', + 'c', + 'x@abc', + 'q', + ], + prog => '../lib/perl5db/t/rt-120174', + } + ); + + $wrapper->contents_like( + qr/0\s+1\n1\s+2\n2\s+3\n3\s+4/ms, + q/RT 120174: x command can be invoked without space after 'x' before array/, + ); +} + +{ + # perl 5 RT #120174 - 'x' command on array ref + my $wrapper = DebugWrap->new( + { + cmds => + [ + 'b 2', + 'c', + 'x\@abc', + 'q', + ], + prog => '../lib/perl5db/t/rt-120174', + } + ); + + $wrapper->contents_like( + qr/\s+0\s+1\n\s+1\s+2\n\s+2\s+3\n\s+3\s+4/ms, + q/RT 120174: x command can be invoked without space after 'x' before array ref/, + ); +} + +{ + # perl 5 RT #120174 - 'x' command on hash ref + my $wrapper = DebugWrap->new( + { + cmds => + [ + 'b 4', + 'c', + 'x\%xyz', + 'q', + ], + prog => '../lib/perl5db/t/rt-120174', + } + ); + + $wrapper->contents_like( + qr/\s+'alpha'\s+=>\s+'beta'\n\s+'gamma'\s+=>\s+'delta'/ms, + q/RT 120174: x command can be invoked without space after 'x' before hash ref/, + ); +} + END { 1 while unlink ($rc_filename, $out_fn); } diff --git a/lib/perl5db/t/rt-120174 b/lib/perl5db/t/rt-120174 new file mode 100644 index 0000000..c79c851 --- /dev/null +++ b/lib/perl5db/t/rt-120174 @@ -0,0 +1,4 @@ +@abc = (1..4); +print "hello world\n"; +%xyz = ( 'alpha' => 'beta', 'gamma' => 'delta' ); +print "goodbye world\n"; -- 2.7.4
RT-Send-CC: perl5-porters [...] perl.org
On Sun, 03 Sep 2017 03:02:20 GMT, jkeenan wrote: Add diff. -- James E Keenan (jkeenan@cpan.org)
Subject: 120174.68cd360812.1799399c45.perl5db.pl.diff.gz

Message body not shown because it is not plain text.

RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 10.5k
On Sun, 03 Sep 2017 03:02:20 GMT, jkeenan wrote: Show quoted text
> On Thu, 10 Oct 2013 11:14:24 GMT, smylers@stripey.com wrote:
> > > > This is a bug report for perl from smylers@stripey.com, > > generated with the help of perlbug 1.39 running under perl 5.19.5. > > > > > > ----------------------------------------------------------------- > > > > In the Perl debugger commands written like the following used to > > work, > > but now throw errors: > > > > p@ARGV > > x@ARGV > > x\@ARGV > > x\%INC > > > > They still work if you type a space between the command letter and > > the > > punctuation character which starts its argument, but they used to > > work > > without the space too. > > > > It only seems to be the single-letter debugger commands that are > > affected. print@ARGV and so on still works without spaces. > > > > The error message in blead is: > > > > DB<72> x\@ARGV > > Backslash found where operator expected at (eval > > 8)[lib/perl5db.pl:732] line 2, near "x\" > > at (eval 8)[lib/perl5db.pl:732] line 2. > > eval 'no strict; ($@, $!, $^E, $,, $/, $\\, $^W) = @DB::saved;package > > main; $^D = $^D | $DB::db_stop; > > x\\@ARGV; > > ' called at lib/perl5db.pl line 732 > > DB::eval called at lib/perl5db.pl line 3091 > > DB::DB called at -e line 1 > > syntax error at (eval 8)[lib/perl5db.pl:732] line 2, near "x\" > > > > This is it working in v5.14.3: > > > > DB<72> x\@ARGV > > 0 ARRAY(0x2267d18) > > empty array > > > > I haven't tried to narrow it down any further, mainly cos I don't > > know > > how to come up with a non-interactive way of demonstrating the > > change. > >
> > I have confirmed the regression. It is still present in blead > (5.27.3). > > However, we should first ask: Is this a bug? That is, was the fact > that we once could use the 'p' and 'x' commands in the debugger > without a subsequent wordspace intentional or accidental? > > Note: I'm not saying this should not be fixed. I simply think the > question should be faced. > > Use of perlbrew to get various versions of perl shows roughly when the > regression occurred: > > ##### > good perlbrew use perl-5.16.3 Loading DB routines from perl5db.pl > version 1.37 > bad perlbrew use perl-5.18.4 Loading DB routines from perl5db.pl > version 1.39_11 > ##### > > Which means that the problem occurred roughly between these two > commits: > > ##### > commit 68cd360812f9eaa2d34c45c501e2fef87c44ccde > Author: Jesse Vincent <jesse@bestpractical.com> > Date: Tue Apr 24 19:02:34 2012 -0400 > -$VERSION = '1.36'; > +$VERSION = '1.37'; > > commit 1799399c45baa7e5af45db33567d2a2de629d5e0 > Author: Ricardo Signes <rjbs@cpan.org> > AuthorDate: Fri Jun 7 11:51:45 2013 -0400 > Commit: Ricardo Signes <rjbs@cpan.org> > CommitDate: Fri Jun 7 11:51:45 2013 -0400 > > version bumps and perldelta for debugger depth > -$VERSION = '1.39_10'; > +$VERSION = '1.40'; > ##### > > There was a lot of work being done on the debugger during this period. > The diff is large enough that I'm attaching it gzipped first. > > I don't know how to bisect a debugger problem. But I was able to > create a branch based on perl-5.16.3 (jkeenan/120174-starting-from- > 5.16.3) in which I wrote some tests for the problem with passed. > Those same tests, when added to a branch based on blead (5.27.3) > (jkeenan/120174-starting-from-5.27.3), fail. Hence, the tests -- > which, I concede, are rough, as I'm not familiar with the tests in > lib/perl5db.t -- can be used as regression tests once the problem is > corrected (assuming we want to correct it). >
I have figured out how to bisect a debugger problem. The problems with the 'x' command first appeared in this commit: ##### $ git show --format=fuller d478d7a00517925a72f7077951fbed1283fb6a07 |cat commit d478d7a00517925a72f7077951fbed1283fb6a07 Author: Shlomi Fish <shlomif@shlomifish.org> AuthorDate: Sun Oct 14 12:56:53 2012 +0200 Commit: Ricardo Signes <rjbs@cpan.org> CommitDate: Mon Nov 12 09:18:40 2012 -0500 Add more commands to the lookup table. diff --git a/lib/perl5db.pl b/lib/perl5db.pl index c9844fd..ed34703 100644 --- a/lib/perl5db.pl +++ b/lib/perl5db.pl @@ -2423,7 +2423,15 @@ sub _DB__at_end_of_every_command { # 's' is subroutine. my %cmd_lookup = ( - 'q' => { t => 'm', v => '_handle_q_command' }, + '.' => { t => 's', v => \&_DB__handle_dot_command, }, + 'f' => { t => 's', v => \&_DB__handle_f_command, }, + 'm' => { t => 's', v => \&_DB__handle_m_command, }, + 'q' => { t => 'm', v => '_handle_q_command', }, + 'S' => { t => 'm', v => '_handle_S_command', }, + 't' => { t => 'm', v => '_handle_t_command', }, + 'x' => { t => 'm', v => '_handle_x_command', }, + (map { $_ => { t => 'm', v => '_handle_V_command_and_X_command', }, } + ('X', 'V')), ); sub DB { @@ -2763,16 +2771,12 @@ If level is specified, set C<$trace_to_depth>. =cut - $obj->_handle_t_command; - =head4 C<S> - list subroutines matching/not matching a pattern Walks through C<%sub>, checking to see whether or not to print the name. =cut - $obj->_handle_S_command; - =head4 C<X> - list variables in current package Since the C<V> command actually processes this, just change this to the @@ -2784,8 +2788,6 @@ Uses C<dumpvar.pl> to dump out the current values for selected variables. =cut - $obj->_handle_V_command_and_X_command; - =head4 C<x> - evaluate and print an expression Hands the expression off to C<DB::eval>, setting it up to print the value @@ -2793,22 +2795,16 @@ via C<dumpvar.pl> instead of just printing it directly. =cut - $obj->_handle_x_command; - =head4 C<m> - print methods Just uses C<DB::methods> to determine what methods are available. =cut - _DB__handle_m_command($obj); - =head4 C<f> - switch files =cut - _DB__handle_f_command($obj); - =head4 C<.> - return to last-executed line. We set C<$incr> to -1 to indicate that the debugger shouldn't move ahead, @@ -2816,8 +2812,6 @@ and then we look up the line in the magical C<%dbline> hash. =cut - _DB__handle_dot_command($obj); - =head4 C<-> - back one window We change C<$start> to be one window back; if we go back past the first line, ##### The changes to the 'p' command first appeared at this commit: ##### $ git show --format=fuller 8f144dfcb6b7d7a28faa9e82a687338b141c272f |cat commit 8f144dfcb6b7d7a28faa9e82a687338b141c272f Author: Shlomi Fish <shlomif@shlomifish.org> AuthorDate: Sun Oct 14 13:22:45 2012 +0200 Commit: Ricardo Signes <rjbs@cpan.org> CommitDate: Mon Nov 12 09:18:41 2012 -0500 Low hanging fruit is now in %cmd_lookup. diff --git a/lib/perl5db.pl b/lib/perl5db.pl index d79be51..56b8d65 100644 --- a/lib/perl5db.pl +++ b/lib/perl5db.pl @@ -2425,20 +2425,32 @@ my %cmd_lookup = ( '-' => { t => 'm', v => '_handle_dash_command', }, '.' => { t => 's', v => \&_DB__handle_dot_command, }, + '=' => { t => 'm', v => '_handle_equal_sign_command', }, + 'H' => { t => 'm', v => '_handle_H_command', }, 'S' => { t => 'm', v => '_handle_S_command', }, 'T' => { t => 'm', v => '_handle_T_command', }, + 'W' => { t => 'm', v => '_handle_W_command', }, 'c' => { t => 's', v => \&_DB__handle_c_command, }, 'f' => { t => 's', v => \&_DB__handle_f_command, }, 'm' => { t => 's', v => \&_DB__handle_m_command, }, 'n' => { t => 'm', v => '_handle_n_command', }, + 'p' => { t => 'm', v => '_handle_p_command', }, 'q' => { t => 'm', v => '_handle_q_command', }, 'r' => { t => 'm', v => '_handle_r_command', }, 's' => { t => 'm', v => '_handle_s_command', }, + 'save' => { t => 'm', v => '_handle_save_command', }, + 'source' => { t => 'm', v => '_handle_source_command', }, 't' => { t => 'm', v => '_handle_t_command', }, + 'w' => { t => 'm', v => '_handle_w_command', }, 'x' => { t => 'm', v => '_handle_x_command', }, 'y' => { t => 's', v => \&_DB__handle_y_command, }, (map { $_ => { t => 'm', v => '_handle_V_command_and_X_command', }, } ('X', 'V')), + (map { $_ => { t => 'm', v => '_handle_enable_disable_commands', }, } + qw(enable disable)), + (map { $_ => + { t => 's', v => \&_DB__handle_restart_and_rerun_commands, }, + } qw(R rerun)), ); sub DB { @@ -2888,18 +2900,10 @@ Just calls C<DB::print_trace>. Just calls C<DB::cmd_w>. -=cut - - $obj->_handle_w_command; - =head4 C<W> - watch-expression processing. Just calls C<DB::cmd_W>. -=cut - - $obj->_handle_W_command; - =head4 C</> - search forward for a string in the source We take the argument and treat it as a pattern. If it turns out to be a @@ -2963,10 +2967,6 @@ C<DB::system> to avoid problems with C<STDIN> and C<STDOUT>. Prints the contents of C<@hist> (if any). -=cut - - $obj->_handle_H_command; - =head4 C<man, doc, perldoc> - look up documentation Just calls C<runman()> to print the appropriate document. @@ -2980,36 +2980,19 @@ Just calls C<runman()> to print the appropriate document. Builds a C<print EXPR> expression in the C<$cmd>; this will get executed at the bottom of the loop. -=cut - - $obj->_handle_p_command; - =head4 C<=> - define command alias Manipulates C<%alias> to add or list command aliases. -=cut - - # = - set up a command alias. - $obj->_handle_equal_sign_command; - =head4 C<source> - read commands from a file. Opens a lexical filehandle and stacks it on C<@cmdfhs>; C<DB::readline> will pick it up. -=cut - - $obj->_handle_source_command; - =head4 C<enable> C<disable> - enable or disable breakpoints This enables or disables breakpoints. -=cut - - $obj->_handle_enable_disable_commands; - =head4 C<save> - send current history to a file Takes the complete history, (not the shrunken version you see with C<H>), @@ -3017,11 +3000,6 @@ and saves it to the given filename, so it can be replayed using C<source>. Note that all C<^(save|source)>'s are commented out with a view to minimise recursion. -=cut - - # save source - write commands to a file for later use - $obj->_handle_save_command; - =head4 C<R> - restart Restart the debugger session. @@ -3030,12 +3008,6 @@ Restart the debugger session. Return to any given position in the B<true>-history list -=cut - - # R - restart execution. - # rerun - controlled restart execution. - _DB__handle_restart_and_rerun_commands($obj); - =head4 C<|, ||> - pipe output through the pager. For C<|>, we save C<OUT> (the debugger's output filehandle) and C<STDOUT> ##### Thank you very much. -- James E Keenan (jkeenan@cpan.org)
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 1.7k
On Sat, 02 Sep 2017 20:02:20 -0700, jkeenan wrote: Show quoted text
> > On Thu, 10 Oct 2013 11:14:24 GMT, smylers@stripey.com wrote: >
> > In the Perl debugger commands written like the following used to > > work, but now throw errors: > > > > p@ARGV
> > I have confirmed the regression. It is still present in blead > (5.27.3).
Thanks, James. And for the automated tests and finding the specific commits that caused the regression. Show quoted text
> However, we should first ask: Is this a bug?
That's a fair question. I think there are three arguments in favour: 1 perl itself doesn't require a space between keywords and variable names, such as in: $ perl -lE "print@ARGV" one two three onetwothree So it's an extra hurdle for users to have to remember that the debugger is different. 2 Commands typed at the debugger prompt are by definition single-use-only throwaway instructions. As such, saving typing time generally matters more than readability. Show quoted text
> That is, was the fact that we once could use the 'p' and 'x' commands > in the debugger without a subsequent wordspace intentional or > accidental?
3 Perl has a history of retaining backwards compatibility even of ‘accidental’ features, except where these get in the way of something else (and that doesn't seem to be the case here). So even if this weren't an intentional feature originally, it was a feature and used by people. (However, after half a decade of this being broken, people have presumably got used to living without this by now.) I see from the commits you tracked this to that a bunch of single-letter debugger commands were added to a lookup table, not just p and x. It may be that the regression also affects other commands. Smylers
CC: Perl5 Porters <perl5-porters [...] perl.org>
To: perlbug <perlbug-followup [...] perl.org>
Date: Mon, 4 Sep 2017 13:52:19 +0200
From: Leon Timmermans <fawaka [...] gmail.com>
Subject: Re: [perl #120174] Debugger command regression: Now requires more spaces
Download (untitled) / with headers
text/plain 1.6k
On Sun, Sep 3, 2017 at 5:02 AM, James E Keenan via RT <perlbug-followup@perl.org> wrote:
Show quoted text
I have confirmed the regression.  It is still present in blead (5.27.3).

However, we should first ask:  Is this a bug?  That is, was the fact that we once could use the 'p' and 'x' commands in the debugger without a subsequent wordspace intentional or accidental?

Note:  I'm not saying this should not be fixed.  I simply think the question should be faced.

I can think of no reason not call this regression a bug.
 
Show quoted text
Use of perlbrew to get various versions of perl shows roughly when the regression occurred:

#####
  good  perlbrew use perl-5.16.3  Loading DB routines from perl5db.pl version 1.37
  bad   perlbrew use perl-5.18.4  Loading DB routines from perl5db.pl version 1.39_11
#####

Which means that the problem occurred roughly between these two commits:

#####
commit 68cd360812f9eaa2d34c45c501e2fef87c44ccde
Author: Jesse Vincent <jesse@bestpractical.com>
Date:   Tue Apr 24 19:02:34 2012 -0400
-$VERSION = '1.36';
+$VERSION = '1.37';

commit 1799399c45baa7e5af45db33567d2a2de629d5e0
Author:     Ricardo Signes <rjbs@cpan.org>
AuthorDate: Fri Jun 7 11:51:45 2013 -0400
Commit:     Ricardo Signes <rjbs@cpan.org>
CommitDate: Fri Jun 7 11:51:45 2013 -0400

    version bumps and perldelta for debugger depth
-$VERSION = '1.39_10';
+$VERSION = '1.40';
#####

There was a lot of work being done on the debugger during this period.  The diff is large enough that I'm attaching it gzipped first.

This period coincides with a grant on the debugger. It's far from the only regression from that period -_-.

Leon
From: Shlomi Fish <shlomif [...] shlomifish.org>
Subject: Re: [perl #120174] Debugger command regression: Now requires more spaces
Date: Tue, 5 Sep 2017 22:50:00 +0300
To: perl5-porters [...] perl.org
CC: perlbug-followup [...] perl.org
Download (untitled) / with headers
text/plain 1.5k
Hi all! On Mon, 04 Sep 2017 01:51:18 -0700 "Smylers via RT" <perlbug-followup@perl.org> wrote: Show quoted text
> On Sat, 02 Sep 2017 20:02:20 -0700, jkeenan wrote:
> > > > On Thu, 10 Oct 2013 11:14:24 GMT, smylers@stripey.com wrote: > >
> > > In the Perl debugger commands written like the following used to > > > work, but now throw errors: > > > > > > p@ARGV
> > > > I have confirmed the regression. It is still present in blead > > (5.27.3).
> > Thanks, James. And for the automated tests and finding the specific > commits that caused the regression. >
Since it was my work on refactoring and adding tests to the debugger that broke this (and which I got paid for), I am willing to take it on myself to work on a fix. But I'd like to wait for the final verdict of whether this should be fixed? Is it ultimately the pumpking's decision? My personal inclination is to keep the regression because: 1. I feel it was an accidental misfeature. 2. It will mean more work for me. 3. It may complicate the code of perl5db.pl and introduce ugliness, workarounds and edge cases. But that is just my opinion and I feel I don't have the final say on the matter. If the powers at be decide otherwise, then I will accept it and work on a fix. Regards, Shlomi Fish [snipped] -- ----------------------------------------------------------------- Shlomi Fish http://www.shlomifish.org/ My Favourite FOSS - http://www.shlomifish.org/open-source/favourite/ One thing I could never understand is why in Microsoft Word, it often happens that I press enter… and the font changes.
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 2.6k
On Tue, 05 Sep 2017 19:51:00 GMT, shlomif@shlomifish.org wrote: Show quoted text
> Hi all! > > On Mon, 04 Sep 2017 01:51:18 -0700 > "Smylers via RT" <perlbug-followup@perl.org> wrote: >
> > On Sat, 02 Sep 2017 20:02:20 -0700, jkeenan wrote:
> > > > > > On Thu, 10 Oct 2013 11:14:24 GMT, smylers@stripey.com wrote: > > >
> > > > In the Perl debugger commands written like the following used to > > > > work, but now throw errors: > > > > > > > > p@ARGV
> > > > > > I have confirmed the regression. It is still present in blead > > > (5.27.3).
> > > > Thanks, James. And for the automated tests and finding the specific > > commits that caused the regression. > >
> > Since it was my work on refactoring and adding tests to the debugger > that broke > this (and which I got paid for), I am willing to take it on myself to > work on a > fix. But I'd like to wait for the final verdict of whether this should > be > fixed? Is it ultimately the pumpking's decision? >
Yes, ultimately it is the pumpking's decision. In this case, I hope he will perform the most difficult but most necessary function in the pumpking's role. I hope he will say, "No." Show quoted text
> My personal inclination is to keep the regression because: 1. I feel > it was an > accidental misfeature.
I agree. Type 'perldoc perldebug' in perl-5.16.3 -- the last production version where you could type a 'p' or an 'x' not followed by a wordspace -- and you will find that the wordspace-less version of the command is neither documented nor tested. You are correct is stating that it was an accidental misfeature. Show quoted text
> 2. It will mean more work for me.
I somewhat disagree. The former pumpking was the person who actually committed your work to blead, and all the rest of us committers had the opportunity to review your work in detail. So it's certainly not your fault alone. Since it was an accidental misfeature, restoring it to working condition should be the responsibility of those who want to restore it. And it will be *a lot more* work for them! Show quoted text
> 3. It may > complicate > the code of perl5db.pl and introduce ugliness, workarounds and edge > cases.
Will it ever! The debugger is an ugly beast, but it was uglier still before your refactoring project. We have more than a few open RT tickets concerning the debugger. I would rather have someone focus on them than on restoring this accidental misfeature. Show quoted text
> But > that is just my opinion and I feel I don't have the final say on the > matter. If > the powers at be decide otherwise, then I will accept it and work on a > fix. > > Regards, > > Shlomi Fish >
So I hope the pumpking says, "No." Thank you very much. -- James E Keenan (jkeenan@cpan.org)
To: "James E Keenan via RT" <perlbug-followup [...] perl.org>
CC: perl5-porters [...] perl.org
Date: Wed, 6 Sep 2017 01:08:29 +0300
From: Shlomi Fish <shlomif [...] shlomifish.org>
Subject: Re: [perl #120174] Debugger command regression: Now requires more spaces
Download (untitled) / with headers
text/plain 3.6k
Hi James and all, On Tue, 05 Sep 2017 13:59:21 -0700 "James E Keenan via RT" <perlbug-followup@perl.org> wrote: Show quoted text
> On Tue, 05 Sep 2017 19:51:00 GMT, shlomif@shlomifish.org wrote:
> > Hi all! > > > > On Mon, 04 Sep 2017 01:51:18 -0700 > > "Smylers via RT" <perlbug-followup@perl.org> wrote: > >
> > > On Sat, 02 Sep 2017 20:02:20 -0700, jkeenan wrote:
> > > > > > > > On Thu, 10 Oct 2013 11:14:24 GMT, smylers@stripey.com wrote: > > > >
> > > > > In the Perl debugger commands written like the following used to > > > > > work, but now throw errors: > > > > > > > > > > p@ARGV
> > > > > > > > I have confirmed the regression. It is still present in blead > > > > (5.27.3).
> > > > > > Thanks, James. And for the automated tests and finding the specific > > > commits that caused the regression. > > >
> > > > Since it was my work on refactoring and adding tests to the debugger > > that broke > > this (and which I got paid for), I am willing to take it on myself to > > work on a > > fix. But I'd like to wait for the final verdict of whether this should > > be > > fixed? Is it ultimately the pumpking's decision? > >
> > Yes, ultimately it is the pumpking's decision.
I see. Show quoted text
> In this case, I hope he will > perform the most difficult but most necessary function in the pumpking's > role. I hope he will say, "No." >
Yes. I recall hearing that Linus Torvalds also once said that that was his most important function as the chief maintainer of the Linux kernel. Show quoted text
> > My personal inclination is to keep the regression because: 1. I feel > > it was an > > accidental misfeature.
> > I agree. Type 'perldoc perldebug' in perl-5.16.3 -- the last production > version where you could type a 'p' or an 'x' not followed by a wordspace -- > and you will find that the wordspace-less version of the command is neither > documented nor tested. You are correct is stating that it was an accidental > misfeature.
Yes. Show quoted text
>
> > 2. It will mean more work for me.
> > I somewhat disagree. The former pumpking was the person who actually > committed your work to blead, and all the rest of us committers had the > opportunity to review your work in detail. So it's certainly not your fault > alone. >
Right. Nevertheless, I did it as part of a paid grant, and so I'm also responsible. Show quoted text
> Since it was an accidental misfeature, restoring it to working condition > should be the responsibility of those who want to restore it. And it will be > *a lot more* work for them!
Yes. I still volunteer to provide a fix patch if the pumpking thinks it should be fixed, but naturally those who want it restored may need to test it. Show quoted text
>
> > 3. It may > > complicate > > the code of perl5db.pl and introduce ugliness, workarounds and edge > > cases.
> > Will it ever! The debugger is an ugly beast, but it was uglier still before > your refactoring project.
Heh. :-) Show quoted text
> > We have more than a few open RT tickets concerning the debugger. I would > rather have someone focus on them than on restoring this accidental > misfeature.
I agree. Show quoted text
>
> > But > > that is just my opinion and I feel I don't have the final say on the > > matter. If > > the powers at be decide otherwise, then I will accept it and work on a > > fix. > > > > Regards, > > > > Shlomi Fish > >
> > So I hope the pumpking says, "No." >
I hope so to, but it is up to him. Show quoted text
> Thank you very much. >
-- ----------------------------------------------------------------- Shlomi Fish http://www.shlomifish.org/ Beginners Site for the Vim text editor - http://vim.begin-site.org/ <rjbs> sub id { my $self = shift; $json_parser_for{ $self } ->decode($json_for{ $self })->{id} } # Inside‐out JSON‐notated objects
Subject: Re: [perl #120174] Debugger command regression: Now requires more spaces
From: Aaron Crane <arc [...] cpan.org>
CC: Perl5 Porters <perl5-porters [...] perl.org>
To: James E Keenan via RT <perlbug-followup [...] perl.org>
Date: Thu, 7 Sep 2017 13:01:58 +0100
Download (untitled) / with headers
text/plain 2.1k
James E Keenan via RT <perlbug-followup@perl.org> wrote: Show quoted text
> Yes, ultimately it is the pumpking's decision. In this case, I hope he will perform the most difficult but most necessary function in the pumpking's role. I hope he will say, "No." >
>> My personal inclination is to keep the regression because: 1. I feel >> it was an accidental misfeature.
> > I agree. Type 'perldoc perldebug' in perl-5.16.3 -- the last production version where you could type a 'p' or an 'x' not followed by a wordspace -- and you will find that the wordspace-less version of the command is neither documented nor tested. You are correct is stating that it was an accidental misfeature.
I disagree. This change in behaviour has caused sufficient annoyance that at least one user has felt motivated to put the effort into reporting it, and to engaging with subsequent discussion about the issue. Furthermore, the previous behaviour dates back to at least Perl 4.000, as far as I can tell; we've certainly been implicitly teaching our users for a long time that they can rely on it. "We probably wouldn't intentionally do it like that today" doesn't seem like a strong argument in favour of retaining this regression. Show quoted text
> Since it was an accidental misfeature, restoring it to working condition should be the responsibility of those who want to restore it.
That seems harsh. Wanting a feature (accidental or not, mis- or not) to remain shouldn't be dependent on knowing how to implement it. Show quoted text
> And it will be *a lot more* work for [those who want to restore it]! >
>> 3. It may complicate the code of perl5db.pl and introduce ugliness, >> workarounds and edge cases.
> > Will it ever! The debugger is an ugly beast, but it was uglier still before your refactoring project.
On the contrary, fixing this regression involves making a trivial change to exactly one regex — it's much easier to fix it than to write a test for the behaviour (in either direction)! (And I say this as someone who had literally never looked at the internals of the debugger before now.) Accordingly, I've attached a patch that fixes this regression, including a test for the original behaviour; and I will apply it in a few days unless overruled by the pumpking. -- Aaron Crane

Message body is not shown because sender requested not to inline it.

Subject: [PATCH] Debugger command regression: Now requires more spaces
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 1.6k
Aaron Crane via RT writes: Show quoted text
> James E Keenan via RT <perlbug-followup@perl.org> wrote: >
> > Since it was an accidental misfeature, restoring it to working > > condition should be the responsibility of those who want to restore > > it.
> > That seems harsh. Wanting a feature (accidental or not, mis- or not) > to remain shouldn't be dependent on knowing how to implement it.
However, I thought I might more usefully spend time looking into what it would take to do this than simply to argue for it. After Jim's excellent dissection, and spotting that the diffs with the regressions looked to be handling debugger commands through some sort of dispatch table, I thought it sounds like the kind of thing that might just require a tweak to a regexp that pulls the commands out of the input, so I started looking in that file, and came up with the attached. Show quoted text
>
> > And it will be *a lot more* work for [those who want to restore it]! > >
> >> 3. It may complicate the code of perl5db.pl and introduce ugliness, > >> workarounds and edge cases.
> > > > Will it ever! The debugger is an ugly beast, but it was uglier still > > before your refactoring project.
> > On the contrary, fixing this regression involves making a trivial > change to exactly one regex — it's much easier to fix it than to write > a test for the behaviour (in either direction)! (And I say this as > someone who had literally never looked at the internals of the > debugger before now.) > > Accordingly, I've attached a patch that fixes this regression, > including a test for the original behaviour; and I will apply it in a > few days unless overruled by the pumpking.
Subject: 0001-Add-tests-for-p-and-x-commands-without-subsequent-wh.patch
From 96a4037df59f332f3cb9affdc33718e114098227 Mon Sep 17 00:00:00 2001 From: James E Keenan <jkeenan@cpan.org> Date: Sat, 2 Sep 2017 22:28:20 -0400 Subject: [PATCH 1/2] Add tests for 'p' and 'x' commands without subsequent whitespace. Tests pass on perl-5.16.3 but should fail (until source code is corrected) on subsequent versions. For: RT #120174 --- MANIFEST | 1 + lib/perl5db.t | 86 ++++++++++++++++++++++++++++++++++++++++++++++++- lib/perl5db/t/rt-120174 | 4 +++ 3 files changed, 90 insertions(+), 1 deletion(-) create mode 100644 lib/perl5db/t/rt-120174 diff --git MANIFEST MANIFEST index effc4665..9a200067 100644 --- MANIFEST +++ MANIFEST @@ -4604,6 +4604,7 @@ lib/perl5db/t/lvalue-bug Tests for the Perl debugger lib/perl5db/t/MyModule.pm Tests for the Perl debugger lib/perl5db/t/proxy-constants Tests for the Perl debugger lib/perl5db/t/rt-104168 Tests for the Perl debugger +lib/perl5db/t/rt-120174 Tests for the Perl debugger lib/perl5db/t/rt-121509-restart-after-chdir Tests for the Perl debugger lib/perl5db/t/rt-61222 Tests for the Perl debugger lib/perl5db/t/rt-66110 Tests for the Perl debugger diff --git lib/perl5db.t lib/perl5db.t index a2dccc6f..3d432ad5 100644 --- lib/perl5db.t +++ lib/perl5db.t @@ -31,7 +31,7 @@ BEGIN { $ENV{PERL_RL} = 'Perl'; # Suppress system Term::ReadLine::Gnu } -plan(123); +plan(127); my $rc_filename = '.perldb'; @@ -2817,6 +2817,90 @@ SKIP: ); } +{ + # perl 5 RT #120174 - 'p' command + my $wrapper = DebugWrap->new( + { + cmds => + [ + 'b 2', + 'c', + 'p@abc', + 'q', + ], + prog => '../lib/perl5db/t/rt-120174', + } + ); + + $wrapper->contents_like( + qr/1234/, + q/RT 120174: p command can be invoked without space after 'p'/, + ); +} + +{ + # perl 5 RT #120174 - 'x' command on array + my $wrapper = DebugWrap->new( + { + cmds => + [ + 'b 2', + 'c', + 'x@abc', + 'q', + ], + prog => '../lib/perl5db/t/rt-120174', + } + ); + + $wrapper->contents_like( + qr/0\s+1\n1\s+2\n2\s+3\n3\s+4/ms, + q/RT 120174: x command can be invoked without space after 'x' before array/, + ); +} + +{ + # perl 5 RT #120174 - 'x' command on array ref + my $wrapper = DebugWrap->new( + { + cmds => + [ + 'b 2', + 'c', + 'x\@abc', + 'q', + ], + prog => '../lib/perl5db/t/rt-120174', + } + ); + + $wrapper->contents_like( + qr/\s+0\s+1\n\s+1\s+2\n\s+2\s+3\n\s+3\s+4/ms, + q/RT 120174: x command can be invoked without space after 'x' before array ref/, + ); +} + +{ + # perl 5 RT #120174 - 'x' command on hash ref + my $wrapper = DebugWrap->new( + { + cmds => + [ + 'b 4', + 'c', + 'x\%xyz', + 'q', + ], + prog => '../lib/perl5db/t/rt-120174', + } + ); + + $wrapper->contents_like( + qr/\s+'alpha'\s+=>\s+'beta'\n\s+'gamma'\s+=>\s+'delta'/ms, + q/RT 120174: x command can be invoked without space after 'x' before hash ref/, + ); +} + END { 1 while unlink ($rc_filename, $out_fn); } diff --git lib/perl5db/t/rt-120174 lib/perl5db/t/rt-120174 new file mode 100644 index 00000000..c79c8510 --- /dev/null +++ lib/perl5db/t/rt-120174 @@ -0,0 +1,4 @@ +@abc = (1..4); +print "hello world\n"; +%xyz = ( 'alpha' => 'beta', 'gamma' => 'delta' ); +print "goodbye world\n"; -- 1.9.1
Subject: 0002-perl-120174-Debugger-cmds-not-requiring-spaces.patch
From ae6c2f451db98ef95c3f9b7813fd9e2eec21d29e Mon Sep 17 00:00:00 2001 From: Smylers <Smylers@stripey.com> Date: Wed, 6 Sep 2017 12:32:09 +0100 Subject: [PATCH 2/2] [perl #120174] Debugger cmds not requiring spaces Make debugger commands like these work again, not requiring a space between a single-letter command and a following argument which starts with punctuation: p$^V x@ARGV x\@ARGV x\%INC Regressions were in d478d7a0 and 8f144dfc. --- lib/perl5db.pl | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git lib/perl5db.pl lib/perl5db.pl index 265b4441..d0c707e8 100644 --- lib/perl5db.pl +++ lib/perl5db.pl @@ -529,7 +529,7 @@ BEGIN { use vars qw($VERSION $header); # bump to X.XX in blead, only use X.XX_XX in maint -$VERSION = '1.51'; +$VERSION = '1.52'; $header = "perl5db.pl version $VERSION"; @@ -1871,7 +1871,10 @@ sub _DB__trim_command_and_return_first_component { $cmd =~ s/\A\s+//s; # trim annoying leading whitespace $cmd =~ s/\s+\z//s; # trim annoying trailing whitespace - my ($verb, $args) = $cmd =~ m{\A(\S*)\s*(.*)}s; + # A single-character debugger command can be immediately followed by its + # argument if they aren't both alphanumeric; otherwise require space + # between commands and arguments: + my ($verb, $args) = $cmd =~ m{\A(.\b|\S*)\s*(.*)}s; $obj->cmd_verb($verb); $obj->cmd_args($args); -- 1.9.1
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 2.1k
I just wrote: Show quoted text
> However, I thought I might more usefully spend time looking into what > it would take to do this than simply to argue for it.
Then somehow only part of my message made it out of my editor window into RT's web form. Apologies for messing that up. Here's what the rest of my mail should've said: After Jim's excellent bisection, and spotting that the diffs with the regressions looked to be handling debugger commands through some sort of dispatch table, I thought it sounds like the kind of thing that might just require a tweak to a regexp that pulls the commands out of the input, so I had a look. That turned out to be pretty much the case: the fix, which passes all the original tests plus Jim's new ones, is basically just adding 4 characters to a pattern match. Attached are patches for Jim's tests plus my fix. Show quoted text
> ... fixing this regression involves making a trivial change to exactly > one regex — it's much easier to fix it than to write a test for the > behaviour (in either direction)!
Snap! Sorry; I did the work yesterday, but ran out of time to submit it then. My apologies for not getting to that sooner, or otherwise indicating I was working on it, and the resultant duplication of effort. Show quoted text
> (And I say this as someone who had literally never looked at the > internals of the debugger before now.)
Ditto. I'm wondering if I could've found and fixed this 4 years ago when I submitted the bug report — and saved everybody much time — though I doubt I would've known where to look without Jim's bisecting last week. Show quoted text
> Accordingly, I've attached a patch that fixes this regression, > including a test for the original behaviour; and I will apply it in a > few days unless overruled by the pumpking.
Thank you! Our patches are basically the same (and that we came up with them independently provides something like code review): • Your match uses /x, making it easier to read what it's doing. Mine has a comment attempting to explain the reasoning behind it. • I used Jim's tests; you wrote a new test. I haven't checked whether his tests something that yours doesn't. Yours makes use of a file that's already there, whereas Jim's adds a new file. Smylers
From: Shlomi Fish <shlomif [...] shlomifish.org>
Subject: Re: [perl #120174] Debugger command regression: Now requires more spaces
CC: perl5-porters [...] perl.org
To: "Smylers via RT" <perlbug-followup [...] perl.org>
Date: Thu, 7 Sep 2017 18:49:09 +0300
Download (untitled) / with headers
text/plain 3.2k
Hi all, On Thu, 07 Sep 2017 07:21:53 -0700 "Smylers via RT" <perlbug-followup@perl.org> wrote: Show quoted text
> I just wrote: >
> > However, I thought I might more usefully spend time looking into what > > it would take to do this than simply to argue for it.
> > Then somehow only part of my message made it out of my editor window > into RT's web form. Apologies for messing that up. > > Here's what the rest of my mail should've said: > > After Jim's excellent bisection, and spotting that the diffs with the > regressions looked to be handling debugger commands through some sort of > dispatch table, I thought it sounds like the kind of thing that might > just require a tweak to a regexp that pulls the commands out of the > input, so I had a look. > > That turned out to be pretty much the case: the fix, which passes all > the original tests plus Jim's new ones, is basically just adding 4 > characters to a pattern match. > > Attached are patches for Jim's tests plus my fix. >
> > ... fixing this regression involves making a trivial change to exactly > > one regex — it's much easier to fix it than to write a test for the > > behaviour (in either direction)!
> > Snap! > > Sorry; I did the work yesterday, but ran out of time to submit it then. > My apologies for not getting to that sooner, or otherwise indicating I > was working on it, and the resultant duplication of effort. >
> > (And I say this as someone who had literally never looked at the > > internals of the debugger before now.)
> > Ditto. I'm wondering if I could've found and fixed this 4 years ago when > I submitted the bug report — and saved everybody much time — though I > doubt I would've known where to look without Jim's bisecting last week. >
> > Accordingly, I've attached a patch that fixes this regression, > > including a test for the original behaviour; and I will apply it in a > > few days unless overruled by the pumpking.
> > Thank you! > > Our patches are basically the same (and that we came up with them > independently provides something like code review): > > • Your match uses /x, making it easier to read what it's doing. Mine has > a comment attempting to explain the reasoning behind it. > > • I used Jim's tests; you wrote a new test. I haven't checked whether > his tests something that yours doesn't. Yours makes use of a file > that's already there, whereas Jim's adds a new file. >
Seeing this twist of events where two patches were provided and they are not too intrusive, I'd like to say that I am leaning towards applying one of them (or a third one that combines them) so the people who reported the problem and those who prepared the patches will be happier. Note though that I'd prefer if the regex read "\S\b" instead of ".\b" because "." is quite unspecific. I still think this was an accidental misfeature but don't mind it being restored and supported. So +0.5 from me. Regards, Shlomi Show quoted text
> Smylers > > --- > via perlbug: queue: perl5 status: open > https://rt.perl.org/Ticket/Display.html?id=120174
-- ----------------------------------------------------------------- Shlomi Fish http://www.shlomifish.org/ I come to bury Caesar, not to praise him. — https://en.wikiquote.org/wiki/Julius_Caesar_%28play%29
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 1.2k
On Thu, 07 Sep 2017 09:11:55 -0700, shlomif@shlomifish.org wrote: Show quoted text
> Seeing this twist of events where two patches were provided and they > are not too intrusive, I'd like to say that I am leaning towards > applying one of them
Yay! Thanks, Shlomi. Show quoted text
> Note though that I'd prefer if the regex read "\S\b" instead of ".\b" > because "." is quite unspecific.
Note the lines immediately preceding the change are: $cmd =~ s/\A\s+//s; # trim annoying leading whitespace $cmd =~ s/\s+\z//s; # trim annoying trailing whitespace So /^./ can't possibly be whitespace here, and as such is equivalent to /^\S/. Aaron's patch has /^\w\b/ instead of /^.\b/ — I chose the latter cos it would also match perl's behaviour in not requiring a space between a punctuation operator and a following word. Looking to see whether any such debugger commands exist, it means that with my patch you can do things like this, without a space after the = sign: =help h That doesn't work without the patch, and presumably also doesn't work with Aaron's patch. I haven't checked whether it used to work. The | and ! debugger commands don't seem to require a space even without my patch (presumably they are handled separately), so allowing = to work without the space would make = consistent with those. Smylers
Subject: Re: [perl #120174] Debugger command regression: Now requires more spaces
From: Sawyer X <xsawyerx [...] gmail.com>
Date: Mon, 11 Sep 2017 14:15:31 +0300
CC: Perl5 Porters <perl5-porters [...] perl.org>
To: Aaron Crane <arc [...] cpan.org>, James E Keenan via RT <perlbug-followup [...] perl.org>
Download (untitled) / with headers
text/plain 3.4k
From the desk of the pumpking: (Sorry, I was just role-dropped a few times, I thought of injecting some humor into this.) It is far more common for me to say "No" than "Yes," despite enjoying seeing more happen than less. In this case I do not see yet why a "No" is a clear decision. Here is what I've seen so far: * This went unnoticed for a while, but now it has. * This isn't documented, but at least one report shows someone tripped on it, enough to report it. (This could lend to assuming others have as well.) * The fix is not complicated, nor does it complicate the code. * It stays in line with how you would expect one-liners to be written in Perl ("my$x=0"). Considering: 1. We prefer backwards compatible when possible[1]. 2. The patches provided exhibit a limited-scope simple fix. 3. This does not add any load or complexity to the code (or abysmal one); I really do not see why we shouldn't fix it and move on I would be happy to hear strong arguments as to why we shouldn't apply one of the available fixes, keep backwards compatibility, and make those affected[2] happy. Otherwise, I'm in favor of applying one the patches. S. [1] And I know, I've often been quoting as though I don't. But I do prefer it. [2] Especially Smylers! On 09/07/2017 03:01 PM, Aaron Crane wrote: Show quoted text
> James E Keenan via RT <perlbug-followup@perl.org> wrote:
>> Yes, ultimately it is the pumpking's decision. In this case, I hope he will perform the most difficult but most necessary function in the pumpking's role. I hope he will say, "No." >>
>>> My personal inclination is to keep the regression because: 1. I feel >>> it was an accidental misfeature.
>> I agree. Type 'perldoc perldebug' in perl-5.16.3 -- the last production version where you could type a 'p' or an 'x' not followed by a wordspace -- and you will find that the wordspace-less version of the command is neither documented nor tested. You are correct is stating that it was an accidental misfeature.
> I disagree. This change in behaviour has caused sufficient annoyance > that at least one user has felt motivated to put the effort into > reporting it, and to engaging with subsequent discussion about the > issue. Furthermore, the previous behaviour dates back to at least Perl > 4.000, as far as I can tell; we've certainly been implicitly teaching > our users for a long time that they can rely on it. "We probably > wouldn't intentionally do it like that today" doesn't seem like a > strong argument in favour of retaining this regression. >
>> Since it was an accidental misfeature, restoring it to working condition should be the responsibility of those who want to restore it.
> That seems harsh. Wanting a feature (accidental or not, mis- or not) > to remain shouldn't be dependent on knowing how to implement it. >
>> And it will be *a lot more* work for [those who want to restore it]! >>
>>> 3. It may complicate the code of perl5db.pl and introduce ugliness, >>> workarounds and edge cases.
>> Will it ever! The debugger is an ugly beast, but it was uglier still before your refactoring project.
> On the contrary, fixing this regression involves making a trivial > change to exactly one regex — it's much easier to fix it than to write > a test for the behaviour (in either direction)! (And I say this as > someone who had literally never looked at the internals of the > debugger before now.) > > Accordingly, I've attached a patch that fixes this regression, > including a test for the original behaviour; and I will apply it in a > few days unless overruled by the pumpking. >
CC: Perl5 Porters <perl5-porters [...] perl.org>
To: James E Keenan via RT <perlbug-followup [...] perl.org>
Date: Sat, 30 Sep 2017 17:36:37 +0100
Subject: Re: [perl #120174] Debugger command regression: Now requires more spaces
From: Aaron Crane <arc [...] cpan.org>
Download (untitled) / with headers
text/plain 312b
Smylers via RT <perlbug-followup@perl.org> wrote: Show quoted text
> Attached are patches for Jim's tests plus my fix.
I've gone with Jim's tests (which are rather more extensive than mine) and your fix. Thanks, applied as 7fdd4f080863703d44282c6988834455d1290405 and 582a8ad99532af5c8db4e42c8880618fbce41c6d. -- Aaron Crane


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

For issues related to this RT instance (aka "perlbug"), please contact perlbug-admin at perl.org