[ruby-core:29141] [Feature #1473] Improvements on expect.rb

Issue #1473 has been updated by Yusuke Endoh.

Category changed from build to ext
Assigned to set to Yusuke Endoh

Hi,

> I was using expect.rb to substitute expect command (as I'm not in a mood to learn Tcl). However, it demands some attention.
>
> I took expect(1) command as a idea, but not a rule, of how expect should behave.

Thank you for your writing a patch, and sorry for late response.


> First, a little bug that incorrect pattern classes are not treated correctly. It would result in a "e_pat unspecified" which gives no clues of what happened. Now I raise an exception.

Generally, standard library may raise unexpected error when argument
type is unexpected.  It is the fate of duck typing.  See #2495.

But in this case, I think it can be considered as a bug because it
explicitly dispatches on the type.


> I let the another one parameter, timeout, unchecked cause the error messagem like "String can't be coerced into Fixnum" would give a good hint of what is wrong.

I think this is a fate of duck typing.


> I changed pat and e_pat to pattern to meet some other similar parameter references.

I can't get your point.
What do you mean "some other similar parameter reference" ?


> Back to the timeout parameter, timeout was not a global timeout but a single char timeout. If the io keeps giving input, it never stops. For example: While using expect.rb to interact with a dlink switch telnet, if it generates any log information on the terminal (as it does constantly like "link down") before the timeout time, even if my expression never occurs, the timeout is never over. I changed this to use timeout as a global timeout as expect command does.

Hmm.  One might expect the behavior you expected, but other might
expect the original behavior, I think.
I cannot determine, but at least, we should not delete the original
behavior in terms of compatibility.


> Keeping on timeout stuff, if it read some input and times out, where this buffer goes? In expect.rb, it is just lost. For the expect command, this buffer can be used in the next matching. I use a instance variable to keep and reuse the unused buffer on the next expect call. 

It is reasonable for me, though I'm not sure whether or not your
implementation (buffering @unusedBuf) has no issue...


> I changed the default timeout value to meet the expect command default one. If someone really wants to use 9999999 seconds, just go ahead ans specify it on parameters. However, interactive scripts generally deal with unexpected behaviors. 30s, as expect command uses, seems to be reasonable.

Rejected for the compatibility reason.


> I also added some documentation that was missing.

Great, thanks.


I'll commit the following patch and close the ticket.
If there is objection, I'm happy to revert it.


diff --git a/ext/pty/lib/expect.rb b/ext/pty/lib/expect.rb
index 08191b0..156b024 100644
--- a/ext/pty/lib/expect.rb
+++ b/ext/pty/lib/expect.rb
@@ -1,6 +1,12 @@
 $expect_verbose = false
 
 class IO
+  # Reads ios until pattern matches or the timeout is over. It returns
+  # an array with the read buffer, followed by the matches. If a block is given,
+  # the result is yielded to the block and returns nil. The optional timeout parameter defines,
+  # in seconds, the total time to wait for pattern. If it is over of eof is found, it 
+  # returns/yields nil. However, the buffer in a timeout session is kept for the next expect call.
+  # The default timeout is 9999999 seconds.
   def expect(pat,timeout=9999999)
     buf = ''
     case pat
@@ -8,13 +14,20 @@ class IO
       e_pat = Regexp.new(Regexp.quote(pat))
     when Regexp
       e_pat = pat
+    else
+      raise ArgumentError, "unsupported pattern class: #{pattern.class}"
     end
+    @unusedBuf ||= ''
     while true
-      if !IO.select([self],nil,nil,timeout) or eof? then
+      if not @unusedBuf.empty?
+        c = @unusedBuf.slice!(0).chr
+      elsif !IO.select([self],nil,nil,timeout) or eof? then
         result = nil
+        @unusedBuf = buf
         break
+      else
+        c = getc.chr
       end
-      c = getc.chr
       buf << c
       if $expect_verbose
         STDOUT.print c

-- 
Yusuke Endoh <[e-mail]>
----------------------------------------
http://redmine.ruby-lang.org/issues/show/1473

----------------------------------------
http://redmine.ruby-lang.org