hirooih / perl-trg

Term::ReadLine::Gnu (TRG) is an implementation of Term::ReadLine Perl module, and is the interface to the GNU Readline/History Library. This Perl module gives you input line editing facility, input history management facility, word completion facility, etc.
https://metacpan.org/release/Term-ReadLine-Gnu
5 stars 3 forks source link

creating a second Term::ReadLine::Gnu leads to confusion #16

Closed rjbs closed 1 year ago

rjbs commented 1 year ago

Describe the bug

Creating multiple Term::ReadLine instances, when Gnu is used, will call "Term::ReadLine::Gnu->new" more than once. This leads to problems. The same object is re-used, and partly reinitialized, but parts of the attributes from any previous configuration are kept.

This seems unavoidable: there can be only one readline instance anyway.

I have a program that moves between various activities, and each one got its own ReadLine. This led to problems with registering too many custom functions, because the same instance kept being used. (Gnu.xs:rl_add_defun: custom function table is full.)

Term::ReadLine::Perl refuses to construct a second instance, presumably for similar reasons. If new is called twice, I believe Gnu should at least warn. If it doesn't refuse (and create a Stub instead), I also think you should add a Term::ReadLine::Gnu->has_been_initialized method, which could check whether %Attribs was blessed.

I have worked around this problem by changing my application framework.

I see now that my production environment is out of date! I'm using v1.37. I will attempt to reproduce this on the latest Term::ReadLine::Gnu, but it may be difficult. I apologize if this report is redundant.

hirooih commented 1 year ago

Thank you for your report. Sorry for my late response, because I has been out of town.

This seems unavoidable: there can be only one readline instance anyway.

Yes. The GNU Readline Library has many global variables. From the design the library and its perl wrapper module Term::ReadLine::Gnu cannot support multiple instances.

If new is called twice, I believe Gnu should at least warn.

I will add this check on the next release.

This led to problems with registering too many custom functions, because the same instance kept being used. (Gnu.xs:rl_add_defun: custom function table is full.)

This is another issue described in the BUGS section of the manual (https://github.com/hirooih/perl-trg/blob/master/Gnu.pm#L2287). The current implementation uses fixed size tables. I thought 16 was enough. But I found I was wrong apparently. I will try to fix this as well.

rjbs commented 1 year ago

Thanks for your reply. I think the "check for being already initialized" is a good and sufficient change.

I (for one person) have not yet needed more than 16 entries in that table. It only came up because I was continually making new ReadLine objects. Now that I save, reconfigure, and reuse one, everything is fine.

hirooih commented 1 year ago

Fixed by the PR #20. It will be included by the next release.


The current implementation uses fixed size tables. I thought 16 was enough. But I found I was wrong apparently. I will try to fix this as well.

I could not find any way to support flexible size of table for custom functions. If anyone has a good way, please contact me.

hirooih commented 1 year ago

I've released Term::ReadLine::Gnu 1.46 including this PR #20. Thank you for your contribution.