From: Hugh Sasse <hgs@...>
Date: 2010-01-26T19:15:51+09:00
Subject: [ruby-core:27849] Re: [Bug #2656] Inconsistent docs for Zlib.

Excuse me following up to myself, but there's a first cut of
the patch below...

On Tue, 26 Jan 2010, Hugh Sasse wrote:

> On Tue, 26 Jan 2010, Yui NARUSE wrote:
> 
> > Issue #2656 has been updated by Yui NARUSE.
> > 
        [...]
> > > Both GzipReader and GzipWriter inherit from GzipFile which:
> > 
> > Yes, so Zlib::GzipWriter.wrap and Zlib::GzipReader are inherited methods
> > of Zlib::GzipFile.wrap.
> > 
> > > This looks to me as if the structure have changed, and maybe GzipFile#wrap
> > > just raised an exception in the past, to create an abstract method. I've
> > > not checked earlier versions to see.  However, for the superclass to 
> > > refer to the subclasses for documentation seems odd.
> > 
> > Those documents are confusing but they are only typo.
> > Things didn't change.
> 
> OK, I thought that was possible, too. 
> > 
> > If you create a patch for Ruby's trunk, I'll merge it.
> > Now this ticket move to Ruby 1.9.
> 
> OK, I'll see what I can do.  Do you just want me to (effectively)
> s/Gzip(Read|Writ)er#wrap/GzipFile#wrap/
> for these cases?  Is that the desired fix?
> > 
> > P.S.
> > We recommend:
> > * a problem is in trunk (unstable trunk/branch), the patch should be for trunk
> > * a problem isn't in trunk but in ruby_1_8 (stable branch), the patch should be for ruby_1_8
> > * a problem is only in release branch, the patch should be for the branch
> > If you test some releases please write them, and we can remember to backport to the release branch.
> 
> OK, I'll explore the structure in a little more detail, then, so I'm
> working on the correct part.  Thank you.

Checked http://www.ruby-lang.org/en/community/ruby-core/#patching-ruby
and I have a checkout of trunk, it seems, in a dir called ruby.

My patch against that is like this at the moment:


Index: ext/zlib/zlib.c
===================================================================
--- ext/zlib/zlib.c	(revision 26420)
+++ ext/zlib/zlib.c	(working copy)
@@ -2420,7 +2420,12 @@
 }
 
 /*
- * See Zlib::GzipReader#wrap and Zlib::GzipWriter#wrap.
+ * Creates a GzipFile object associated with ((|io|)), and
+ * executes the block with the newly created GzipFile object,
+ * just like File::open. The GzipFile object will be closed
+ * automatically after executing the block. If you want to keep
+ * the associated IO object opening, you may call
+ * ((<Zlib::GzipFile#finish>)) method in the block.
  */
 static VALUE
 rb_gzfile_s_wrap(int argc, VALUE *argv, VALUE klass)
@@ -2785,7 +2790,7 @@
  *
  * Opens a file specified by +filename+ for writing gzip compressed data, and
  * returns a GzipWriter object associated with that file.  Further details of
- * this method are found in Zlib::GzipWriter.new and Zlib::GzipWriter#wrap.
+ * this method are found in Zlib::GzipWriter.new and Zlib::GzipFile#wrap.
  */
 static VALUE
 rb_gzwriter_s_open(int argc, VALUE *argv, VALUE klass)
@@ -2985,7 +2990,7 @@
  *
  * Opens a file specified by +filename+ as a gzipped file, and returns a
  * GzipReader object associated with that file.  Further details of this method
- * are in Zlib::GzipReader.new and ZLib::GzipReader.wrap.
+ * are in Zlib::GzipReader.new and ZLib::GzipFile.wrap.
  */
 static VALUE
 rb_gzreader_s_open(int argc, VALUE *argv, VALUE klass)
Index: ext/zlib/doc/zlib.rd
===================================================================
--- ext/zlib/doc/zlib.rd	(revision 26420)
+++ ext/zlib/doc/zlib.rd	(working copy)
@@ -485,7 +485,12 @@
 
 --- Zlib::GzipFile.wrap(args...) {|gz| ... }
 
-    See ((<Zlib::GzipReader.wrap>)) and ((<Zlib::GzipWriter.wrap>)).
+    Creates a GzipFile object associated with ((|io|)), and
+    executes the block with the newly created GzipFile object,
+    just like File::open. The GzipFile object will be closed
+    automatically after executing the block. If you want to keep
+    the associated IO object opening, you may call
+    ((<Zlib::GzipFile#finish>)) method in the block.
 
 --- Zlib::GzipFile.open(args...) {|gz| ... }
 


And this means in the rd document I have left the references in to
GzipReader#wrap and GzipWriter#wrap.  The case for doing that is
that the object passed into the block will not be the superclass of
these, I think, which seems to be what this code is saying:

    VALUE obj = rb_class_new_instance(argc, argv, klass);

    if (rb_block_given_p()) {
          return rb_ensure(rb_yield, obj, gzfile_ensure_close, obj) ;
    }

That is, the GzipReader#wrap will yield a GzipReader, not a GzipFile.
In the rdocs, I've just left the user to figure out the inheritance.

Is that what you had in mind for the right fix, or am I at cross-purposes?

        Thank you,
        Hugh