Perl / perl5

šŸŖ The Perl programming language
https://dev.perl.org/perl5/
Other
1.85k stars 527 forks source link

Term::ReadLine should use have a generic event loop hook #11875

Closed p5pRT closed 12 years ago

p5pRT commented 12 years ago

Migrated from rt.perl.org#108470 (status was 'resolved')

Searchable as RT108470$

p5pRT commented 12 years ago

From darin.mcbride@shaw.ca

Created by darin.mcbride@shaw.ca

Term​::ReadLine only allows the Tk event loop to be called during a readline call. This should be updated to use AnyEvent which will still work with Tk\, as well as any other event loop the user may need.

The only downside of this patch is that users currently using TRL with Tk will need to install AE\, too.

Inline Patch ```diff diff -u -r Term-ReadLine-1.07.orig/blib/lib/Term/ReadLine.pm Term-ReadLine-1.07/blib/lib/Term/ReadLine.pm --- Term-ReadLine-1.07.orig/blib/lib/Term/ReadLine.pm 2011-07-07 09:10:31.000000000 -0600 +++ Term-ReadLine-1.07/blib/lib/Term/ReadLine.pm 2012-01-17 13:06:11.000000000 -0700 @@ -109,10 +109,10 @@ =over 12 -=item C +=item C -makes Tk event loop run when waiting for user input (i.e., during -C method). +makes AnyEvent event loop run when waiting for user input (i.e., during +C method). C is an alias for this. =item C @@ -161,7 +161,7 @@ use strict; package Term::ReadLine::Stub; -our @ISA = qw'Term::ReadLine::Tk Term::ReadLine::TermCap'; +our @ISA = qw'Term::ReadLine::AE Term::ReadLine::TermCap'; $DB::emacs = $DB::emacs; # To peacify -w our @rl_term_set; @@ -175,9 +175,8 @@ my ($in,$out,$str) = @$self; my $prompt = shift; print $out $rl_term_set[0], $prompt, $rl_term_set[1], $rl_term_set[2]; - $self->register_Tk - if not $Term::ReadLine::registered and $Term::ReadLine::toloop - and defined &Tk::DoOneEvent; + $self->register_AE + if $Term::ReadLine::toloop; #$str = scalar <$in>; $str = $self->get_line; utf8::upgrade($str) @@ -296,7 +295,7 @@ eval "use Term::ReadLine::Gnu;"; } elsif ($which =~ /\bperl\b/i) { eval "use Term::ReadLine::Perl;"; - } elsif ($which =~ /^(Stub|TermCap|Tk)$/) { + } elsif ($which =~ /^(Stub|TermCap|AE)$/) { # it is already in memory to avoid false exception as seen in: # PERL_RL=Stub perl -e'$SIG{__DIE__} = sub { print @_ }; require Term::ReadLine' } else { @@ -357,41 +356,50 @@ } -package Term::ReadLine::Tk; +package Term::ReadLine::AE; -our($count_handle, $count_DoOne, $count_loop); -$count_handle = $count_DoOne = $count_loop = 0; +our $cv; +our $fe; -our($giveup); -sub handle {$giveup = 1; $count_handle++} - -sub Tk_loop { - # Tk->tkwait('variable',\$giveup); # needs Widget - $count_DoOne++, Tk::DoOneEvent(0) until $giveup; - $count_loop++; - $giveup = 0; +# for the other modules to use +if (not defined &Tk::DoOneEvent) +{ + *Tk::DoOneEvent = sub { + die "what?"; # this shouldn't be called. + } } -sub register_Tk { - my $self = shift; - $Term::ReadLine::registered++ - or Tk->fileevent($self->IN,'readable',\&handle); +sub AE_loop { + my $self = shift; + $cv = AE::cv(); + $cv->recv(); +} +# backwards compatibility +*Tk_loop = \&AE_loop; + +sub register_AE { + my $self = shift; + $fe ||= AE::io($self->IN, 0, sub { $cv->send() }); } +# backwards compatibility +*register_Tk = \®ister_AE; -sub tkRunning { +sub AErunning { $Term::ReadLine::toloop = $_[1] if @_ > 1; $Term::ReadLine::toloop; } +# backwards compatibility +*tkRunning = \&AErunning; sub get_c { my $self = shift; - $self->Tk_loop if $Term::ReadLine::toloop && defined &Tk::DoOneEvent; + $self->AE_loop if $Term::ReadLine::toloop; return getc $self->IN; } sub get_line { my $self = shift; - $self->Tk_loop if $Term::ReadLine::toloop && defined &Tk::DoOneEvent; + $self->AE_loop if $Term::ReadLine::toloop; my $in = $self->IN; local ($/) = "\n"; return scalar <$in>; ```

Only in Term-ReadLine-1.07/blib/man3: Term::ReadLine.3pm

Inline Patch ```diff diff -u -r Term-ReadLine-1.07.orig/lib/Term/ReadLine.pm Term-ReadLine-1.07/lib/Term/ReadLine.pm --- Term-ReadLine-1.07.orig/lib/Term/ReadLine.pm 2011-07-07 09:10:31.000000000 -0600 +++ Term-ReadLine-1.07/lib/Term/ReadLine.pm 2012-01-17 13:08:39.000000000 -0700 @@ -109,10 +109,10 @@ =over 12 -=item C +=item C -makes Tk event loop run when waiting for user input (i.e., during -C method). +makes AnyEvent event loop run when waiting for user input (i.e., during +C method). C is an alias for this. =item C @@ -161,7 +161,7 @@ use strict; package Term::ReadLine::Stub; -our @ISA = qw'Term::ReadLine::Tk Term::ReadLine::TermCap'; +our @ISA = qw'Term::ReadLine::AE Term::ReadLine::TermCap'; $DB::emacs = $DB::emacs; # To peacify -w our @rl_term_set; @@ -175,9 +175,8 @@ my ($in,$out,$str) = @$self; my $prompt = shift; print $out $rl_term_set[0], $prompt, $rl_term_set[1], $rl_term_set[2]; - $self->register_Tk - if not $Term::ReadLine::registered and $Term::ReadLine::toloop - and defined &Tk::DoOneEvent; + $self->register_AE + if $Term::ReadLine::toloop; #$str = scalar <$in>; $str = $self->get_line; utf8::upgrade($str) @@ -296,7 +295,7 @@ eval "use Term::ReadLine::Gnu;"; } elsif ($which =~ /\bperl\b/i) { eval "use Term::ReadLine::Perl;"; - } elsif ($which =~ /^(Stub|TermCap|Tk)$/) { + } elsif ($which =~ /^(Stub|TermCap|AE)$/) { # it is already in memory to avoid false exception as seen in: # PERL_RL=Stub perl -e'$SIG{__DIE__} = sub { print @_ }; require Term::ReadLine' } else { @@ -357,41 +356,51 @@ } -package Term::ReadLine::Tk; +package Term::ReadLine::AE; -our($count_handle, $count_DoOne, $count_loop); -$count_handle = $count_DoOne = $count_loop = 0; +our $cv; +our $fe; -our($giveup); -sub handle {$giveup = 1; $count_handle++} - -sub Tk_loop { - # Tk->tkwait('variable',\$giveup); # needs Widget - $count_DoOne++, Tk::DoOneEvent(0) until $giveup; - $count_loop++; - $giveup = 0; +# for the other modules to use to check if it exists, +# should be eventually removed. +if (not defined &Tk::DoOneEvent) +{ + *Tk::DoOneEvent = sub { + die "what?"; # this shouldn't be called. + } } -sub register_Tk { - my $self = shift; - $Term::ReadLine::registered++ - or Tk->fileevent($self->IN,'readable',\&handle); +sub AE_loop { + my $self = shift; + $cv = AE::cv(); + $cv->recv(); +} +# backwards compatibility +*Tk_loop = \&AE_loop; + +sub register_AE { + my $self = shift; + $fe ||= AE::io($self->IN, 0, sub { $cv->send() }); } +# backwards compatibility +*register_Tk = \®ister_AE; -sub tkRunning { +sub AErunning { $Term::ReadLine::toloop = $_[1] if @_ > 1; $Term::ReadLine::toloop; } +# backwards compatibility +*tkRunning = \&AErunning; sub get_c { my $self = shift; - $self->Tk_loop if $Term::ReadLine::toloop && defined &Tk::DoOneEvent; + $self->AE_loop if $Term::ReadLine::toloop; return getc $self->IN; } sub get_line { my $self = shift; - $self->Tk_loop if $Term::ReadLine::toloop && defined &Tk::DoOneEvent; + $self->AE_loop if $Term::ReadLine::toloop; my $in = $self->IN; local ($/) = "\n"; return scalar <$in>; ```
Perl Info ``` Flags: category=library severity=wishlist module=Term::ReadLine Site configuration information for perl 5.12.4: Configured by Gentoo at Wed Sep 28 07:02:37 MDT 2011. Summary of my perl5 (revision 5 version 12 subversion 4) configuration: Platform: osname=linux, osvers=2.6.39-gentoo-r3, archname=x86_64-linux-thread-multi uname='linux naboo 2.6.39-gentoo-r3 #1 smp sun jul 17 07:13:38 mdt 2011 x86_64 intel(r) core(tm) i7 cpu 930 @ 2.80ghz genuineintel gnulinux ' config_args='-des -Duseshrplib -Darchname=x86_64-linux-thread -Dcc=x86_64-pc-linux-gnu-gcc -Doptimize=-O3 -pipe -march=core2 -Dldflags=-Wl,-O1 -Wl,--as-needed -Dprefix=/usr -Dsiteprefix=/usr -Dvendorprefix=/usr -Dscriptdir=/usr/bin -Dprivlib=/usr/lib64/perl5/5.12.4 -Darchlib=/usr/lib64/perl5/5.12.4/x86_64-linux-thread-multi -Dsitelib=/usr/lib64/perl5/site_perl/5.12.4 -Dsitearch=/usr/lib64/perl5/site_perl/5.12.4/x86_64-linux-thread-multi -Dvendorlib=/usr/lib64/perl5/vendor_perl/5.12.4 -Dvendorarch=/usr/lib64/perl5/vendor_perl/5.12.4/x86_64-linux-thread-multi -Dman1dir=/usr/share/man/man1 -Dman3dir=/usr/share/man/man3 -Dsiteman1dir=/usr/share/man/man1 -Dsiteman3dir=/usr/share/man/man3 -Dvendorman1dir=/usr/share/man/man1 -Dvendorman3dir=/usr/share/man/man3 -Dman1ext=1 -Dman3ext=3pm -Dlibperl=libperl.so.5.12.4 -Dlocincpth= -Duselargefiles -Dd_semctl_semun -Dcf_by=Gentoo -Dmyhostname=localhost -Dperladmin=root@localhost -Dinstallusrbinperl=n -Ud_csh -Uusenm -Di_ndbm -Di_gdbm -Di_db -Dusethreads -DDEBUGGING=none -Dinc_version_list=5.12.3/x86_64-linux-thread-multi 5.12.3 5.12.2/x86_64-linux-thread-multi 5.12.2 5.12.1/x86_64-linux-thread-multi 5.12.1 5.12.0/x86_64-linux-thread-multi 5.12.0 -Dlibpth=/usr/local/lib64 /lib64 /usr/lib64' hint=recommended, useposix=true, d_sigaction=define useithreads=define, usemultiplicity=define useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef use64bitint=define, use64bitall=define, uselongdouble=undef usemymalloc=n, bincompat5005=undef Compiler: cc='x86_64-pc-linux-gnu-gcc', ccflags ='-D_REENTRANT -D_GNU_SOURCE -fno-strict-aliasing -pipe -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64', optimize='-O3 -pipe -march=core2', cppflags='-D_REENTRANT -D_GNU_SOURCE -fno-strict-aliasing -pipe' ccversion='', gccversion='4.5.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='x86_64-pc-linux-gnu-gcc', ldflags ='-Wl,-O1 -Wl,--as-needed' libpth=/usr/local/lib64 /lib64 /usr/lib64 libs=-lnsl -lgdbm -ldb -ldl -lm -lcrypt -lutil -lpthread -lc -lgdbm_compat perllibs=-lnsl -ldl -lm -lcrypt -lutil -lpthread -lc libc=/lib/libc-2.12.2.so, so=so, useshrplib=true, libperl=libperl.so.5.12.4 gnulibc_version='2.12.2' Dynamic Linking: dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E' cccdlflags='-fPIC', lddlflags='-shared -O3 -pipe -march=core2 -Wl,-O1 -Wl,--as-needed' Locally applied patches: 0001-gentoo_MakeMaker-RUNPATH.diff 0002-gentoo_config_over.diff 0003-gentoo_cpan_definstalldirs.diff 0004-gentoo_cpanplus_definstalldirs.diff 0005-gentoo_create-libperl-soname.diff 0006-gentoo_MakeMaker-delete_packlist.diff 0007-fixes_8d66b3f9_h2hp_fix.diff 0008-fixes_f178b03b_h2ph_using_deprecated_goto.diff 0009-gentoo_mod-paths.diff 0010-gentoo_enc2xs.diff 0011-gentoo_IO-Compress_AutoLoader_dropped_from_Compress-Zlib.diff 0012-gentoo_drop-fstack-protector.diff @INC for perl 5.12.4: /etc/perl /usr/lib64/perl5/site_perl/5.12.4/x86_64-linux-thread-multi /usr/lib64/perl5/site_perl/5.12.4 /usr/lib64/perl5/vendor_perl/5.12.4/x86_64-linux-thread-multi /usr/lib64/perl5/vendor_perl/5.12.4 /usr/lib64/perl5/site_perl/5.12.3/x86_64-linux-thread-multi /usr/lib64/perl5/site_perl/5.12.3 /usr/lib64/perl5/site_perl/5.12.2/x86_64-linux-thread-multi /usr/lib64/perl5/site_perl/5.12.2 /usr/lib64/perl5/site_perl /usr/lib64/perl5/vendor_perl/5.12.3/x86_64-linux-thread-multi /usr/lib64/perl5/vendor_perl/5.12.3 /usr/lib64/perl5/vendor_perl/5.12.2/x86_64-linux-thread-multi /usr/lib64/perl5/vendor_perl/5.12.2 /usr/lib64/perl5/vendor_perl /usr/lib64/perl5/5.12.4/x86_64-linux-thread-multi /usr/lib64/perl5/5.12.4 /usr/local/lib/site_perl . Environment for perl 5.12.4: HOME=/home/dmcbride LANG=en_US.utf8 LANGUAGE= LC_ALL=en_US.utf8 LD_LIBRARY_PATH (unset) LOGDIR (unset) PATH=/usr/local/bin:/home/dmcbride/bin:/usr/lib/distcc/bin:/usr/bin:/bin:/opt/bin:/usr/x86_64-pc-linux-gnu/i686-pc-linux-gnu/gcc-bin/4.5.3:/usr/x86_64-pc-linux-gnu/gcc-bin/4.5.3:/share/cvs/bin:/usr/games/bin:/share/bin:/share/darin/bin:/share/cvs/work/shared PERL5LIB (unset) PERL_BADLANG (unset) SHELL=/bin/bash ```
p5pRT commented 12 years ago

From @cpansprout

On Tue Jan 17 12​:27​:38 2012\, dmcbride wrote​:

The only downside of this patch is that users currently using TRL with Tk will need to install AE\, too.

Couldnā€™t you detect whether Tk is installed without AnyEvent\, and fall back to the old code in that case?

--

Father Chrysostomos

p5pRT commented 12 years ago

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

p5pRT commented 12 years ago

From @dmcbride

On Tuesday January 17 2012 1​:09​:03 PM you wrote​:

Couldnā€™t you detect whether Tk is installed without AnyEvent\, and fall back to the old code in that case?

Yes. I guess the first question is whether anyone with the power/authority to commit a change would be interested in it. I was providing the patch as much as a possible tactic as to simply check if anyone was interested. Even this patch likely requires some documentation changes (beyond that which is given here)​:

diff -u -r Term-ReadLine-1.07.orig/lib/Term/ReadLine.pm Term- ReadLine-1.07/lib/Term/ReadLine.pm --- Term-ReadLine-1.07.orig/lib/Term/ReadLine.pm 2011-07-07 09​:10​:31.000000000 -0600 +++ Term-ReadLine-1.07/lib/Term/ReadLine.pm 2012-01-17 16​:20​:40.000000000 -0700 @​@​ -111\,8 +111\,9 @​@​

=item C\

-makes Tk event loop run when waiting for user input (i.e.\, during -C\ method). +makes an event loop run when waiting for user input (i.e.\, during +C\ method). If AnyEvent is loaded\, it is used\, otherwise Tk +is used.

=item C\

