perl5
perl5 copied to clipboard
perldebug: clarify the readline support
Also I don't see any traces that Term::ReadKey is used (rg ReadKey /usr/share/perl5 /usr/lib/perl5). I'm running perl-5.36.0 under Linux. And it doesn't seem like installing it makes any difference. Is it needed only under Windows and for it to get used you need Term::ReadLine::Perl5? But the former is a dependency of the latter, so that should be handled automatically. Which means under Windows Term::ReadLine::Perl5 is preferred over Term::ReadLine::Gnu?
In other words a couple of corrections might be needed. Another one: Perl -> Perl5.
Also I don't see any traces that
Term::ReadKeyis used (rg ReadKey /usr/share/perl5 /usr/lib/perl5). I'm runningperl-5.36.0under Linux. And it doesn't seem like installing it makes any difference. Is it needed only under Windows and for it to get used you needTerm::ReadLine::Perl5? But the former is a dependency of the latter, so that should be handled automatically. Which means under WindowsTerm::ReadLine::Perl5is preferred overTerm::ReadLine::Gnu?In other words a couple of corrections might be needed. Another one: Perl -> Perl5.
I'm confused by your comment. If Term::ReadKey is indeed not used, then why did you not delete the mention of it in your patch?
Also, is this pull request at all related to the issue you filed today: https://github.com/Perl/perl5/issues/20307 ?
I'm confused by your comment. If Term::ReadKey is indeed not used, then why did you not delete the mention of it in your patch?
Well, I made a change. But than a couple of thoughts came to mind. And while I'm certain about the change, I'm not sure about the rest:
-
Term::ReadKeymight be needed only on Windows:Version 2.30.01: Added handling of arrows, page up/down, home/end, insert/delete keys under Win32. These keys emit xterm-compatible sequences. Works with Term::ReadLine::Perl.
In that case for it to be used you need
Term::ReadLine::Perl5. ButTerm::ReadLine::Perl5depends onTerm::ReadKeyanyway. As such the latter doesn't need to be mentioned. Unless I'm missing something. -
Which makes me think, if
Term::ReadLine::Perl5was created to overcome issues withTerm::ReadLine::Gnuon Windows. Another possibility to improve the docs. -
Shouldn't
Term::ReadLine::Perlbe changed toTerm::ReadLine::Perl5?Term::ReadLine::Perlseems to exist, but old and maybe abandoned.
I think we could discuss here if any of the above assumptions are true and should be implemented in this PR. Or I'm open to your other suggestions.
Also, is this pull request at all related to the issue you filed today: https://github.com/Perl/perl5/issues/20307?
No, not related. I don't have a solution for that one in mind.
Term::ReadLine::Perl (no 5) uses Term::ReadKey to read the terminal size, falling back to an ioctl(), the 5 version always uses Term::ReadKey to read the terminal size.
Shouldn't Term::ReadLine::Perl be changed to Term::ReadLine::Perl5? Term::ReadLine::Perl seems to exist, but old and maybe abandoned.
The plain Perl version is passing on CPAN testers, and on many platforms, perhaps the author doesn't think it needs an update.
I think the change adds a little clarity (you don't need all of the Term::RL::* modules, just one).
@x-yuri, more than a year has elapsed since there last was activity in this pull request. No one has spoken up in favor of this request, so I'm going to label the ticket 'Closable?'. I am self-assigning it for the purpose of closing it within 1 or 2 weeks unless there is a strong argument for keeping it open.
I think the change as is is an improvement.
Installing Term::Key is desirable so I think leaving the suggestion to install it is fine.
I think this should be merged. @tonycoz spoke up in favor of it, but @jkeenan says no one has. I don't understand
I think this should be merged. @tonycoz spoke up in favor of it, but @jkeenan says no one has. I don't understand
@khwilliamson, in his Oct 18 comment @tonycoz expressed some doubts about the patch. Subsequently I reviewed the ticket, did not find any unambiguous support for the patch and applied the "Closable?" label. Tony then clarified that he was in favor of the patch. At that point I removed the "Closable?" label. So, if both you and Tony support the patch, you can merge.