@​@​ -176\,8 +177\,7 @​@​   my $prompt = shift;   print $out $rl_term_set[0]\, $prompt\, $rl_term_set[1]\, $rl_term_set[2];   $self->register_Tk - if not $Term​::ReadLine​::registered and $Term​::ReadLine​::toloop - and defined &Tk​::DoOneEvent; + if not $Term​::ReadLine​::registered and $Term​::ReadLine​::toloop;   #$str = scalar \<$in>;   $str = $self->get_line;   utf8​::upgrade($str) @​@​ -359\,23 +359\,50 @​@​

package Term​::ReadLine​::Tk;

-our($count_handle\, $count_DoOne\, $count_loop); -$count_handle = $count_DoOne = $count_loop = 0; - -our($giveup); -sub handle {$giveup = 1; $count_handle++} - -sub Tk_loop { - # Tk->tkwait('variable'\,\$giveup); # needs Widget - $count_DoOne++\, Tk​::DoOneEvent(0) until $giveup; - $count_loop++; - $giveup = 0; +# if AnyEvent is loaded\, use it. +if (defined &AE​::cv) +{ + my ($cv\, $fe); + + # maintain old name for backward-compatibility + *AE_loop = *Tk_loop = sub { + my $self = shift; + $cv = AE​::cv(); + $cv->recv(); + }; +
+ *register_AE = *register_Tk = sub { + my $self = shift; + $fe ||= AE​::io($self->IN\, 0\, sub { $cv->send() }); + }; + + # just because AE is loaded doesn't mean Tk isn't. + if (not defined &Tk​::DoOneEvent) + { + # create the stub as some T​::RL implementations still check + # this directly. This should eventually be removed. + *Tk​::DoOneEvent = sub { + die "should not happen"; + }; + } } +else +{ + my ($giveup); + + # technically\, not AE\, but maybe in the future the Tk-specific + # aspects will be removed. + *AE_loop = *Tk_loop = sub { + Tk​::DoOneEvent(0) until $giveup; + $giveup = 0; + }; +
+ *register_AE = *register_Tk = sub { + my $self = shift; + $Term​::ReadLine​::registered++ + or Tk->fileevent($self->IN\,'readable'\,sub { $giveup = 1}); + };

-sub register_Tk { - my $self = shift; - $Term​::ReadLine​::registered++ - or Tk->fileevent($self->IN\,'readable'\,\&handle); }

sub tkRunning { @​@​ -385\,13 +412\,13 @​@​

sub get_c {   my $self = shift; - $self->Tk_loop if $Term​::ReadLine​::toloop && defined &Tk​::DoOneEvent; + $self->Tk_loop if $Term​::ReadLine​::toloop;   return getc $self->IN; }

sub get_line {   my $self = shift; - $self->Tk_loop if $Term​::ReadLine​::toloop && defined &Tk​::DoOneEvent; + $self->Tk_loop if $Term​::ReadLine​::toloop;   my $in = $self->IN;   local ($/) = "\n";   return scalar \<$in>;

p5pRT commented 12 years ago

From [Unknown Contact. See original ticket]

On Tuesday January 17 2012 1​:09​:03 PM you wrote​:

Couldnā€™t you detect whether Tk is installed without AnyEvent\, and fall back to the old code in that case?

Yes. I guess the first question is whether anyone with the power/authority to commit a change would be interested in it. I was providing the patch as much as a possible tactic as to simply check if anyone was interested. Even this patch likely requires some documentation changes (beyond that which is given here)​:

diff -u -r Term-ReadLine-1.07.orig/lib/Term/ReadLine.pm Term- ReadLine-1.07/lib/Term/ReadLine.pm --- Term-ReadLine-1.07.orig/lib/Term/ReadLine.pm 2011-07-07 09​:10​:31.000000000 -0600 +++ Term-ReadLine-1.07/lib/Term/ReadLine.pm 2012-01-17 16​:20​:40.000000000 -0700 @​@​ -111\,8 +111\,9 @​@​

=item C\

-makes Tk event loop run when waiting for user input (i.e.\, during -C\ method). +makes an event loop run when waiting for user input (i.e.\, during +C\ method). If AnyEvent is loaded\, it is used\, otherwise Tk +is used.

=item C\

@​@​ -176\,8 +177\,7 @​@​   my $prompt = shift;   print $out $rl_term_set[0]\, $prompt\, $rl_term_set[1]\, $rl_term_set[2];   $self->register_Tk - if not $Term​::ReadLine​::registered and $Term​::ReadLine​::toloop - and defined &Tk​::DoOneEvent; + if not $Term​::ReadLine​::registered and $Term​::ReadLine​::toloop;   #$str = scalar \<$in>;   $str = $self->get_line;   utf8​::upgrade($str) @​@​ -359\,23 +359\,50 @​@​

package Term​::ReadLine​::Tk;

-our($count_handle\, $count_DoOne\, $count_loop); -$count_handle = $count_DoOne = $count_loop = 0; - -our($giveup); -sub handle {$giveup = 1; $count_handle++} - -sub Tk_loop { - # Tk->tkwait('variable'\,\$giveup); # needs Widget - $count_DoOne++\, Tk​::DoOneEvent(0) until $giveup; - $count_loop++; - $giveup = 0; +# if AnyEvent is loaded\, use it. +if (defined &AE​::cv) +{ + my ($cv\, $fe); + + # maintain old name for backward-compatibility + *AE_loop = *Tk_loop = sub { + my $self = shift; + $cv = AE​::cv(); + $cv->recv(); + }; +
+ *register_AE = *register_Tk = sub { + my $self = shift; + $fe ||= AE​::io($self->IN\, 0\, sub { $cv->send() }); + }; + + # just because AE is loaded doesn't mean Tk isn't. + if (not defined &Tk​::DoOneEvent) + { + # create the stub as some T​::RL implementations still check + # this directly. This should eventually be removed. + *Tk​::DoOneEvent = sub { + die "should not happen"; + }; + } } +else +{ + my ($giveup); + + # technically\, not AE\, but maybe in the future the Tk-specific + # aspects will be removed. + *AE_loop = *Tk_loop = sub { + Tk​::DoOneEvent(0) until $giveup; + $giveup = 0; + }; +
+ *register_AE = *register_Tk = sub { + my $self = shift; + $Term​::ReadLine​::registered++ + or Tk->fileevent($self->IN\,'readable'\,sub { $giveup = 1}); + };

-sub register_Tk { - my $self = shift; - $Term​::ReadLine​::registered++ - or Tk->fileevent($self->IN\,'readable'\,\&handle); }

sub tkRunning { @​@​ -385\,13 +412\,13 @​@​

sub get_c {   my $self = shift; - $self->Tk_loop if $Term​::ReadLine​::toloop && defined &Tk​::DoOneEvent; + $self->Tk_loop if $Term​::ReadLine​::toloop;   return getc $self->IN; }

sub get_line {   my $self = shift; - $self->Tk_loop if $Term​::ReadLine​::toloop && defined &Tk​::DoOneEvent; + $self->Tk_loop if $Term​::ReadLine​::toloop;   my $in = $self->IN;   local ($/) = "\n";   return scalar \<$in>;

p5pRT commented 12 years ago

From @cpansprout

On Tue Jan 17 20​:33​:48 2012\, dmcbride@​cpan.org wrote​:

On Tuesday January 17 2012 1​:09​:03 PM you wrote​:

Couldnā€™t you detect whether Tk is installed without AnyEvent\, and fall back to the old code in that case?

Yes. I guess the first question is whether anyone with the power/authority to commit a change would be interested in it. I was providing the patch as much as a possible tactic as to simply check if anyone was interested.

Not being a user of the module\, itā€™s hard for me to say that Iā€™m interested. But if you find it helpful\, and you think others will\, thatā€™s good enough for me.

Is there any way you can write tests for this\, either using a dummy module or using AE but skipping the tests when it is not installed? Otherwise I would feel very uncomfortable committing it\, even if I can prove to myself that the code works.

--

Father Chrysostomos

p5pRT commented 12 years ago

From @dmcbride

On Tuesday January 17 2012 1​:09​:03 PM you wrote​:

Couldnā€™t you detect whether Tk is installed without AnyEvent\, and fall back to the old code in that case?

Yes. I guess the first question is whether anyone with the power/authority to commit a change would be interested in it. I was providing the patch as much as a possible tactic as to simply check if anyone was interested. Even this patch likely requires some documentation changes (beyond that which is given here)​:

diff -u -r Term-ReadLine-1.07.orig/lib/Term/ReadLine.pm Term- ReadLine-1.07/lib/Term/ReadLine.pm --- Term-ReadLine-1.07.orig/lib/Term/ReadLine.pm 2011-07-07 09​:10​:31.000000000 -0600 +++ Term-ReadLine-1.07/lib/Term/ReadLine.pm 2012-01-17 16​:20​:40.000000000 -0700 @​@​ -111\,8 +111\,9 @​@​

=item C\

-makes Tk event loop run when waiting for user input (i.e.\, during -C\ method). +makes an event loop run when waiting for user input (i.e.\, during +C\ method). If AnyEvent is loaded\, it is used\, otherwise Tk +is used.

=item C\

@​@​ -176\,8 +177\,7 @​@​   my $prompt = shift;   print $out $rl_term_set[0]\, $prompt\, $rl_term_set[1]\, $rl_term_set[2];   $self->register_Tk - if not $Term​::ReadLine​::registered and $Term​::ReadLine​::toloop - and defined &Tk​::DoOneEvent; + if not $Term​::ReadLine​::registered and $Term​::ReadLine​::toloop;   #$str = scalar \<$in>;   $str = $self->get_line;   utf8​::upgrade($str) @​@​ -359\,23 +359\,50 @​@​

package Term​::ReadLine​::Tk;

-our($count_handle\, $count_DoOne\, $count_loop); -$count_handle = $count_DoOne = $count_loop = 0; - -our($giveup); -sub handle {$giveup = 1; $count_handle++} - -sub Tk_loop { - # Tk->tkwait('variable'\,\$giveup); # needs Widget - $count_DoOne++\, Tk​::DoOneEvent(0) until $giveup; - $count_loop++; - $giveup = 0; +# if AnyEvent is loaded\, use it. +if (defined &AE​::cv) +{ + my ($cv\, $fe); + + # maintain old name for backward-compatibility + *AE_loop = *Tk_loop = sub { + my $self = shift; + $cv = AE​::cv(); + $cv->recv(); + }; +
+ *register_AE = *register_Tk = sub { + my $self = shift; + $fe ||= AE​::io($self->IN\, 0\, sub { $cv->send() }); + }; + + # just because AE is loaded doesn't mean Tk isn't. + if (not defined &Tk​::DoOneEvent) + { + # create the stub as some T​::RL implementations still check + # this directly. This should eventually be removed. + *Tk​::DoOneEvent = sub { + die "should not happen"; + }; + } } +else +{ + my ($giveup); + + # technically\, not AE\, but maybe in the future the Tk-specific + # aspects will be removed. + *AE_loop = *Tk_loop = sub { + Tk​::DoOneEvent(0) until $giveup; + $giveup = 0; + }; +
+ *register_AE = *register_Tk = sub { + my $self = shift; + $Term​::ReadLine​::registered++ + or Tk->fileevent($self->IN\,'readable'\,sub { $giveup = 1}); + };

-sub register_Tk { - my $self = shift; - $Term​::ReadLine​::registered++ - or Tk->fileevent($self->IN\,'readable'\,\&handle); }

sub tkRunning { @​@​ -385\,13 +412\,13 @​@​

sub get_c {   my $self = shift; - $self->Tk_loop if $Term​::ReadLine​::toloop && defined &Tk​::DoOneEvent; + $self->Tk_loop if $Term​::ReadLine​::toloop;   return getc $self->IN; }

sub get_line {   my $self = shift; - $self->Tk_loop if $Term​::ReadLine​::toloop && defined &Tk​::DoOneEvent; + $self->Tk_loop if $Term​::ReadLine​::toloop;   my $in = $self->IN;   local ($/) = "\n";   return scalar \<$in>;

p5pRT commented 12 years ago

From @dmcbride

On Tuesday January 17 2012 1​:09​:03 PM you wrote​:

Couldnā€™t you detect whether Tk is installed without AnyEvent\, and fall back to the old code in that case?

Yes. I guess the first question is whether anyone with the power/authority to commit a change would be interested in it. I was providing the patch as much as a possible tactic as to simply check if anyone was interested. Even this patch likely requires some documentation changes (beyond that which is given here)​:

diff -u -r Term-ReadLine-1.07.orig/lib/Term/ReadLine.pm Term- ReadLine-1.07/lib/Term/ReadLine.pm --- Term-ReadLine-1.07.orig/lib/Term/ReadLine.pm 2011-07-07 09​:10​:31.000000000 -0600 +++ Term-ReadLine-1.07/lib/Term/ReadLine.pm 2012-01-17 16​:20​:40.000000000 -0700 @​@​ -111\,8 +111\,9 @​@​

=item C\

-makes Tk event loop run when waiting for user input (i.e.\, during -C\ method). +makes an event loop run when waiting for user input (i.e.\, during +C\ method). If AnyEvent is loaded\, it is used\, otherwise Tk +is used.

=item C\

@​@​ -176\,8 +177\,7 @​@​   my $prompt = shift;   print $out $rl_term_set[0]\, $prompt\, $rl_term_set[1]\, $rl_term_set[2];   $self->register_Tk - if not $Term​::ReadLine​::registered and $Term​::ReadLine​::toloop - and defined &Tk​::DoOneEvent; + if not $Term​::ReadLine​::registered and $Term​::ReadLine​::toloop;   #$str = scalar \<$in>;   $str = $self->get_line;   utf8​::upgrade($str) @​@​ -359\,23 +359\,50 @​@​

package Term​::ReadLine​::Tk;

-our($count_handle\, $count_DoOne\, $count_loop); -$count_handle = $count_DoOne = $count_loop = 0; - -our($giveup); -sub handle {$giveup = 1; $count_handle++} - -sub Tk_loop { - # Tk->tkwait('variable'\,\$giveup); # needs Widget - $count_DoOne++\, Tk​::DoOneEvent(0) until $giveup; - $count_loop++; - $giveup = 0; +# if AnyEvent is loaded\, use it. +if (defined &AE​::cv) +{ + my ($cv\, $fe); + + # maintain old name for backward-compatibility + *AE_loop = *Tk_loop = sub { + my $self = shift; + $cv = AE​::cv(); + $cv->recv(); + }; +
+ *register_AE = *register_Tk = sub { + my $self = shift; + $fe ||= AE​::io($self->IN\, 0\, sub { $cv->send() }); + }; + + # just because AE is loaded doesn't mean Tk isn't. + if (not defined &Tk​::DoOneEvent) + { + # create the stub as some T​::RL implementations still check + # this directly. This should eventually be removed. + *Tk​::DoOneEvent = sub { + die "should not happen"; + }; + } } +else +{ + my ($giveup); + + # technically\, not AE\, but maybe in the future the Tk-specific + # aspects will be removed. + *AE_loop = *Tk_loop = sub { + Tk​::DoOneEvent(0) until $giveup; + $giveup = 0; + }; +
+ *register_AE = *register_Tk = sub { + my $self = shift; + $Term​::ReadLine​::registered++ + or Tk->fileevent($self->IN\,'readable'\,sub { $giveup = 1}); + };

-sub register_Tk { - my $self = shift; - $Term​::ReadLine​::registered++ - or Tk->fileevent($self->IN\,'readable'\,\&handle); }

sub tkRunning { @​@​ -385\,13 +412\,13 @​@​

sub get_c {   my $self = shift; - $self->Tk_loop if $Term​::ReadLine​::toloop && defined &Tk​::DoOneEvent; + $self->Tk_loop if $Term​::ReadLine​::toloop;   return getc $self->IN; }

sub get_line {   my $self = shift; - $self->Tk_loop if $Term​::ReadLine​::toloop && defined &Tk​::DoOneEvent; + $self->Tk_loop if $Term​::ReadLine​::toloop;   my $in = $self->IN;   local ($/) = "\n";   return scalar \<$in>;

p5pRT commented 12 years ago

From joelz@pobox.com

On Tue\, Jan 17\, 2012 at 12​:27​:39PM -0800\, Darin McBride wrote​:

# New Ticket Created by Darin McBride # Please include the string​: [perl #108470] # in the subject line of all future correspondence about this issue. # \<URL​: https://rt-archive.perl.org/perl5/Ticket/Display.html?id=108470 >

This is a bug report for perl from darin.mcbride@​shaw.ca\, generated with the help of perlbug 1.39 running under perl 5.12.4.

----------------------------------------------------------------- [Please describe your issue here]

Term​::ReadLine only allows the Tk event loop to be called during a readline call. This should be updated to use AnyEvent which will still work with Tk\, as well as any other event loop the user may need.

The only downside of this patch is that users currently using TRL with Tk will need to install AE\, too.

Hi Darin\,

I am author of a Audio​::Nama\, a Tk application that uses Term​::ReadLine​::Gnu with the tkRunning option to allow the user to control the app by entering commands via T​::RL as well as by manipulating Tk widgets.

While I don't really understand how event loops work\, it appears that the tkRunning setting actually belongs to Term​::ReadLine​::Gnu. IOW\, it is necessary to install the latter for the tkRunning setting to work.

So I wonder\, will it be sufficient to patch Term​::ReadLine? Perhaps Gnu ReadLine will need patched as well?

Second\, I wonder\, why change the existing API? Couldn't another setting\, AERunning\, be simply added rather than replacing tkRunning? As you point out\, not everyone may want to add another dependency to their code.

Third\, if such patches were proposed\, I would want them to be tested fully (and I will be happy to help.)

A final note\, Audio​::Nama already uses AnyEvent\, allowing a choice of event loops. Event is selected for terminal-only mode and Tk for terminal+Tk mode.

As far as I understand\, the Tk event loop will still be required for Tk applications to play friendly with Term​::ReadLine\, however the AnyEvent layer might allow me to\, for example\, rewrite the GUI in Qt.

Bonus questions​: Does any know if tkRunning setting supports other Tk implementations such as Tcl​::Tk or Tkx? Do they provide events such as Tk​::after?

If so\, it may be necessary that these sister implementations of Tk be verified to function properly under patches proposed to Term​::ReadLine\, Term​::ReadLine​::Gnu\, or possibly Gnu ReadLine.

All this is written in theoretical ignorance\, but with a measure of actual experience with the packages in question.

Regards\,

Joel Roth

diff -u -r Term-ReadLine-1.07.orig/blib/lib/Term/ReadLine.pm Term-ReadLine-1.07/blib/lib/Term/ReadLine.pm --- Term-ReadLine-1.07.orig/blib/lib/Term/ReadLine.pm 2011-07-07 09​:10​:31.000000000 -0600 +++ Term-ReadLine-1.07/blib/lib/Term/ReadLine.pm 2012-01-17 13​:06​:11.000000000 -0700 @​@​ -109\,10 +109\,10 @​@​

=over 12

-=item C\ +=item C\

-makes Tk event loop run when waiting for user input (i.e.\, during -C\ method). +makes AnyEvent event loop run when waiting for user input (i.e.\, during +C\ method). C\ is an alias for this.

=item C\

@​@​ -161\,7 +161\,7 @​@​ use strict;

package Term​::ReadLine​::Stub; -our @​ISA = qw'Term​::ReadLine​::Tk Term​::ReadLine​::TermCap'; +our @​ISA = qw'Term​::ReadLine​::AE Term​::ReadLine​::TermCap';

$DB​::emacs = $DB​::emacs; # To peacify -w our @​rl_term_set; @​@​ -175\,9 +175\,8 @​@​ my ($in\,$out\,$str) = @​$self; my $prompt = shift; print $out $rl_term_set[0]\, $prompt\, $rl_term_set[1]\, $rl_term_set[2]; - $self->register_Tk - if not $Term​::ReadLine​::registered and $Term​::ReadLine​::toloop - and defined &Tk​::DoOneEvent; + $self->register_AE + if $Term​::ReadLine​::toloop; #$str = scalar \<$in>; $str = $self->get_line; utf8​::upgrade($str) @​@​ -296\,7 +295\,7 @​@​ eval "use Term​::ReadLine​::Gnu;"; } elsif ($which =~ /\bperl\b/i) { eval "use Term​::ReadLine​::Perl;"; - } elsif ($which =~ /^(Stub|TermCap|Tk)$/) { + } elsif ($which =~ /^(Stub|TermCap|AE)$/) { # it is already in memory to avoid false exception as seen in​: # PERL_RL=Stub perl -e'$SIG{__DIE__} = sub { print @​_ }; require Term​::ReadLine' } else { @​@​ -357\,41 +356\,50 @​@​ }

-package Term​::ReadLine​::Tk; +package Term​::ReadLine​::AE;

-our($count_handle\, $count_DoOne\, $count_loop); -$count_handle = $count_DoOne = $count_loop = 0; +our $cv; +our $fe;

-our($giveup); -sub handle {$giveup = 1; $count_handle++} - -sub Tk_loop { - # Tk->tkwait('variable'\,\$giveup); # needs Widget - $count_DoOne++\, Tk​::DoOneEvent(0) until $giveup; - $count_loop++; - $giveup = 0; +# for the other modules to use +if (not defined &Tk​::DoOneEvent) +{ + *Tk​::DoOneEvent = sub { + die "what?"; # this shouldn't be called. + } }

-sub register_Tk { - my $self = shift; - $Term​::ReadLine​::registered++ - or Tk->fileevent($self->IN\,'readable'\,\&handle); +sub AE_loop { + my $self = shift; + $cv = AE​::cv(); + $cv->recv(); +} +# backwards compatibility +*Tk_loop = \&AE_loop; + +sub register_AE { + my $self = shift; + $fe ||= AE​::io($self->IN\, 0\, sub { $cv->send() }); } +# backwards compatibility +*register_Tk = \&register_AE;

-sub tkRunning { +sub AErunning { $Term​::ReadLine​::toloop = $_[1] if @​_ > 1; $Term​::ReadLine​::toloop; } +# backwards compatibility +*tkRunning = \&AErunning;

sub get_c { my $self = shift; - $self->Tk_loop if $Term​::ReadLine​::toloop && defined &Tk​::DoOneEvent; + $self->AE_loop if $Term​::ReadLine​::toloop; return getc $self->IN; }

sub get_line { my $self = shift; - $self->Tk_loop if $Term​::ReadLine​::toloop && defined &Tk​::DoOneEvent; + $self->AE_loop if $Term​::ReadLine​::toloop; my $in = $self->IN; local ($/) = "\n"; return scalar \<$in>; Only in Term-ReadLine-1.07/blib/man3​: Term​::ReadLine.3pm diff -u -r Term-ReadLine-1.07.orig/lib/Term/ReadLine.pm Term-ReadLine-1.07/lib/Term/ReadLine.pm --- Term-ReadLine-1.07.orig/lib/Term/ReadLine.pm 2011-07-07 09​:10​:31.000000000 -0600 +++ Term-ReadLine-1.07/lib/Term/ReadLine.pm 2012-01-17 13​:08​:39.000000000 -0700 @​@​ -109\,10 +109\,10 @​@​

=over 12

-=item C\ +=item C\

-makes Tk event loop run when waiting for user input (i.e.\, during -C\ method). +makes AnyEvent event loop run when waiting for user input (i.e.\, during +C\ method). C\ is an alias for this.

=item C\

@​@​ -161\,7 +161\,7 @​@​ use strict;

package Term​::ReadLine​::Stub; -our @​ISA = qw'Term​::ReadLine​::Tk Term​::ReadLine​::TermCap'; +our @​ISA = qw'Term​::ReadLine​::AE Term​::ReadLine​::TermCap';

$DB​::emacs = $DB​::emacs; # To peacify -w our @​rl_term_set; @​@​ -175\,9 +175\,8 @​@​ my ($in\,$out\,$str) = @​$self; my $prompt = shift; print $out $rl_term_set[0]\, $prompt\, $rl_term_set[1]\, $rl_term_set[2]; - $self->register_Tk - if not $Term​::ReadLine​::registered and $Term​::ReadLine​::toloop - and defined &Tk​::DoOneEvent; + $self->register_AE + if $Term​::ReadLine​::toloop; #$str = scalar \<$in>; $str = $self->get_line; utf8​::upgrade($str) @​@​ -296\,7 +295\,7 @​@​ eval "use Term​::ReadLine​::Gnu;"; } elsif ($which =~ /\bperl\b/i) { eval "use Term​::ReadLine​::Perl;"; - } elsif ($which =~ /^(Stub|TermCap|Tk)$/) { + } elsif ($which =~ /^(Stub|TermCap|AE)$/) { # it is already in memory to avoid false exception as seen in​: # PERL_RL=Stub perl -e'$SIG{__DIE__} = sub { print @​_ }; require Term​::ReadLine' } else { @​@​ -357\,41 +356\,51 @​@​ }

-package Term​::ReadLine​::Tk; +package Term​::ReadLine​::AE;

-our($count_handle\, $count_DoOne\, $count_loop); -$count_handle = $count_DoOne = $count_loop = 0; +our $cv; +our $fe;

-our($giveup); -sub handle {$giveup = 1; $count_handle++} - -sub Tk_loop { - # Tk->tkwait('variable'\,\$giveup); # needs Widget - $count_DoOne++\, Tk​::DoOneEvent(0) until $giveup; - $count_loop++; - $giveup = 0; +# for the other modules to use to check if it exists\, +# should be eventually removed. +if (not defined &Tk​::DoOneEvent) +{ + *Tk​::DoOneEvent = sub { + die "what?"; # this shouldn't be called. + } }

-sub register_Tk { - my $self = shift; - $Term​::ReadLine​::registered++ - or Tk->fileevent($self->IN\,'readable'\,\&handle); +sub AE_loop { + my $self = shift; + $cv = AE​::cv(); + $cv->recv(); +} +# backwards compatibility +*Tk_loop = \&AE_loop; + +sub register_AE { + my $self = shift; + $fe ||= AE​::io($self->IN\, 0\, sub { $cv->send() }); } +# backwards compatibility +*register_Tk = \&register_AE;

-sub tkRunning { +sub AErunning { $Term​::ReadLine​::toloop = $_[1] if @​_ > 1; $Term​::ReadLine​::toloop; } +# backwards compatibility +*tkRunning = \&AErunning;

sub get_c { my $self = shift; - $self->Tk_loop if $Term​::ReadLine​::toloop && defined &Tk​::DoOneEvent; + $self->AE_loop if $Term​::ReadLine​::toloop; return getc $self->IN; }

sub get_line { my $self = shift; - $self->Tk_loop if $Term​::ReadLine​::toloop && defined &Tk​::DoOneEvent; + $self->AE_loop if $Term​::ReadLine​::toloop; my $in = $self->IN; local ($/) = "\n"; return scalar \<$in>;

[Please do not change anything below this line] ----------------------------------------------------------------- --- Flags​: category=library severity=wishlist module=Term​::ReadLine --- Site configuration information for perl 5.12.4​:

Configured by Gentoo at Wed Sep 28 07​:02​:37 MDT 2011.

Summary of my perl5 (revision 5 version 12 subversion 4) configuration​:

Platform​: osname=linux\, osvers=2.6.39-gentoo-r3\, archname=x86_64-linux-thread-multi uname='linux naboo 2.6.39-gentoo-r3 #1 smp sun jul 17 07​:13​:38 mdt 2011 x86_64 intel(r) core(tm) i7 cpu 930 @​ 2.80ghz genuineintel gnulinux ' config_args='-des -Duseshrplib -Darchname=x86_64-linux-thread -Dcc=x86_64-pc-linux-gnu-gcc -Doptimize=-O3 -pipe -march=core2 -Dldflags=-Wl\,-O1 -Wl\,--as-needed -Dprefix=/usr -Dsiteprefix=/usr -Dvendorprefix=/usr -Dscriptdir=/usr/bin -Dprivlib=/usr/lib64/perl5/5.12.4 -Darchlib=/usr/lib64/perl5/5.12.4/x86_64-linux-thread-multi -Dsitelib=/usr/lib64/perl5/site_perl/5.12.4 -Dsitearch=/usr/lib64/perl5/site_perl/5.12.4/x86_64-linux-thread-multi -Dvendorlib=/usr/lib64/perl5/vendor_perl/5.12.4 -Dvendorarch=/usr/lib64/perl5/vendor_perl/5.12.4/x86_64-linux-thread-multi -Dman1dir=/usr/share/man/man1 -Dman3dir=/usr/share/man/man3 -Dsiteman1dir=/usr/share/man/man1 -Dsiteman3dir=/usr/share/man/man3 -Dvendorman1dir=/usr/share/man/man1 -Dvendorman3dir=/usr/share/man/man3 -Dman1ext=1 -Dman3ext=3pm -Dlibperl=libperl.so.5.12.4 -Dlocincpth= -Duselargefiles -Dd_semctl_semun -Dcf_by=Gentoo -Dmyhostname=localhost -Dperladmin=root@​localhost -Dinstallusrbinperl=n -Ud_csh -Uusenm -Di_ndbm -Di_gdbm -Di_db -Dusethreads -DDEBUGGING=none -Dinc_version_list=5.12.3/x86_64-linux-thread-multi 5.12.3 5.12.2/x86_64-linux-thread-multi 5.12.2 5.12.1/x86_64-linux-thread-multi 5.12.1 5.12.0/x86_64-linux-thread-multi 5.12.0 -Dlibpth=/usr/local/lib64 /lib64 /usr/lib64' hint=recommended\, useposix=true\, d_sigaction=define useithreads=define\, usemultiplicity=define useperlio=define\, d_sfio=undef\, uselargefiles=define\, usesocks=undef use64bitint=define\, use64bitall=define\, uselongdouble=undef usemymalloc=n\, bincompat5005=undef Compiler​: cc='x86_64-pc-linux-gnu-gcc'\, ccflags ='-D_REENTRANT -D_GNU_SOURCE -fno-strict-aliasing -pipe -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64'\, optimize='-O3 -pipe -march=core2'\, cppflags='-D_REENTRANT -D_GNU_SOURCE -fno-strict-aliasing -pipe' ccversion=''\, gccversion='4.5.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='x86_64-pc-linux-gnu-gcc'\, ldflags ='-Wl\,-O1 -Wl\,--as-needed' libpth=/usr/local/lib64 /lib64 /usr/lib64 libs=-lnsl -lgdbm -ldb -ldl -lm -lcrypt -lutil -lpthread -lc -lgdbm_compat perllibs=-lnsl -ldl -lm -lcrypt -lutil -lpthread -lc libc=/lib/libc-2.12.2.so\, so=so\, useshrplib=true\, libperl=libperl.so.5.12.4 gnulibc_version='2.12.2' Dynamic Linking​: dlsrc=dl_dlopen.xs\, dlext=so\, d_dlsymun=undef\, ccdlflags='-Wl\,-E' cccdlflags='-fPIC'\, lddlflags='-shared -O3 -pipe -march=core2 -Wl\,-O1 -Wl\,--as-needed'

Locally applied patches​: 0001-gentoo_MakeMaker-RUNPATH.diff 0002-gentoo_config_over.diff 0003-gentoo_cpan_definstalldirs.diff 0004-gentoo_cpanplus_definstalldirs.diff 0005-gentoo_create-libperl-soname.diff 0006-gentoo_MakeMaker-delete_packlist.diff 0007-fixes_8d66b3f9_h2hp_fix.diff 0008-fixes_f178b03b_h2ph_using_deprecated_goto.diff 0009-gentoo_mod-paths.diff 0010-gentoo_enc2xs.diff 0011-gentoo_IO-Compress_AutoLoader_dropped_from_Compress-Zlib.diff 0012-gentoo_drop-fstack-protector.diff

--- @​INC for perl 5.12.4​: /etc/perl /usr/lib64/perl5/site_perl/5.12.4/x86_64-linux-thread-multi /usr/lib64/perl5/site_perl/5.12.4 /usr/lib64/perl5/vendor_perl/5.12.4/x86_64-linux-thread-multi /usr/lib64/perl5/vendor_perl/5.12.4 /usr/lib64/perl5/site_perl/5.12.3/x86_64-linux-thread-multi /usr/lib64/perl5/site_perl/5.12.3 /usr/lib64/perl5/site_perl/5.12.2/x86_64-linux-thread-multi /usr/lib64/perl5/site_perl/5.12.2 /usr/lib64/perl5/site_perl /usr/lib64/perl5/vendor_perl/5.12.3/x86_64-linux-thread-multi /usr/lib64/perl5/vendor_perl/5.12.3 /usr/lib64/perl5/vendor_perl/5.12.2/x86_64-linux-thread-multi /usr/lib64/perl5/vendor_perl/5.12.2 /usr/lib64/perl5/vendor_perl /usr/lib64/perl5/5.12.4/x86_64-linux-thread-multi /usr/lib64/perl5/5.12.4 /usr/local/lib/site_perl .

--- Environment for perl 5.12.4​: HOME=/home/dmcbride LANG=en_US.utf8 LANGUAGE= LC_ALL=en_US.utf8 LD_LIBRARY_PATH (unset) LOGDIR (unset) PATH=/usr/local/bin​:/home/dmcbride/bin​:/usr/lib/distcc/bin​:/usr/bin​:/bin​:/opt/bin​:/usr/x86_64-pc-linux-gnu/i686-pc-linux-gnu/gcc-bin/4.5.3​:/usr/x86_64-pc-linux-gnu/gcc-bin/4.5.3​:/share/cvs/bin​:/usr/games/bin​:/share/bin​:/share/darin/bin​:/share/cvs/work/shared PERL5LIB (unset) PERL_BADLANG (unset) SHELL=/bin/bash

-- Joel Roth

p5pRT commented 12 years ago

From darin.mcbride@shaw.ca

My apologies for the duplicate posts... apparently the delay on email was longer than I had expected. :-(

On Wednesday January 18 2012 1​:40​:21 AM you wrote​:

I am author of a Audio​::Nama\, a Tk application that uses Term​::ReadLine​::Gnu with the tkRunning option to allow the user to control the app by entering commands via T​::RL as well as by manipulating Tk widgets.

While I don't really understand how event loops work\, it appears that the tkRunning setting actually belongs to Term​::ReadLine​::Gnu. IOW\, it is necessary to install the latter for the tkRunning setting to work.

Looking through the T​::RL​::G code\, it calls $self->register_Tk\, which is in T​::RL​::Tk - where my patches are\, and I *think* it registers the perl Tk_getc function as the getc for readline (it's hard for me to follow)\, which\, again\, is where my patches are.

So I wonder\, will it be sufficient to patch Term​::ReadLine? Perhaps Gnu ReadLine will need patched as well?

Perhaps. However\, at worst\, the Tk mode won't be affected.

Second\, I wonder\, why change the existing API? Couldn't another setting\, AERunning\, be simply added rather than replacing tkRunning? As you point out\, not everyone may want to add another dependency to their code.

The existing API isn't changed\, but my answer to that is that AE is both more modern (not sufficient by itself) and more generic. The point of AE seems to be to support any event model you might need\, including Tk.

Third\, if such patches were proposed\, I would want them to be tested fully (and I will be happy to help.)

My issue here is merely that the existing tests seem less than adequate. I've run some sample code manually - with AE\, Tk\, and AE & Tk\, and it all seemed to work. However\, that's all manual\, and with T​::RL​::Perl\, not T​::RL​::Gnu (I never have luck with T​::RL​::G\, I don't know why). I'll have to look at the tests for the T​::RL implementations to see if they have anything I can use for a base for testing. Otherwise\, I have some really nasty ideas - we'll have to see. I'm having problems enough keeping up the test files for $work :-)

A final note\, Audio​::Nama already uses AnyEvent\, allowing a choice of event loops. Event is selected for terminal-only mode and Tk for terminal+Tk mode.

I'm curious as to how you would receive events registered wtih Event while T​::RL is waiting for text\, since T​::RL is currently going to be spinning the Tk event loop\, not the Event event loop\, unless you have an idle callback registered with Tk that simply spins the Event loop once.

In other words\, as far as I can tell\, today\, T​::RL will only spin a Tk loop.
If Tk isn't loaded\, it won't spin any loop\, and all events will be on hold (with a possible exception of signal handlers) until the user hits enter.

As far as I understand\, the Tk event loop will still be required for Tk applications to play friendly with Term​::ReadLine\, however the AnyEvent layer might allow me to\, for example\, rewrite the GUI in Qt.

Right - by switching T​::RL to use AE (or at least default to it when it's there)\, your GUI can use anything. I would also suggest reading some of the Anyevent​::Impl​::* modules' perldocs - they're informative as to what bugs the author had to overcome. In the Tk module\, he talks about limitations regarding the event loop\, and how the event IDs can wrap around\, clobbering events. "So don't run Tk programs for more than an hour or so." Allowing the user of the module the ability to work around these types of issues simply by using another event model seems prudent.

Bonus questions​: Does any know if tkRunning setting supports other Tk implementations such as Tcl​::Tk or Tkx? Do they provide events such as Tk​::after?

One of my "test that I didn't break anything" scripts is using Tk​::after just to prove that the events are being called. I don't know about Tcl​::Tk or Tkx - I actually don't use Tk\, which is one of the driving factors behind this itch.

If so\, it may be necessary that these sister implementations of Tk be verified to function properly under patches proposed to Term​::ReadLine\, Term​::ReadLine​::Gnu\, or possibly Gnu ReadLine.

More testing == good. (But too much testing == unreasonable delay) Since my new patch leaves everything alone when AE is not loaded\, but works fine with Tk when AE and Tk are loaded\, these should continue to work. But testing our questions is definitely good - I just don't know anything about these other modules to know how to test them.

All this is written in theoretical ignorance\, but with a measure of actual experience with the packages in question.

:-)

Thanks!

p5pRT commented 12 years ago

From @nwc10

On Wed\, Jan 18\, 2012 at 07​:27​:35AM -0700\, Darin McBride wrote​:

The existing API isn't changed\, but my answer to that is that AE is both more modern (not sufficient by itself) and more generic. The point of AE seems to be to support any event model you might need\, including Tk.

And more portable?

Or less portable?

Given the author in question\, and his preferences for how he spends his time and where the cost/benefit of work arounds tips\, I know which I'm going to bet on.

ie I'd love to see AnyEvent as an option\, even the default. But I don't feel comfortable with making it a hard dependency\, and removing the option to use the existing Tk code.

Nicholas Clark

p5pRT commented 12 years ago

From @tux

On Wed\, 18 Jan 2012 15​:39​:58 +0000\, Nicholas Clark \nick@&#8203;ccl4\.org wrote​:

On Wed\, Jan 18\, 2012 at 07​:27​:35AM -0700\, Darin McBride wrote​:

The existing API isn't changed\, but my answer to that is that AE is both more modern (not sufficient by itself) and more generic. The point of AE seems to be to support any event model you might need\, including Tk.

And more portable?

Or less portable?

Given the author in question\, and his preferences for how he spends his time and where the cost/benefit of work arounds tips\, I know which I'm going to bet on.

ie I'd love to see AnyEvent as an option\, even the default. But I don't feel comfortable with making it a hard dependency\, and removing the option to use the existing Tk code.

You are so capable of writing my opinion way better that I would have been able myself :)

-- H.Merijn Brand http​://tux.nl Perl Monger http​://amsterdam.pm.org/ using 5.00307 through 5.14 and porting perl5.15.x on HP-UX 10.20\, 11.00\, 11.11\, 11.23 and 11.31\, OpenSuSE 10.1\, 11.0 .. 11.4 and AIX 5.2 and 5.3. http​://mirrors.develooper.com/hpux/ http​://www.test-smoke.org/ http​://qa.perl.org http​://www.goldmark.org/jeff/stupid-disclaimers/

p5pRT commented 12 years ago

From @nwc10

On Wed\, Jan 18\, 2012 at 04​:43​:11PM +0100\, H.Merijn Brand wrote​:

You are so capable of writing my opinion way better that I would have been able myself :)

I had 3 or 4 edits to get it right.

Portability is a trade off.

And if one is putting in portability for platforms one doesn't know\, hasn't used\, doesn't use\, and likely never will\, it's (likely) an externality being dumped on one.

So it's not wrong not to do it. [Abigail has made similar valid arguments about File​::Copy versus system("cp"\, ...). I think that MJD has emphasised in at least one talk that it's a trade off\, not a mantra]

But there's more than one way to express this :-/

Nicholas Clark

p5pRT commented 12 years ago

From darin.mcbride@shaw.ca

On Wednesday January 18 2012 7​:40​:35 AM Nicholas Clark via RT wrote​:

ie I'd love to see AnyEvent as an option\, even the default. But I don't feel comfortable with making it a hard dependency\, and removing the option to use the existing Tk code.

Tk is not a hard dependency at the moment\, unless you want an event loop. AE will not be a hard dependency (with my proposed patch)\, either. I don't want to see AE dragged in to the core. At least\, not due to this wishlist item - that's a completely separate discussion which I'm not interested in opening here.

It's a soft dependency​: if you want to use AE\, T​::RL should be able to support it. If you want to use Tk\, I'd suggest using AE anyway\, but the revised patch doesn't force it. If you don't want any event loop\, that still works as before\, too. I don't want to force users of T​::RL into using an event loop as I expect that wanting an event loop is likely to be a minority.

So\, other than a lack of automated unit tests\, I see the revised patch as one that likely answers your concerns.

As for portability\, I have no idea. According to cpantesters\, AE seems to pass on all platforms\, so I'm not sure where the concern is there\, and\, even then\, if it doesn't work on your platform\, don't use it\, this patch still falls back to Tk-only if Tk is loaded but AE isn't.

p5pRT commented 12 years ago

From @nwc10

On Wed\, Jan 18\, 2012 at 10​:32​:08AM -0700\, Darin McBride wrote​:

It's a soft dependency​: if you want to use AE\, T​::RL should be able to support it. If you want to use Tk\, I'd suggest using AE anyway\, but the revised patch doesn't force it. If you don't want any event loop\, that still works as before\, too. I don't want to force users of T​::RL into using an event loop as I expect that wanting an event loop is likely to be a minority.

So\, other than a lack of automated unit tests\, I see the revised patch as one that likely answers your concerns.

It does. I'd missed that this was in the revised patch. Thanks.

Nicholas Clark

p5pRT commented 12 years ago

From joelz@pobox.com

On Wed\, Jan 18\, 2012 at 07​:27​:35AM -0700\, Darin McBride wrote​:

On Wednesday January 18 2012 1​:40​:21 AM Joel Roth wrote​:

A final note\, Audio​::Nama already uses AnyEvent\, allowing a choice of event loops. Event is selected for terminal-only mode and Tk for terminal+Tk mode.

I'm curious as to how you would receive events registered wtih Event while T​::RL is waiting for text\, since T​::RL is currently going to be spinning the Tk event loop\, not the Event event loop\, unless you have an idle callback registered with Tk that simply spins the Event loop once.

I use Event in a situation that the app runs without loaded the Tk based GUI\, to provide timers\, etc. In that case\, tkRunning is not set.

In other words\, as far as I can tell\, today\, T​::RL will only spin a Tk loop.
If Tk isn't loaded\, it won't spin any loop\, and all events will be on hold (with a possible exception of signal handlers) until the user hits enter.

I'm glad no one told *me* that! :-)

A T​::RL based command prompt seems to run fine alongside an Event event loop. I may be able to dig up some test code to verify this FYI.

As far as I understand\, the Tk event loop will still be required for Tk applications to play friendly with Term​::ReadLine\, however the AnyEvent layer might allow me to\, for example\, rewrite the GUI in Qt.

Right - by switching T​::RL to use AE (or at least default to it when it's there)\, your GUI can use anything. I would also suggest reading some of the Anyevent​::Impl​::* modules' perldocs - they're informative as to what bugs the author had to overcome. In the Tk module\, he talks about limitations regarding the event loop\, and how the event IDs can wrap around\, clobbering events. "So don't run Tk programs for more than an hour or so." Allowing the user of the module the ability to work around these types of issues simply by using another event model seems prudent.

Ouch! This is an audio app that might record or play for long periods. Thanks\, I will look into it.

Bonus questions​: Does any know if tkRunning setting supports other Tk implementations such as Tcl​::Tk or Tkx? Do they provide events such as Tk​::after?

One of my "test that I didn't break anything" scripts is using Tk​::after just to prove that the events are being called. I don't know about Tcl​::Tk or Tkx - I actually don't use Tk\, which is one of the driving factors behind this itch.

If so\, it may be necessary that these sister implementations of Tk be verified to function properly under patches proposed to Term​::ReadLine\, Term​::ReadLine​::Gnu\, or possibly Gnu ReadLine.

More testing == good. (But too much testing == unreasonable delay) Since my new patch leaves everything alone when AE is not loaded\, but works fine with Tk when AE and Tk are loaded\, these should continue to work. But testing our questions is definitely good - I just don't know anything about these other modules to know how to test them.

All this is written in theoretical ignorance\, but with a measure of actual experience with the packages in question.

:-)

Thanks!

Seems to be going both ways. :-)

-- Joel Roth

p5pRT commented 12 years ago

From darin.mcbride@shaw.ca

On Wednesday January 18 2012 10​:13​:48 AM you wrote​:

I'm curious as to how you would receive events registered wtih Event while T​::RL is waiting for text\, since T​::RL is currently going to be spinning the Tk event loop\, not the Event event loop\, unless you have an idle callback registered with Tk that simply spins the Event loop once.

I use Event in a situation that the app runs without loaded the Tk based GUI\, to provide timers\, etc. In that case\, tkRunning is not set.

Yeah\, I'm very curious. Are you running multi-threaded? That should do it\, but otherwise\, I can't figure it out.

In other words\, as far as I can tell\, today\, T​::RL will only spin a Tk loop. If Tk isn't loaded\, it won't spin any loop\, and all events will be on hold (with a possible exception of signal handlers) until the user hits enter. I'm glad no one told *me* that! :-)

A T​::RL based command prompt seems to run fine alongside an Event event loop. I may be able to dig up some test code to verify this FYI.

You've piqued my curiosity. :-) Here's the test I've been running\, it's as simple as I can make it. If you uncomment out the two tk lines and comment out the Event line\, it works. But if you try to remove Tk\, the timer doesn't print out the elapsed time (or something approximating such) at the top of the console\, assuming ANSI escape sequences are being interpreted (I was too lazy to do any other method of controlling the cursor as this one dates back to my DOS 5.0 days).

I assume that if I moved the Event loop to one thread and the Tk loop (or basically\, T​::RL without tk) to a separate thread from the Event loop\, it'll work. But that now comes with all the joys and tribulations of multithreading. As opposed to the joys and tribulations of event programming.

#!/usr/bin/perl

use strict; use warnings;

use Event; #use Tk; use AnyEvent; use Term​::ReadLine;

my $esc; BEGIN { $esc = "\x1b[";   print "${esc}2J${esc}3H"; }

my $t = 0; my $w = AE​::timer (0\,1\,sub {print "${esc}s${esc}1H$t s ${esc}u";++$t}); my $term = Term​::ReadLine->new('...'); #$term->tkRunning(1);

my $x = $term->readline('> ');

p5pRT commented 12 years ago

From @rcaputo

On Jan 18\, 2012\, at 10​:39\, Nicholas Clark wrote​:

On Wed\, Jan 18\, 2012 at 07​:27​:35AM -0700\, Darin McBride wrote​:

The existing API isn't changed\, but my answer to that is that AE is both more modern (not sufficient by itself) and more generic. The point of AE seems to be to support any event model you might need\, including Tk.

And more portable?

Or less portable?

Given the author in question\, and his preferences for how he spends his time and where the cost/benefit of work arounds tips\, I know which I'm going to bet on.

Interoperability requires cooperation from the author\, and he's been known to break it for modules he doesn't like. Ask Paul Evans and/or Matt Trout about this​:

https://metacpan.org/source/MLEHMANN/AnyEvent-6.13/lib/AnyEvent.pm#L1396

-- Rocco Caputo \rcaputo@&#8203;pobox\.com

p5pRT commented 12 years ago

From @cpansprout

On Wed Jan 18 12​:53​:06 2012\, rcaputo wrote​:

On Jan 18\, 2012\, at 10​:39\, Nicholas Clark wrote​:

On Wed\, Jan 18\, 2012 at 07​:27​:35AM -0700\, Darin McBride wrote​:

The existing API isn't changed\, but my answer to that is that AE is both more modern (not sufficient by itself) and more generic. The point of AE seems to be to support any event model you might need\, including Tk.

And more portable?

Or less portable?

Given the author in question\, and his preferences for how he spends his time and where the cost/benefit of work arounds tips\, I know which I'm going to bet on.

Interoperability requires cooperation from the author\, and he's been known to break it for modules he doesn't like. Ask Paul Evans and/or Matt Trout about this​:

https://metacpan.org/source/MLEHMANN/AnyEvent- 6.13/lib/AnyEvent.pm#L1396

Tying %INC could solve that. :-)

--

Father Chrysostomos

p5pRT commented 12 years ago

From darin.mcbride@shaw.ca

On Wednesday January 18 2012 12​:53​:07 PM you wrote​:

Interoperability requires cooperation from the author\, and he's been known to break it for modules he doesn't like.

We have more than one developer in our community who has issues with their attitude. Without speaking a personal opinion on whether AE's author is one or not\, or outing myself\, I don't see this as a barrier​: today\, T​::RL supports one event loop. With this patch\, it will support *most* event loops available in Perl. And any that aren't supported could potentially be added to AE without having to maintain T​::RL's event support\, benefiting not just T​::RL\, but any other AE-based module.

So\, I suppose before we stray entirely off-topic\, is it the opinion of someone willing to commit this or a similar patch to T​::RL that all that is missing is some useful unit tests\, or are there other\, more basic issues that need to be resolved?

Also\, do the tests need to conform to core-modules-only (plus AE and Tk)\, or can they be skippable tests that use other modules? (Coro may make testing this much easier\, for example\, I'm not sure yet.)

p5pRT commented 12 years ago

From @cpansprout

On Wed Jan 18 15​:21​:51 2012\, dmcbride wrote​:

So\, I suppose before we stray entirely off-topic\, is it the opinion of someone willing to commit this or a similar patch to T​::RL that all that is missing is some useful unit tests\, or are there other\, more basic issues that need to be resolved?

Unless you can get someone else familiar with this to sign off on the patch\, I would have to spend some time studying it myself before I feel comfortable applying it.

Also\, do the tests need to conform to core-modules-only (plus AE and Tk)\, or can they be skippable tests that use other modules? (Coro may make testing this much easier\, for example\, I'm not sure yet.)

It would make testing easier if you could avoid non-core modules (fewer modules for people to install to make sure they donā€™t break things).

It would make testing even easier if you could use a mock object (preferably in addition to the AE/Tk tests).

--

Father Chrysostomos

p5pRT commented 12 years ago

From vadim.konovalov@alcatel-lucent.com

From​: Darin McBride

today\, T​::RL supports one event loop. With this patch\, it will support *most* event loops available in Perl. And any that aren't supported could potentially be added to AE without having to maintain T​::RL's event support\, benefiting not just T​::RL\, but any other AE-based module.

what most event loops it do support? Gtk? Tcl? Qt?

Actually\, when looking into ./lib/Term/ReadLine.pm and seeking for "tk" I found inside   package Term​::ReadLine​::Tk;

IMO this should not be there - you support your favourite GUI but not mine.

This Term​::ReadLine​::Tk should be on CPAN and not in the CORE. This small group of users of Term​::ReadLine​::Tk will find it on CPAN.

Regards\, Vadim.

p5pRT commented 12 years ago

From @demerphq

On 18 January 2012 21​:52\, Rocco Caputo \rcaputo@&#8203;pobox\.com wrote​:

On Jan 18\, 2012\, at 10​:39\, Nicholas Clark wrote​:

On Wed\, Jan 18\, 2012 at 07​:27​:35AM -0700\, Darin McBride wrote​:

The existing API isn't changed\, but my answer to that is that AE is both more modern (not sufficient by itself) and more generic. Ā The point of AE seems to be to support any event model you might need\, including Tk.

And more portable?

Or less portable?

Given the author in question\, and his preferences for how he spends his time and where the cost/benefit of work arounds tips\, I know which I'm going to bet on.

Interoperability requires cooperation from the author\, and he's been known to break it for modules he doesn't like. Ā Ask Paul Evans and/or Matt Trout about this​:

https://metacpan.org/source/MLEHMANN/AnyEvent-6.13/lib/AnyEvent.pm#L1396

Wow.

IMO the author of IO​::Async​::Loop​::AnyEvent should just redefine AnyEvent​::detect() to bypass this monstrosity.

But if they do are we going to see an arms race over what modules you allowed to use with other modules?!

I consider the piece of code you pointed out to most unperlish\, and an affront to the community and the spirit of CPAN.

IMO AnyEvent should be removed from CPAN until this code is removed\, it seems inappropriate for CPAN to host code that forbids you from using something else on CPAN.

Yves

-- perl -Mre=debug -e "/just|another|perl|hacker/"

p5pRT commented 12 years ago

From @leonerd

On Thu\, Jan 19\, 2012 at 07​:06​:11AM +0100\, Konovalov\, Vadim (Vadim)** CTR ** wrote​:

From​: Darin McBride

today\, T​::RL supports one event loop. With this patch\, it will support *most* event loops available in Perl. And any that aren't supported could potentially be added to AE without having to maintain T​::RL's event support\, benefiting not just T​::RL\, but any other AE-based module.

what most event loops it do support? Gtk? Tcl? Qt?

Actually\, when looking into ./lib/Term/ReadLine.pm and seeking for "tk" I found inside package Term​::ReadLine​::Tk;

IMO this should not be there - you support your favourite GUI but not mine.

This Term​::ReadLine​::Tk should be on CPAN and not in the CORE. This small group of users of Term​::ReadLine​::Tk will find it on CPAN.

+1

This has no business in core's Term​::ReadLine. Anyone who wants integration with their favourite event loop of choice can go to CPAN and install Term​::ReadLine​::AnyEvent\, Term​::ReadLine​::IOAsync\, Term​::ReadLine​::POE\, Term​::ReadLine​::Reflex\, Term​::ReadLine​::Tk\, ...

-- Paul "LeoNerd" Evans

leonerd@​leonerd.org.uk ICQ# 4135350 | Registered Linux# 179460 http​://www.leonerd.org.uk/

p5pRT commented 12 years ago

From @leonerd

On Thu\, Jan 19\, 2012 at 01​:01​:39PM +0100\, demerphq wrote​:

Interoperability requires cooperation from the author\, and he's been known to break it for modules he doesn't like. Ā Ask Paul Evans and/or Matt Trout about this​:

https://metacpan.org/source/MLEHMANN/AnyEvent-6.13/lib/AnyEvent.pm#L1396

Wow.

IMO the author of IO​::Async​::Loop​::AnyEvent should just redefine AnyEvent​::detect() to bypass this monstrosity.

But if they do are we going to see an arms race over what modules you allowed to use with other modules?!

I'm not going to get into a silly childish arms-race over this issue.

MLEHMANN is upset because I found a way to layer IO​::Async atop AnyEvent instead of vice versa\, but doing so required a small peek inside the internals to obtain one feature the AE API doesn't provide (namely\, a "run this piece of code after the next event timeslice"). Were that added\, there could be no possible objection to IO​::Async​::Loop​::AnyEvent\, as it becomes simply another AE-API using module.

He choses not to do that\, thus forcing me to peek inside\, and so he gets upset.

I consider the piece of code you pointed out to most unperlish\, and an affront to the community and the spirit of CPAN.

Which again is why I'm not going to get into an arms race as it will only end badly.

IMO AnyEvent should be removed from CPAN until this code is removed\, it seems inappropriate for CPAN to host code that forbids you from using something else on CPAN.

That seems a -little- OTT as a response\, surely? Perhaps a far better solution would simply be to bring this change to more people's attention\, and point out that other alternatives exist (namely IO​::Async\, POE and Reflex come to mind); and that people should be free to decide which one(s) they want to use.

See also

  http​://leonerds-code.blogspot.com/2011/05/wearing-two-hats.html

TMTOWTDI\, after all...

-- Paul "LeoNerd" Evans

leonerd@​leonerd.org.uk ICQ# 4135350 | Registered Linux# 179460 http​://www.leonerd.org.uk/

p5pRT commented 12 years ago

From @demerphq

On 19 January 2012 17​:42\, Paul LeoNerd Evans \leonerd@&#8203;leonerd\.org\.uk wrote​:

On Thu\, Jan 19\, 2012 at 01​:01​:39PM +0100\, demerphq wrote​:

Interoperability requires cooperation from the author\, and he's been known to break it for modules he doesn't like. Ā Ask Paul Evans and/or Matt Trout about this​:

https://metacpan.org/source/MLEHMANN/AnyEvent-6.13/lib/AnyEvent.pm#L1396

Wow.

IMO the author of IO​::Async​::Loop​::AnyEvent should just redefine AnyEvent​::detect() to bypass this monstrosity.

But if they do are we going to see an arms race over what modules you allowed to use with other modules?!

I'm not going to get into a silly childish arms-race over this issue.

MLEHMANN is upset because I found a way to layer IO​::Async atop AnyEvent instead of vice versa\, but doing so required a small peek inside the internals to obtain one feature the AE API doesn't provide (namely\, a "run this piece of code after the next event timeslice"). Were that added\, there could be no possible objection to IO​::Async​::Loop​::AnyEvent\, as it becomes simply another AE-API using module.

He choses not to do that\, thus forcing me to peek inside\, and so he gets upset.

I consider the piece of code you pointed out to most unperlish\, and an affront to the community and the spirit of CPAN.

Which again is why I'm not going to get into an arms race as it will only end badly.

IMO AnyEvent should be removed from CPAN until this code is removed\, it seems inappropriate for CPAN to host code that forbids you from using something else on CPAN.

That seems a -little- OTT as a response\, surely? Perhaps a far better solution would simply be to bring this change to more people's attention\, and point out that other alternatives exist (namely IO​::Async\, POE and Reflex come to mind); and that people should be free to decide which one(s) they want to use.

Yeah\, its probably OTT as a response. And I am glad you aren't going to start an arms race over it.

I guess I shouldn't let it bother me\, but it just seems wrong to do something like that​: the antithesis of the intent of CPAN and sharing. "You can only use my module if you obey my rules" does not seem to be the right attitude for something on CPAN.

I actually checked to see if Larry Wall had said anything relevant about stuff like this and I found two quotes I think fit​:

from perlstyle​: Ā· Be nice. from Larry Wall*​: Perl doesn't have an infatuation with enforced privacy. It would prefer that you stayed out of its living room because you weren't invited\, not because it has a shotgun

Seems to me that the code we are discussing violates both.

cheers\, yves

* http​://www.goodreads.com/quotes/show/81807

-- perl -Mre=debug -e "/just|another|perl|hacker/"

p5pRT commented 12 years ago

From @Mons

Paul\, I know\, Marc is very hard man to talk\, but why donā€™t you and Marc come to mutual understanding?

Your :​:Loop​::AnyEvent goes into AE's internals. It's not a good point of interaction with other modules

You abuse Marc's module\, so he insert a protection from abusing.

If I know\, that something on CPAN will not work\, or will work badly with my module I prefer to protect from such interaction.

Just try to talk with him and many users of AE and IO​::Async will win instead of stupid war between AE and IO​::Async

On Thu\, Jan 19\, 2012 at 4​:42 AM\, Paul LeoNerd Evans \leonerd@&#8203;leonerd\.org\.ukwrote​:

On Thu\, Jan 19\, 2012 at 01​:01​:39PM +0100\, demerphq wrote​:

Interoperability requires cooperation from the author\, and he's been known to break it for modules he doesn't like. Ask Paul Evans and/or Matt Trout about this​:

https://metacpan.org/source/MLEHMANN/AnyEvent-6.13/lib/AnyEvent.pm#L1396

IMO AnyEvent should be removed from CPAN until this code is removed\, it seems inappropriate for CPAN to host code that forbids you from using something else on CPAN.

That seems a -little- OTT as a response\, surely? Perhaps a far better solution would simply be to bring this change to more people's attention\, and point out that other alternatives exist (namely IO​::Async\, POE and Reflex come to mind); and that people should be free to decide which one(s) they want to use.

-- Best wishes\, Vladimir V. Perepelitsa aka Mons Anderson \inthrax@&#8203;gmail\.com\, \mons@&#8203;cpan\.org http​://github.com/Mons

p5pRT commented 12 years ago

From @demerphq

On 19 January 2012 18​:08\, Vladimir V. Perepelitsa \inthrax@&#8203;gmail\.com wrote​:

Paul\, I know\, Marc is very hard man to talk\, but why donā€™t you and Marc come to mutual understanding?

Your :​:Loop​::AnyEvent goes into AE's internals. It's not a good point of interaction with other modules

You abuse Marc's module\, so he insert a protection from abusing.

This is a gift culture. Its not up to the giver to determine how the receiver uses the gift.

It is also not Perlish to prevent someone from doing something that they want to do because you don't think its a good idea. That is up to the consumer of the module to decide.

It seems to me that if the module warned\, perhaps something like "You have loaded ... prior to this module\, which is known to cause instability in other things using this module. Please do not report bugs about problems you might encounter" then it would be pretty reasonable.

But forbidding someone to use a module because you want to use another module the author does not approve of seems pretty unreasonable to me.

If I know\, that something on CPAN will not work\, or will work badly with my module I prefer to protect from such interaction.

If that is your attitude you should hide your code away and make sure that anyone that uses it signs a contract about terms of use. But that also means it doesn't belong on CPAN and isn't really what I would call "free software". AnyEvent is released under the same terms as Perl itself. Therefore it is "free software". Therefore anyone can take the code and do pretty much whatever they want to it. But they can't do the same with the version on CPAN\, doesn't that strike you as being the antithesis of "free software"?

It simply is not YOUR problem what happens when someone decides to do something you think is stupid with your code. It is that persons problem. Trying to forbid people from doing something you think is stupid just means you are going to stop someone smarter than you doing something smart that you never thought of. This actually seems like a good example of that.

Just try to talk with him and many users of AE and IO​::Async will win instead of stupid war between AE and IO​::Async

Of course it is always better to talk about things. Hopefully they can figure something out.

cheers\, Yves

-- perl -Mre=debug -e "/just|another|perl|hacker/"

p5pRT commented 12 years ago

From @Mons

It is also not Perlish to prevent someone from doing something that they want to do because you don't think its a good idea. That is up to the consumer of the module to decide.

That's a bit another problem. Let's look​: I'm a user of AE and IO​::Async. I don't know about possible problems when I will use them together. So\, from point of user I rather say thanks to Marc for warning me (maybe in a too strict way​: warn would be enough)\, than to Paul for anyevent brokage (possible) without warning.

So I respect that point of author. And if I'm an author of "gun"\, then I will try to protect user from "shells"\, which should be loaded via barrel\, instead of lock.

Of course\, there are some smart users of our modules\, that will do things right and will not cry "A-a-a\, that module is bad"\, just because something was used wrong. But unfortunatelly there are much more such users (

It seems to me that if the module warned\, perhaps something like "You have loaded ... prior to this module\, which is known to cause instability in other things using this module. Please do not report bugs about problems you might encounter" then it would be pretty reasonable.

But forbidding someone to use a module because you want to use another module the author does not approve of seems pretty unreasonable to me.

Completely agree

If I know\, that something on CPAN will not work\, or will work badly with my module I prefer to protect from such interaction.

If that is your attitude you should hide your code away and make sure that anyone that uses it signs a contract about terms of use. But that also means it doesn't belong on CPAN and isn't really what I would call "free software". AnyEvent is released under the same terms as Perl itself. Therefore it is "free software". Therefore anyone can take the code and do pretty much whatever they want to it. But they can't do the same with the version on CPAN\, doesn't that strike you as being the antithesis of "free software"?

It simply is not YOUR problem what happens when someone decides to do something you think is stupid with your code. It is that persons problem. Trying to forbid people from doing something you think is stupid just means you are going to stop someone smarter than you doing something smart that you never thought of. This actually seems like a good example of that.

Just try to talk with him and many users of AE and IO​::Async will win instead of stupid war between AE and IO​::Async

Of course it is always better to talk about things. Hopefully they can figure something out.

cheers\, Yves

-- perl -Mre=debug -e "/just|another|perl|hacker/"

-- Best wishes\, Vladimir V. Perepelitsa aka Mons Anderson \inthrax@&#8203;gmail\.com\, \mons@&#8203;cpan\.org http​://github.com/Mons

p5pRT commented 12 years ago

From @Leont

On Thu\, Jan 19\, 2012 at 6​:08 PM\, Vladimir V. Perepelitsa \inthrax@&#8203;gmail\.com wrote​:

You abuse Marc's module\, so he insert a protection from abusing.

Define 'abusing'​: AFAIK Paul didn't actually break anyone else's code\, which would be the only argument that would impress me. If you define abuse as Ā«doing something with a piece of code that the author hadn't envisionedĀ»\, then I'll gladly be an abuser of code ;-)

Leon

p5pRT commented 12 years ago

From @Mons

On Thu\, Jan 19\, 2012 at 5​:46 AM\, Leon Timmermans \fawaka@&#8203;gmail\.com wrote​:

On Thu\, Jan 19\, 2012 at 6​:08 PM\, Vladimir V. Perepelitsa \inthrax@&#8203;gmail\.com wrote​:

You abuse Marc's module\, so he insert a protection from abusing.

Define 'abusing'​: AFAIK Paul didn't actually break anyone else's code\, which would be the only argument that would impress me. If you define abuse as Ā«doing something with a piece of code that the author hadn't envisionedĀ»\, then I'll gladly be an abuser of code ;-)

My fault. I didn't check by myself what really happens.

Leon

-- Best wishes\, Vladimir V. Perepelitsa aka Mons Anderson \inthrax@&#8203;gmail\.com\, \mons@&#8203;cpan\.org http​://github.com/Mons

p5pRT commented 12 years ago

From joelz@pobox.com

On Wed\, Jan 18\, 2012 at 12​:16​:50PM -0700\, Darin McBride wrote​:

On Wednesday January 18 2012 10​:13​:48 AM Joel Roth wrote​:

I'm curious as to how you would receive events registered wtih Event while T​::RL is waiting for text\, since T​::RL is currently going to be spinning the Tk event loop\, not the Event event loop\, unless you have an idle callback registered with Tk that simply spins the Event loop once.

I use Event in a situation that the app runs without loaded the Tk based GUI\, to provide timers\, etc. In that case\, tkRunning is not set.

Yeah\, I'm very curious. Are you running multi-threaded? That should do it\, but otherwise\, I can't figure it out.

In other words\, as far as I can tell\, today\, T​::RL will only spin a Tk loop. If Tk isn't loaded\, it won't spin any loop\, and all events will be on hold (with a possible exception of signal handlers) until the user hits enter. I'm glad no one told *me* that! :-)

A T​::RL based command prompt seems to run fine alongside an Event event loop. I may be able to dig up some test code to verify this FYI.

You've piqued my curiosity. :-) Here's the test I've been running\, it's as simple as I can make it. If you uncomment out the two tk lines and comment out the Event line\, it works. But if you try to remove Tk\, the timer doesn't print out the elapsed time (or something approximating such) at the top of the console\, assuming ANSI escape sequences are being interpreted (I was too lazy to do any other method of controlling the cursor as this one dates back to my DOS 5.0 days).

I assume that if I moved the Event loop to one thread and the Tk loop (or basically\, T​::RL without tk) to a separate thread from the Event loop\, it'll work. But that now comes with all the joys and tribulations of multithreading. As opposed to the joys and tribulations of event programming.

#!/usr/bin/perl

use strict; use warnings;

use Event; #use Tk; use AnyEvent; use Term​::ReadLine;

my $esc; BEGIN { $esc = "\x1b["; print "${esc}2J${esc}3H"; }

my $t = 0; my $w = AE​::timer (0\,1\,sub {print "${esc}s${esc}1H$t s ${esc}u";++$t}); my $term = Term​::ReadLine->new('...'); #$term->tkRunning(1);

my $x = $term->readline('> ');

Hi\,

While I don't have time this morning to get a working test case\, I find that when the app uses Event\, it runs Event​::loop()\, whereas when it uses Tk\, it *doesn't* run MainLoop(); tkRunning(1) somehow substitutes for it.

Again\, this is all using Terminal​::ReadLine​::Gnu\, which I found necessary to get the behaviors I needed.

Sorry for the lack of specifics at the moment.

Regards\,

-- Joel Roth

p5pRT commented 12 years ago

From darin.mcbride@shaw.ca

On Thursday January 19 2012 10​:13​:52 AM you wrote​:

While I don't have time this morning to get a working test case\, I find that when the app uses Event\,

That's okay\, whenever you have time\, it'd be appreciated.

Again\, this is all using Terminal​::ReadLine​::Gnu\,

I've installed T​::RL​::Gnu now\, and found that T​::RL​::G takes a serious control over STDOUT. I had to change my timer to print to STDERR​:

my $w = AE​::timer (0\,1\,sub {print STDERR "${esc}s${esc}1H$t s ${esc}u";++$t});

My earlier patches seem to work with T​::RL​::G\, too.

Without the patches\, the only way to get my script to work is to "use Tk;".
If I load Event prior to AnyEvent being used\, the timer doesn't get called.
With the patches\, it works.

As for the discussion regarding AE's author's interactions with others\, could we drop the cc on this perlbug for that\, please? Thanks.

(I'm still working on time to write some tests\, but if Joel can show me that using multiple loops is transparent\, then everything may become moot.)

p5pRT commented 12 years ago

From @demerphq

On 19 January 2012 18​:32\, demerphq \demerphq@&#8203;gmail\.com wrote​:

On 19 January 2012 18​:08\, Vladimir V. Perepelitsa \inthrax@&#8203;gmail\.com wrote​:

If I know\, that something on CPAN will not work\, or will work badly with my module I prefer to protect from such interaction.

If that is your attitude you should hide your code away and make sure that anyone that uses it signs a contract about terms of use. But that also means it doesn't belong on CPAN and isn't really what I would call "free software". AnyEvent is released under the same terms as Perl itself. Therefore it is "free software". Therefore anyone can take the code and do pretty much whatever they want to it. But they can't do the same with the version on CPAN\, doesn't that strike you as being the antithesis of "free software"?

I thought about this a lot last night\, and I call bullshit on myself.

The code is forkable as it is released under the PAL.

If people don't like the restriction they can fork it.

I still think such a restriction is a pretty anti-social thing to do\, but it has nothing to do with whether the software is free or not.

My apologies for ranting.

cheers\, Yves

-- perl -Mre=debug -e "/just|another|perl|hacker/"

p5pRT commented 12 years ago

From joelz@pobox.com

On Wed\, Jan 18\, 2012 at 07​:27​:35AM -0700\, Darin McBride wrote​:

I'm curious as to how you would receive events registered wtih Event while T​::RL is waiting for text\, since T​::RL is currently going to be spinning the Tk event loop\, not the Event event loop\, unless you have an idle callback registered with Tk that simply spins the Event loop once.

Here is a demonstration code\, which depends on Term​::ReadLine​::Gnu.

Cheers\,

use Term​::ReadLine; # automatically loads Term​::ReadLine​::Gnu\,   # which must be present use Event; use AnyEvent;

my $term = new Term​::ReadLine("Test Event"); my $attribs = $term->Attribs; $term->callback_handler_install( "> "\, sub{ print "got​: @​_\n" }); my $stdin_watcher = AE​::io(*STDIN\, 0\,   sub   {   $attribs->{'callback_read_char'}->();   } ); my $timer = AE​::timer(5\,0\,   sub   {   print "....timed out\n";   $term->rl_deprep_terminal(); # restore usual terminal behavior   exit;   } ); Event​::loop(); __END__

-- Joel Roth

p5pRT commented 12 years ago

From darin.mcbride@shaw.ca

On Friday January 20 2012 5​:52​:11 AM you wrote​:

On Wed\, Jan 18\, 2012 at 07​:27​:35AM -0700\, Darin McBride wrote​:

I'm curious as to how you would receive events registered wtih Event while T​::RL is waiting for text\, since T​::RL is currently going to be spinning the Tk event loop\, not the Event event loop\, unless you have an idle callback registered with Tk that simply spins the Event loop once.

Here is a demonstration code\, which depends on Term​::ReadLine​::Gnu.

This is interesting. You're right - you need Term​::ReadLine​::Gnu. However\, you've completely bypassed all the Tk code completely. There is no Tk loop going on.

I suspect that with these patches\, you may be able to use T​::RL​::P\, albeit a bit differently. T​::RL becomes your event loop\, which is entirely unfortunate\, but would require a vastly different front-end interface\, I believe\, than what we have now\, and require significant changes to the backends as well. Way too much change to shoehorn in here. If I want to go down that road\, it'd be with an entirely new CPAN module that would be unlikely to ever make it to core.
At least with this\, I should be able to use Coro to get it working "right"\, regardless of the event model I need for other purposes\, and still be able to use T​::RL​::Perl.

T​::RL​::G is almost providing that support\, and you've shoehorned it in\, but the tradeoff is that for people like me who have had problems installing T​::RL​::G\, your code won't be supported. I'm not complaining\, merely observing.

$term->callback_handler_install( "> "\, sub{ print "got​: @​_\n" });

This. Turns the whole thing on its head. :-)

Thanks\, Joel. I appreciate it. I'll leave it up to someone higher up on the food chain to decide if this patch is still needed (I think so)\, and what else needs to be done to accept it\, and on what timetable :-)

p5pRT commented 12 years ago

From @dmcbride

On Wednesday January 18 2012 3​:44​:47 PM you wrote​:

Unless you can get someone else familiar with this to sign off on the patch\, I would have to spend some time studying it myself before I feel comfortable applying it.

Not sure who that'd be :-)

It would make testing easier if you could avoid non-core modules (fewer modules for people to install to make sure they donā€™t break things).

Fair enough. I'll obviously need AE and Tk for the AE and Tk tests\, but I've avoided all other non-core modules.

It would make testing even easier if you could use a mock object (preferably in addition to the AE/Tk tests).

I'm not entirely clear how. I need to ensure that the loop is called at all.

A couple things I noticed. First\, T​::RL​::Stub has its own get_line. This does not honour the tkRunning flag at all. If I remove it\, it's fine. My new patch does so.

Second\, get_line only calls the event loop prior to any characters being typed. That is\, once you start typing\, the event loop stops. I expect this to be a limitation of a "stub" readline. I do not plan on fixing this\, though it is a bug/limitation. It would be far too complex for what has been a trying thread already :-) A fix could be to have get_line call get_c for each character (implicitly trying the event loop again)\, but then it would have to handle special characters (at least \b and \n\, though \<- and -> would be nice). I'll leave that to another day\, as I expect most people that are running event loops with T​::RL are going to be interested in a full RL implementation (​::Gnu or :​:Perl).

Please find attached the new patch. It includes the get_line removal above plus three brand new .t files. I first attempted to feed stuff in through a pipe - it wasn't working. So then I realised that I just need to test that the event loop is called at all\, so I could really exit immediately from there.
Thus\, that's what I'm doing here.

p5pRT commented 12 years ago

From @dmcbride

ae.patch ```diff diff -u -Nr Term-ReadLine-1.07.orig/lib/Term/ReadLine.pm Term-ReadLine-1.07/lib/Term/ReadLine.pm --- Term-ReadLine-1.07.orig/lib/Term/ReadLine.pm 2011-07-07 09:10:31.000000000 -0600 +++ Term-ReadLine-1.07/lib/Term/ReadLine.pm 2012-01-20 07:18:29.000000000 -0700 @@ -111,8 +111,9 @@ =item C -makes Tk event loop run when waiting for user input (i.e., during -C method). +makes an event loop run when waiting for user input (i.e., during +C method). If AnyEvent is loaded, it is used, otherwise Tk +is used. =item C @@ -176,8 +177,7 @@ my $prompt = shift; print $out $rl_term_set[0], $prompt, $rl_term_set[1], $rl_term_set[2]; $self->register_Tk - if not $Term::ReadLine::registered and $Term::ReadLine::toloop - and defined &Tk::DoOneEvent; + if not $Term::ReadLine::registered and $Term::ReadLine::toloop; #$str = scalar <$in>; $str = $self->get_line; utf8::upgrade($str) @@ -279,12 +279,12 @@ my %features = (tkRunning => 1, ornaments => 1, 'newTTY' => 1); sub Features { \%features } -sub get_line { - my $self = shift; - my $in = $self->IN; - local ($/) = "\n"; - return scalar <$in>; -} +#sub get_line { +# my $self = shift; +# my $in = $self->IN; +# local ($/) = "\n"; +# return scalar <$in>; +#} package Term::ReadLine; # So late to allow the above code be defined? @@ -359,23 +359,50 @@ package Term::ReadLine::Tk; -our($count_handle, $count_DoOne, $count_loop); -$count_handle = $count_DoOne = $count_loop = 0; - -our($giveup); -sub handle {$giveup = 1; $count_handle++} - -sub Tk_loop { - # Tk->tkwait('variable',\$giveup); # needs Widget - $count_DoOne++, Tk::DoOneEvent(0) until $giveup; - $count_loop++; - $giveup = 0; +# if AnyEvent is loaded, use it. +if (defined &AE::cv) +{ + my ($cv, $fe); + + # maintain old name for backward-compatibility + *AE_loop = *Tk_loop = sub { + my $self = shift; + $cv = AE::cv(); + $cv->recv(); + }; + + *register_AE = *register_Tk = sub { + my $self = shift; + $fe ||= AE::io($self->IN, 0, sub { $cv->send() }); + }; + + # just because AE is loaded doesn't mean Tk isn't. + if (not defined &Tk::DoOneEvent) + { + # create the stub as some T::RL implementations still check + # this directly. This should eventually be removed. + *Tk::DoOneEvent = sub { + die "should not happen"; + }; + } } +else +{ + my ($giveup); + + # technically, not AE, but maybe in the future the Tk-specific + # aspects will be removed. + *AE_loop = *Tk_loop = sub { + Tk::DoOneEvent(0) until $giveup; + $giveup = 0; + }; + + *register_AE = *register_Tk = sub { + my $self = shift; + $Term::ReadLine::registered++ + or Tk->fileevent($self->IN,'readable',sub { $giveup = 1}); + }; -sub register_Tk { - my $self = shift; - $Term::ReadLine::registered++ - or Tk->fileevent($self->IN,'readable',\&handle); } sub tkRunning { @@ -385,13 +412,13 @@ sub get_c { my $self = shift; - $self->Tk_loop if $Term::ReadLine::toloop && defined &Tk::DoOneEvent; + $self->Tk_loop if $Term::ReadLine::toloop; return getc $self->IN; } sub get_line { my $self = shift; - $self->Tk_loop if $Term::ReadLine::toloop && defined &Tk::DoOneEvent; + $self->Tk_loop if $Term::ReadLine::toloop; my $in = $self->IN; local ($/) = "\n"; return scalar <$in>; diff -u -Nr Term-ReadLine-1.07.orig/t/AE.t Term-ReadLine-1.07/t/AE.t --- Term-ReadLine-1.07.orig/t/AE.t 1969-12-31 17:00:00.000000000 -0700 +++ Term-ReadLine-1.07/t/AE.t 2012-01-20 07:24:24.000000000 -0700 @@ -0,0 +1,33 @@ +#!perl + +use Test::More; + +eval "use AnyEvent; 1" or + plan skip_all => "AnyEvent is not installed."; + +# seeing as the entire point of this test is to test the event handler, +# we need to mock as little as possible. To keep things tightly controlled, +# we'll use the Stub directly. +BEGIN { + $ENV{PERL_RL} = 'Stub o=0'; +} +plan tests => 3; + +# need to delay this so that AE is loaded first. +require Term::ReadLine; +use File::Spec; + +my $t = Term::ReadLine->new('AE'); +ok($t, "Created object"); +is($t->ReadLine, 'Term::ReadLine::Stub', 'Correct type'); +$t->tkRunning(1); + +my $text = 'some text'; +my $T = $text . "\n"; +my $w = AE::timer(0,1,sub { +pass("Event loop called"); +exit 0; +}); + +my $result = $t->readline('Do not press enter>'); +fail("Should not get here."); diff -u -Nr Term-ReadLine-1.07.orig/t/AETk.t Term-ReadLine-1.07/t/AETk.t --- Term-ReadLine-1.07.orig/t/AETk.t 1969-12-31 17:00:00.000000000 -0700 +++ Term-ReadLine-1.07/t/AETk.t 2012-01-20 07:35:38.000000000 -0700 @@ -0,0 +1,35 @@ +#!perl + +use Test::More; + +eval "use Tk; use AnyEvent; 1" or + plan skip_all => "AnyEvent and/or Tk is not installed."; + +# seeing as the entire point of this test is to test the event handler, +# we need to mock as little as possible. To keep things tightly controlled, +# we'll use the Stub directly. +BEGIN { + $ENV{PERL_RL} = 'Stub o=0'; + # ensure AE uses Tk. + $ENV{PERL_ANYEVENT_MODEL} = 'Tk'; +} +plan tests => 3; + +# need to delay this so that AE is loaded first. +require Term::ReadLine; +use File::Spec; + +my $t = Term::ReadLine->new('AE/Tk'); +ok($t, "Created object"); +is($t->ReadLine, 'Term::ReadLine::Stub', 'Correct type'); +$t->tkRunning(1); + +my $text = 'some text'; +my $T = $text . "\n"; +my $w = AE::timer(0,1,sub { +pass("Event loop called"); +exit 0; +}); + +my $result = $t->readline('Do not press enter>'); +fail("Should not get here."); diff -u -Nr Term-ReadLine-1.07.orig/t/Tk.t Term-ReadLine-1.07/t/Tk.t --- Term-ReadLine-1.07.orig/t/Tk.t 1969-12-31 17:00:00.000000000 -0700 +++ Term-ReadLine-1.07/t/Tk.t 2012-01-20 07:24:28.000000000 -0700 @@ -0,0 +1,34 @@ +#!perl + +use Test::More; + +eval "use Tk; 1" or + plan skip_all => "Tk is not installed."; + +# seeing as the entire point of this test is to test the event handler, +# we need to mock as little as possible. To keep things tightly controlled, +# we'll use the Stub directly. +BEGIN { + $ENV{PERL_RL} = 'Stub o=0'; +} +plan tests => 3; + +# need to delay this so that Tk is loaded first. +require Term::ReadLine; +use File::Spec; + +my $t = Term::ReadLine->new('Tk'); +ok($t, "Created object"); +is($t->ReadLine, 'Term::ReadLine::Stub', 'Correct type'); +$t->tkRunning(1); + +my $text = 'some text'; +my $T = $text . "\n"; +my $mw = MainWindow->new(); $mw->withdraw(); +my $w = Tk::after($mw,0,sub { +pass("Event loop called"); +exit 0; +}); + +my $result = $t->readline('Do not press enter>'); +fail("Should not get here."); ```
p5pRT commented 12 years ago

From @avar

On Fri\, Jan 20\, 2012 at 13​:15\, demerphq \demerphq@&#8203;gmail\.com wrote​:

On 19 January 2012 18​:32\, demerphq \demerphq@&#8203;gmail\.com wrote​:

On 19 January 2012 18​:08\, Vladimir V. Perepelitsa \inthrax@&#8203;gmail\.com wrote​:

If I know\, that something on CPAN will not work\, or will work badly with my module I prefer to protect from such interaction.

If that is your attitude you should hide your code away and make sure that anyone that uses it signs a contract about terms of use. But that also means it doesn't belong on CPAN and isn't really what I would call "free software". AnyEvent is released under the same terms as Perl itself. Therefore it is "free software". Therefore anyone can take the code and do pretty much whatever they want to it. But they can't do the same with the version on CPAN\, doesn't that strike you as being the antithesis of "free software"?

I thought about this a lot last night\, and I call bullshit on myself.

The code is forkable as it is released under the PAL.

If people don't like the restriction they can fork it.

I still think such a restriction is a pretty anti-social thing to do\, but it has nothing to do with whether the software is free or not.

My apologies for ranting.

Actually I wasn't aware that software you uploaded to the CPAN even had to be free software. E.g. this module is not​:

  https://metacpan.org/module/Lisp::Fmt#COPYRIGHT

And the CPAN FAQ only mentions that most modules are free software\, not that your module has to be as well​:

  http​://www.cpan.org/misc/cpan-faq.html#How_is_Perl_licensed

Anyway\, these sort of problems with the CPAN keep coming up\, usually when some important module needs a bugfix and the original author is unavailable.

It would be much easier to deal with those issues if the CPAN had a selection of community-maintained indexes instead of the single monolothic index that it has now\, then you could just subscribe to another index and make all these problems go away.

p5pRT commented 12 years ago

From @dmcbride

Is there anything else I can do regarding this patch? Since AE runs in pure-perl\, and T​::RL​::Perl runs in pure-perl (mostly)\, this may make it easier to use the whole stack here in an event loop. I've had trouble installing Tk in the past\, too :-) (Back when I ran Redhat\, I believe.)

Thanks\,

p5pRT commented 12 years ago

From @cpansprout

On Fri Jan 27 12​:32​:22 2012\, dmcbride@​cpan.org wrote​:

Is there anything else I can do regarding this patch?

Just keep nagging. :-)

--

Father Chrysostomos

p5pRT commented 12 years ago

From @ap

* demerphq \demerphq@&#8203;gmail\.com [2012-01-19 13​:05]​:

I consider the piece of code you pointed out to most unperlish\, and an affront to the community and the spirit of CPAN.

From some angles I can see a justification. It may well be that users direct questions about breakage caused by module Bā€™s too-clever use of module A to the author of module A\, instead of the author of module B.

In such a case I can understand upstream lashing back against something done downstream.

I have seen such situations occur many times in the wider libre software community. Bad blood of one kind or another is *inevitable* when they happen.

From what I know so far of this particular situation (which amounts to very little)\, Marcā€™s reaction was not motivated by actual misdirected support burden. In that case it does seem like\, ahem\, a Richard move.

Regards\, -- Aristotle Pagaltzis // \<http​://plasmasturm.org/>

p5pRT commented 12 years ago

From @cpansprout

On Sun Jan 29 11​:33​:06 2012\, aristotle wrote​:

* demerphq \demerphq@&#8203;gmail\.com [2012-01-19 13​:05]​:

I consider the piece of code you pointed out to most unperlish\, and an affront to the community and the spirit of CPAN.

From some angles I can see a justification. It may well be that users direct questions about breakage caused by module Bā€™s too-clever use of module A to the author of module A\, instead of the author of module B.

In such a case I can understand upstream lashing back against something done downstream.

I have seen such situations occur many times in the wider libre software community. Bad blood of one kind or another is *inevitable* when they happen.

From what I know so far of this particular situation (which amounts to very little)\, Marcā€™s reaction was not motivated by actual misdirected support burden. In that case it does seem like\, ahem\, a Richard move.

The real irony is that AnyEvent itself abuses Perl similarly by messing with ${^WARNING_BITS}.

--

Father Chrysostomos

p5pRT commented 12 years ago

From @cpansprout

On Fri Jan 20 12​:02​:11 2012\, dmcbride@​cpan.org wrote​:

On Wednesday January 18 2012 3​:44​:47 PM you wrote​:

Unless you can get someone else familiar with this to sign off on the patch\, I would have to spend some time studying it myself before I feel comfortable applying it.

Not sure who that'd be :-)

It would make testing easier if you could avoid non-core modules (fewer modules for people to install to make sure they donā€™t break things).

Fair enough. I'll obviously need AE and Tk for the AE and Tk tests\, but I've avoided all other non-core modules.

It would make testing even easier if you could use a mock object (preferably in addition to the AE/Tk tests).

I'm not entirely clear how. I need to ensure that the loop is called at all.

A couple things I noticed. First\, T​::RL​::Stub has its own get_line. This does not honour the tkRunning flag at all. If I remove it\, it's fine. My new patch does so.

Second\, get_line only calls the event loop prior to any characters being typed. That is\, once you start typing\, the event loop stops. I expect this to be a limitation of a "stub" readline. I do not plan on fixing this\, though it is a bug/limitation. It would be far too complex for what has been a trying thread already :-) A fix could be to have get_line call get_c for each character (implicitly trying the event loop again)\, but then it would have to handle special characters (at least \b and \n\, though \<- and -> would be nice). I'll leave that to another day\, as I expect most people that are running event loops with T​::RL are going to be interested in a full RL implementation (​::Gnu or :​:Perl).

Please find attached the new patch. It includes the get_line removal above plus three brand new .t files. I first attempted to feed stuff in through a pipe - it wasn't working. So then I realised that I just need to test that the event loop is called at all\, so I could really exit immediately from there. Thus\, that's what I'm doing here.

With Tk installed\, but no X11 environment running\, I get this​:

couldn't connect to display "/tmp/launch-YF2sgP/org.x​:0" at /Users/sprout/.cpan/build/Tk-804.030-w5ZSVH/blib/lib/Tk/MainWindow.pm line 53. MainWindow->new() at t/Tk.t line 27 # Looks like you planned 3 tests but ran 2. # Looks like your test exited with 2 just after 2.

In case you havenā€™t guessed\, Iā€™m using Mac OS X. Can you detect that error and skip the test? Iā€™ve noticed that Tk passes its own tests with no X11 running. I havenā€™t yet looked to see what it does.

Apart from that\, your patch looks fine to me.

--

Father Chrysostomos

p5pRT commented 12 years ago

From darin.mcbride@shaw.ca

On Sunday January 29 2012 2​:35​:34 PM you wrote​:

With Tk installed\, but no X11 environment running\, I get this​:

couldn't connect to display "/tmp/launch-YF2sgP/org.x​:0" at /Users/sprout/.cpan/build/Tk-804.030-w5ZSVH/blib/lib/Tk/MainWindow.pm line 53. MainWindow->new() at t/Tk.t line 27 # Looks like you planned 3 tests but ran 2. # Looks like your test exited with 2 just after 2.

In case you havenā€™t guessed\, Iā€™m using Mac OS X. Can you detect that

I couldn't guess that\, I'm not rich enough to have a Mac. :-P :-D

error and skip the test? Iā€™ve noticed that Tk passes its own tests with no X11 running. I havenā€™t yet looked to see what it does.

When I unset my DISPLAY and rerun the TRL tests\, I get about the same problem as you. However\, when I do the same thing with Tk\, I get a bunch of ugly output about it :-S

Anyway\, I think I've about patched this up - I'm almost afraid to make my next comment for fear of another horde forming\, but my system upgraded AE in the meantime\, and since AE 6.11\, there has been a "die" at the top of AnyEvent​::Impl​::Tk\, not sure if it was on purpose or not (it didn't show up in the Changelog\, which makes me think not - Marc doesn't seem to be shy about what he does on purpose)\, and that made my tests fail even with DISPLAY set :-)

Apart from that\, your patch looks fine to me.

Here's the updated patch with eval's around the mainwindow functions to see if it loads or not. It does mean that the AETk patch has to explicitly try it\, whereas that shouldn't normally be necessary\, but them's the breaks I guess.

p5pRT commented 12 years ago

From darin.mcbride@shaw.ca

ae.patch ```diff diff -rN Term-ReadLine-1.07.orig/lib/Term/ReadLine.pm Term-ReadLine-1.07/lib/Term/ReadLine.pm 114,115c114,116 < makes Tk event loop run when waiting for user input (i.e., during < C method). --- > makes an event loop run when waiting for user input (i.e., during > C method). If AnyEvent is loaded, it is used, otherwise Tk > is used. 179,180c180 < if not $Term::ReadLine::registered and $Term::ReadLine::toloop < and defined &Tk::DoOneEvent; --- > if not $Term::ReadLine::registered and $Term::ReadLine::toloop; 282,287c282,287 < sub get_line { < my $self = shift; < my $in = $self->IN; < local ($/) = "\n"; < return scalar <$in>; < } --- > #sub get_line { > # my $self = shift; > # my $in = $self->IN; > # local ($/) = "\n"; > # return scalar <$in>; > #} 362,372c362,388 < our($count_handle, $count_DoOne, $count_loop); < $count_handle = $count_DoOne = $count_loop = 0; < < our($giveup); < sub handle {$giveup = 1; $count_handle++} < < sub Tk_loop { < # Tk->tkwait('variable',\$giveup); # needs Widget < $count_DoOne++, Tk::DoOneEvent(0) until $giveup; < $count_loop++; < $giveup = 0; --- > # if AnyEvent is loaded, use it. > #use Enbugger; Enbugger->stop; > if (defined &AE::cv) > { > my ($cv, $fe); > > # maintain old name for backward-compatibility > *AE_loop = *Tk_loop = sub { > my $self = shift; > $cv = AE::cv(); > $cv->recv(); > }; > > *register_AE = *register_Tk = sub { > my $self = shift; > $fe ||= AE::io($self->IN, 0, sub { $cv->send() }); > }; > > # just because AE is loaded doesn't mean Tk isn't. > if (not defined &Tk::DoOneEvent) > { > # create the stub as some T::RL implementations still check > # this directly. This should eventually be removed. > *Tk::DoOneEvent = sub { > die "should not happen"; > }; > } 373a390,405 > else > { > my ($giveup); > > # technically, not AE, but maybe in the future the Tk-specific > # aspects will be removed. > *AE_loop = *Tk_loop = sub { > Tk::DoOneEvent(0) until $giveup; > $giveup = 0; > }; > > *register_AE = *register_Tk = sub { > my $self = shift; > $Term::ReadLine::registered++ > or Tk->fileevent($self->IN,'readable',sub { $giveup = 1}); > }; 375,378d406 < sub register_Tk { < my $self = shift; < $Term::ReadLine::registered++ < or Tk->fileevent($self->IN,'readable',\&handle); 388c416 < $self->Tk_loop if $Term::ReadLine::toloop && defined &Tk::DoOneEvent; --- > $self->Tk_loop if $Term::ReadLine::toloop; 394c422 < $self->Tk_loop if $Term::ReadLine::toloop && defined &Tk::DoOneEvent; --- > $self->Tk_loop if $Term::ReadLine::toloop; diff -rN Term-ReadLine-1.07.orig/lib/Term/ReadLine.pm~ Term-ReadLine-1.07/lib/Term/ReadLine.pm~ 0a1,429 > =head1 NAME > > Term::ReadLine - Perl interface to various C packages. > If no real package is found, substitutes stubs instead of basic functions. > > =head1 SYNOPSIS > > use Term::ReadLine; > my $term = Term::ReadLine->new('Simple Perl calc'); > my $prompt = "Enter your arithmetic expression: "; > my $OUT = $term->OUT || \*STDOUT; > while ( defined ($_ = $term->readline($prompt)) ) { > my $res = eval($_); > warn $@ if $@; > print $OUT $res, "\n" unless $@; > $term->addhistory($_) if /\S/; > } > > =head1 DESCRIPTION > > This package is just a front end to some other packages. It's a stub to > set up a common interface to the various ReadLine implementations found on > CPAN (under the C namespace). > > =head1 Minimal set of supported functions > > All the supported functions should be called as methods, i.e., either as > > $term = Term::ReadLine->new('name'); > > or as > > $term->addhistory('row'); > > where $term is a return value of Term::ReadLine-Enew(). > > =over 12 > > =item C > > returns the actual package that executes the commands. Among possible > values are C, C, > C. > > =item C > > returns the handle for subsequent calls to following > functions. Argument is the name of the application. Optionally can be > followed by two arguments for C and C filehandles. These > arguments should be globs. > > =item C > > gets an input line, I with actual C > support. Trailing newline is removed. Returns C on C. > > =item C > > adds the line to the history of input, from where it can be used if > the actual C is present. > > =item C, C > > return the filehandles for input and output or C if C > input and output cannot be used for Perl. > > =item C > > If argument is specified, it is an advice on minimal size of line to > be included into history. C means do not include anything into > history. Returns the old value. > > =item C > > returns an array with two strings that give most appropriate names for > files for input and output using conventions C<"E$in">, C<"Eout">. > > =item Attribs > > returns a reference to a hash which describes internal configuration > of the package. Names of keys in this hash conform to standard > conventions with the leading C stripped. > > =item C > > Returns a reference to a hash with keys being features present in > current implementation. Several optional features are used in the > minimal interface: C should be present if the first argument > to C is recognized, and C should be present if > C method is not dummy. C should be present if > lines are put into history automatically (maybe subject to > C), and C if C method is not dummy. > > If C method reports a feature C as present, the > method C is not dummy. > > =back > > =head1 Additional supported functions > > Actually C can use some other package, that will > support a richer set of commands. > > All these commands are callable via method interface and have names > which conform to standard conventions with the leading C stripped. > > The stub package included with the perl distribution allows some > additional methods: > > =over 12 > > =item C > > makes an event loop run when waiting for user input (i.e., during > C method). If AnyEvent is loaded, it is used, otherwise Tk > is used. > > =item C > > makes the command line stand out by using termcap data. The argument > to C should be 0, 1, or a string of a form > C<"aa,bb,cc,dd">. Four components of this string should be names of > I, first two will be issued to make the prompt > standout, last two to make the input line standout. > > =item C > > takes two arguments which are input filehandle and output filehandle. > Switches to use these filehandles. > > =back > > One can check whether the currently loaded ReadLine package supports > these methods by checking for corresponding C. > > =head1 EXPORTS > > None > > =head1 ENVIRONMENT > > The environment variable C governs which ReadLine clone is > loaded. If the value is false, a dummy interface is used. If the value > is true, it should be tail of the name of the package to use, such as > C or C. > > As a special case, if the value of this variable is space-separated, > the tail might be used to disable the ornaments by setting the tail to > be C or C. The head should be as described above, say > > If the variable is not set, or if the head of space-separated list is > empty, the best available package is loaded. > > export "PERL_RL=Perl o=0" # Use Perl ReadLine without ornaments > export "PERL_RL= o=0" # Use best available ReadLine without ornaments > > (Note that processing of C for ornaments is in the discretion of the > particular used C package). > > =cut > > use strict; > > package Term::ReadLine::Stub; > our @ISA = qw'Term::ReadLine::Tk Term::ReadLine::TermCap'; > > $DB::emacs = $DB::emacs; # To peacify -w > our @rl_term_set; > *rl_term_set = \@Term::ReadLine::TermCap::rl_term_set; > > sub PERL_UNICODE_STDIN () { 0x0001 } > > sub ReadLine {'Term::ReadLine::Stub'} > sub readline { > my $self = shift; > my ($in,$out,$str) = @$self; > my $prompt = shift; > print $out $rl_term_set[0], $prompt, $rl_term_set[1], $rl_term_set[2]; > $self->register_Tk > if not $Term::ReadLine::registered and $Term::ReadLine::toloop; > #$str = scalar <$in>; > $str = $self->get_line; > utf8::upgrade($str) > if (${^UNICODE} & PERL_UNICODE_STDIN || defined ${^ENCODING}) && > utf8::valid($str); > print $out $rl_term_set[3]; > # bug in 5.000: chomping empty string creats length -1: > chomp $str if defined $str; > $str; > } > sub addhistory {} > > sub findConsole { > my $console; > my $consoleOUT; > > if (-e "/dev/tty") { > $console = "/dev/tty"; > } elsif (-e "con" or $^O eq 'MSWin32') { > $console = 'CONIN$'; > $consoleOUT = 'CONOUT$'; > } else { > $console = "sys\$command"; > } > > if (($^O eq 'amigaos') || ($^O eq 'beos') || ($^O eq 'epoc')) { > $console = undef; > } > elsif ($^O eq 'os2') { > if ($DB::emacs) { > $console = undef; > } else { > $console = "/dev/con"; > } > } > > $consoleOUT = $console unless defined $consoleOUT; > $console = "&STDIN" unless defined $console; > if ($console eq "/dev/tty" && !open(my $fh, "<", $console)) { > $console = "&STDIN"; > undef($consoleOUT); > } > if (!defined $consoleOUT) { > $consoleOUT = defined fileno(STDERR) && $^O ne 'MSWin32' ? "&STDERR" : "&STDOUT"; > } > ($console,$consoleOUT); > } > > sub new { > die "method new called with wrong number of arguments" > unless @_==2 or @_==4; > #local (*FIN, *FOUT); > my ($FIN, $FOUT, $ret); > if (@_==2) { > my($console, $consoleOUT) = $_[0]->findConsole; > > > # the Windows CONIN$ needs GENERIC_WRITE mode to allow > # a SetConsoleMode() if we end up using Term::ReadKey > open FIN, ( $^O eq 'MSWin32' && $console eq 'CONIN$' ) ? "+<$console" : > "<$console"; > open FOUT,">$consoleOUT"; > > #OUT->autoflush(1); # Conflicts with debugger? > my $sel = select(FOUT); > $| = 1; # for DB::OUT > select($sel); > $ret = bless [\*FIN, \*FOUT]; > } else { # Filehandles supplied > $FIN = $_[2]; $FOUT = $_[3]; > #OUT->autoflush(1); # Conflicts with debugger? > my $sel = select($FOUT); > $| = 1; # for DB::OUT > select($sel); > $ret = bless [$FIN, $FOUT]; > } > if ($ret->Features->{ornaments} > and not ($ENV{PERL_RL} and $ENV{PERL_RL} =~ /\bo\w*=0/)) { > local $Term::ReadLine::termcap_nowarn = 1; > $ret->ornaments(1); > } > return $ret; > } > > sub newTTY { > my ($self, $in, $out) = @_; > $self->[0] = $in; > $self->[1] = $out; > my $sel = select($out); > $| = 1; # for DB::OUT > select($sel); > } > > sub IN { shift->[0] } > sub OUT { shift->[1] } > sub MinLine { undef } > sub Attribs { {} } > > my %features = (tkRunning => 1, ornaments => 1, 'newTTY' => 1); > sub Features { \%features } > > #sub get_line { > # my $self = shift; > # my $in = $self->IN; > # local ($/) = "\n"; > # return scalar <$in>; > #} > > package Term::ReadLine; # So late to allow the above code be defined? > > our $VERSION = '1.07'; > > my ($which) = exists $ENV{PERL_RL} ? split /\s+/, $ENV{PERL_RL} : undef; > if ($which) { > if ($which =~ /\bgnu\b/i){ > eval "use Term::ReadLine::Gnu;"; > } elsif ($which =~ /\bperl\b/i) { > eval "use Term::ReadLine::Perl;"; > } elsif ($which =~ /^(Stub|TermCap|Tk)$/) { > # it is already in memory to avoid false exception as seen in: > # PERL_RL=Stub perl -e'$SIG{__DIE__} = sub { print @_ }; require Term::ReadLine' > } else { > eval "use Term::ReadLine::$which;"; > } > } elsif (defined $which and $which ne '') { # Defined but false > # Do nothing fancy > } else { > eval "use Term::ReadLine::Gnu; 1" or eval "use Term::ReadLine::Perl; 1"; > } > > #require FileHandle; > > # To make possible switch off RL in debugger: (Not needed, work done > # in debugger). > our @ISA; > if (defined &Term::ReadLine::Gnu::readline) { > @ISA = qw(Term::ReadLine::Gnu Term::ReadLine::Stub); > } elsif (defined &Term::ReadLine::Perl::readline) { > @ISA = qw(Term::ReadLine::Perl Term::ReadLine::Stub); > } elsif (defined $which && defined &{"Term::ReadLine::$which\::readline"}) { > @ISA = "Term::ReadLine::$which"; > } else { > @ISA = qw(Term::ReadLine::Stub); > } > > package Term::ReadLine::TermCap; > > # Prompt-start, prompt-end, command-line-start, command-line-end > # -- zero-width beautifies to emit around prompt and the command line. > our @rl_term_set = ("","","",""); > # string encoded: > our $rl_term_set = ',,,'; > > our $terminal; > sub LoadTermCap { > return if defined $terminal; > > require Term::Cap; > $terminal = Tgetent Term::Cap ({OSPEED => 9600}); # Avoid warning. > } > > sub ornaments { > shift; > return $rl_term_set unless @_; > $rl_term_set = shift; > $rl_term_set ||= ',,,'; > $rl_term_set = 'us,ue,md,me' if $rl_term_set eq '1'; > my @ts = split /,/, $rl_term_set, 4; > eval { LoadTermCap }; > unless (defined $terminal) { > warn("Cannot find termcap: $@\n") unless $Term::ReadLine::termcap_nowarn; > $rl_term_set = ',,,'; > return; > } > @rl_term_set = map {$_ ? $terminal->Tputs($_,1) || '' : ''} @ts; > return $rl_term_set; > } > > > package Term::ReadLine::Tk; > > # if AnyEvent is loaded, use it. > use Enbugger; Enbugger->stop; > if (defined &AE::cv) > { > my ($cv, $fe); > > # maintain old name for backward-compatibility > *AE_loop = *Tk_loop = sub { > my $self = shift; > $cv = AE::cv(); > $cv->recv(); > }; > > *register_AE = *register_Tk = sub { > my $self = shift; > $fe ||= AE::io($self->IN, 0, sub { $cv->send() }); > }; > > # just because AE is loaded doesn't mean Tk isn't. > if (not defined &Tk::DoOneEvent) > { > # create the stub as some T::RL implementations still check > # this directly. This should eventually be removed. > *Tk::DoOneEvent = sub { > die "should not happen"; > }; > } > } > else > { > my ($giveup); > > # technically, not AE, but maybe in the future the Tk-specific > # aspects will be removed. > *AE_loop = *Tk_loop = sub { > Tk::DoOneEvent(0) until $giveup; > $giveup = 0; > }; > > *register_AE = *register_Tk = sub { > my $self = shift; > $Term::ReadLine::registered++ > or Tk->fileevent($self->IN,'readable',sub { $giveup = 1}); > }; > > } > > sub tkRunning { > $Term::ReadLine::toloop = $_[1] if @_ > 1; > $Term::ReadLine::toloop; > } > > sub get_c { > my $self = shift; > $self->Tk_loop if $Term::ReadLine::toloop; > return getc $self->IN; > } > > sub get_line { > my $self = shift; > $self->Tk_loop if $Term::ReadLine::toloop; > my $in = $self->IN; > local ($/) = "\n"; > return scalar <$in>; > } > > 1; > diff -rN Term-ReadLine-1.07.orig/t/AE.t Term-ReadLine-1.07/t/AE.t 0a1,33 > #!perl > > use Test::More; > > eval "use AnyEvent; 1" or > plan skip_all => "AnyEvent is not installed."; > > # seeing as the entire point of this test is to test the event handler, > # we need to mock as little as possible. To keep things tightly controlled, > # we'll use the Stub directly. > BEGIN { > $ENV{PERL_RL} = 'Stub o=0'; > } > plan tests => 3; > > # need to delay this so that AE is loaded first. > require Term::ReadLine; > use File::Spec; > > my $t = Term::ReadLine->new('AE'); > ok($t, "Created object"); > is($t->ReadLine, 'Term::ReadLine::Stub', 'Correct type'); > $t->tkRunning(1); > > my $text = 'some text'; > my $T = $text . "\n"; > my $w = AE::timer(0,1,sub { > pass("Event loop called"); > exit 0; > }); > > my $result = $t->readline('Do not press enter>'); > fail("Should not get here."); diff -rN Term-ReadLine-1.07.orig/t/AETk.t Term-ReadLine-1.07/t/AETk.t 0a1,42 > #!perl > > use Test::More; > > eval "use Tk; use AnyEvent; 1" or > plan skip_all => "AnyEvent and/or Tk is not installed."; > > # seeing as the entire point of this test is to test the event handler, > # we need to mock as little as possible. To keep things tightly controlled, > # we'll use the Stub directly. > BEGIN { > $ENV{PERL_RL} = 'Stub o=0'; > # ensure AE uses Tk. > $ENV{PERL_ANYEVENT_MODEL} = 'Tk'; > } > > eval { > use File::Spec; > my $mw = MainWindow->new(); $mw->withdraw(); > 1; > } or plan skip_all => "Tk can't start. DISPLAY not set?"; > > plan tests => 3; > > # need to delay this so that AE is loaded first. > require Term::ReadLine; > use File::Spec; > > my $t = Term::ReadLine->new('AE/Tk'); > ok($t, "Created object"); > is($t->ReadLine, 'Term::ReadLine::Stub', 'Correct type'); > $t->tkRunning(1); > > my $text = 'some text'; > my $T = $text . "\n"; > my $w = AE::timer(0,1,sub { > pass("Event loop called"); > exit 0; > }); > > my $result = $t->readline('Do not press enter>'); > fail("Should not get here."); diff -rN Term-ReadLine-1.07.orig/t/AETk.t~ Term-ReadLine-1.07/t/AETk.t~ 0a1,41 > #!perl > > use Test::More; > > eval "use Tk; use AnyEvent; 1" or > plan skip_all => "AnyEvent and/or Tk is not installed."; > > # seeing as the entire point of this test is to test the event handler, > # we need to mock as little as possible. To keep things tightly controlled, > # we'll use the Stub directly. > BEGIN { > $ENV{PERL_RL} = 'Stub o=0'; > # ensure AE uses Tk. > $ENV{PERL_ANYEVENT_MODEL} = 'Tk'; > } > > eval { > use File::Spec; > my $mw = MainWindow->new(); $mw->withdraw(); > } or plan skip_all => "Tk can't start. DISPLAY not set?"; > > plan tests => 3; > > # need to delay this so that AE is loaded first. > require Term::ReadLine; > use File::Spec; > > my $t = Term::ReadLine->new('AE/Tk'); > ok($t, "Created object"); > is($t->ReadLine, 'Term::ReadLine::Stub', 'Correct type'); > $t->tkRunning(1); > > my $text = 'some text'; > my $T = $text . "\n"; > my $w = AE::timer(0,1,sub { > pass("Event loop called"); > exit 0; > }); > > my $result = $t->readline('Do not press enter>'); > fail("Should not get here."); diff -rN Term-ReadLine-1.07.orig/t/Tk.t Term-ReadLine-1.07/t/Tk.t 0a1,42 > #!perl > > use Test::More; > > eval "use Tk; 1" or > plan skip_all => "Tk is not installed."; > > # seeing as the entire point of this test is to test the event handler, > # we need to mock as little as possible. To keep things tightly controlled, > # we'll use the Stub directly. > BEGIN { > $ENV{PERL_RL} = 'Stub o=0'; > } > > my $mw; > eval { > use File::Spec; > $mw = MainWindow->new(); $mw->withdraw(); > 1; > } or plan skip_all => "Tk can't start. DISPLAY not set?"; > > # need to delay this so that Tk is loaded first. > require Term::ReadLine; > > plan tests => 3; > > my $t = Term::ReadLine->new('Tk'); > ok($t, "Created object"); > is($t->ReadLine, 'Term::ReadLine::Stub', 'Correct type'); > $t->tkRunning(1); > > my $text = 'some text'; > my $T = $text . "\n"; > > my $w = Tk::after($mw,0, > sub { > pass("Event loop called"); > exit 0; > }); > > my $result = $t->readline('Do not press enter>'); > fail("Should not get here."); diff -rN Term-ReadLine-1.07.orig/t/Tk.t~ Term-ReadLine-1.07/t/Tk.t~ 0a1,42 > #!perl > > use Test::More; > > eval "use Tk; 1" or > plan skip_all => "Tk is not installed."; > > # seeing as the entire point of this test is to test the event handler, > # we need to mock as little as possible. To keep things tightly controlled, > # we'll use the Stub directly. > BEGIN { > $ENV{PERL_RL} = 'Stub o=0'; > } > > my $mw; > eval { > use File::Spec; > $mw = MainWindow->new(); $mw->withdraw(); > 1l > } or plan skip_all => "Tk can't start. DISPLAY not set?"; > > # need to delay this so that Tk is loaded first. > require Term::ReadLine; > > plan tests => 3; > > my $t = Term::ReadLine->new('Tk'); > ok($t, "Created object"); > is($t->ReadLine, 'Term::ReadLine::Stub', 'Correct type'); > $t->tkRunning(1); > > my $text = 'some text'; > my $T = $text . "\n"; > > my $w = Tk::after($mw,0, > sub { > pass("Event loop called"); > exit 0; > }); > > my $result = $t->readline('Do not press enter>'); > fail("Should not get here."); ```
p5pRT commented 12 years ago

From @cpansprout

On Sun Jan 29 18​:28​:52 2012\, dmcbride wrote​:

On Sunday January 29 2012 2​:35​:34 PM you wrote​:

With Tk installed\, but no X11 environment running\, I get this​:

couldn't connect to display "/tmp/launch-YF2sgP/org.x​:0" at /Users/sprout/.cpan/build/Tk-804.030- w5ZSVH/blib/lib/Tk/MainWindow.pm line 53. MainWindow->new() at t/Tk.t line 27 # Looks like you planned 3 tests but ran 2. # Looks like your test exited with 2 just after 2.

In case you havenā€™t guessed\, Iā€™m using Mac OS X. Can you detect that

I couldn't guess that\, I'm not rich enough to have a Mac. :-P :-D

error and skip the test? Iā€™ve noticed that Tk passes its own tests with no X11 running. I havenā€™t yet looked to see what it does.

When I unset my DISPLAY and rerun the TRL tests\, I get about the same problem as you. However\, when I do the same thing with Tk\, I get a bunch of ugly output about it :-S

Anyway\, I think I've about patched this up - I'm almost afraid to make my next comment for fear of another horde forming\, but my system upgraded AE in the meantime\, and since AE 6.11\, there has been a "die" at the top of AnyEvent​::Impl​::Tk\, not sure if it was on purpose or not (it didn't show up in the Changelog\, which makes me think not - Marc doesn't seem to be shy about what he does on purpose)\, and that made my tests fail even with DISPLAY set :-)

Apart from that\, your patch looks fine to me.

Here's the updated patch with eval's around the mainwindow functions to see if it loads or not. It does mean that the AETk patch has to explicitly try it\, whereas that shouldn't normally be necessary\, but them's the breaks I guess.

Thank you. Applied as b60dd40238.

--

Father Chrysostomos

p5pRT commented 12 years ago

@cpansprout - Status changed from 'open' to 'resolved'

p5pRT commented 12 years ago

From @cpansprout

On Sun Jan 29 14​:24​:50 2012\, sprout wrote​:

The real irony is that AnyEvent itself abuses Perl similarly by messing with ${^WARNING_BITS}.

I could ā€˜solveā€™ that by putting the same kind of check in mg.c. Hmmm....

--

Father Chrysostomos

p5pRT commented 12 years ago

From CAOgz_5_BE68da5v0gurkoPJojoeSdR2b7aJ-MWuxjnxEUM+opQ@mail.gmail.com

That's a bit another problem. Let's look​: I'm a user of AE and IO​::Async. I don't know about possible problems when I will use them together. So\, from point of user I rather say thanks to Marc for warning me (maybe in a too strict way​: warn would be enough)\, than to Paul for anyevent brokage (possible) without warning.

From the point of view of somebody who -was- an active production user of both\, this isn't what happened.

In fact\, IO​::Async​::Loop​::AnyEvent was written to -avoid- problems.

I had a significant sized AnyEvent application. Since it was already running reliably on its existing Impl\, I did not want to have to switch it to using AnyEvent​::Impl​::IOAsync (which still works imperfectly\, but I don't have good repro cases for the problems I encountered and now never will\, for reasons to be explained).

Thus\, in order to use an IO​::Async component within this process\, I was able to load IO​::Async​::Loop​::AnyEvent\, thereby localising any problems to the new code\, and allowing me to deploy the results to production with confidence.

Then an AnyEvent upgrade killed it.

My client's answer to this was to ban all future use of AnyEvent within their codebase and to authorise a rewrite of the relevant daemon to a pure IO​::Async system\, since they were unwilling to risk that they would again suffer from a situation where Marc intentionally destroyed perfectly working production code over a personal philosophical difference.

As such\, I'm unlikely to ever find out why Impl​::IOAsync doesn't work for that code and I am\, on the whole\, comfortable with this fact.

-- Matt S Trout - Shadowcat Systems - Perl consulting with a commit bit and a clue

http​://shadowcat.co.uk/blog/matt-s-trout/ http​://twitter.com/shadowcat_mst/

Email me now on mst (at) shadowcat.co.uk and let's chat about how our Catalyst commercial support\, training and consultancy packages could help your team.