Bug #2656: Inconsistent docs for Zlib. [ruby-core:27692]

• Status: Closed
• Assignee: -
• Target version: -
• ruby -v: ruby 1.8.7 (2008-08-11 patchlevel 72) [i386-cygwin]

[hgs (Hugh Sasse)]

=begin
(Putting this in redmine so it will be found)

http://ruby-doc.org/stdlib/libdoc/zlib/rdoc/classes/Zlib/GzipWriter.html

has
Zlib::GzipWriter.open(filename, level=nil, strategy=nil) { |gz| ... }

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. 

However, there is no wrap method documented on that page.
Similarly,

http://ruby-doc.org/stdlib/libdoc/zlib/rdoc/classes/Zlib/GzipReader.html

has:
Zlib::GzipReader.open(filename) {|gz| ... }

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. 

Again, there is no wrap method documented on that page.

Both GzipReader and GzipWriter inherit from GzipFile which:

http://ruby-doc.org/stdlib/libdoc/zlib/rdoc/classes/Zlib/GzipFile.html

does have a wrap method documented, but it says:
wrap(...)

See Zlib::GzipReader#wrap and Zlib::GzipWriter#wrap.

which, as we have shown, do not have this documented or even override it.

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.

I have found this:

ruby-1.8.7-p173/ext/zlib/doc/zlib.rd

which has:

--- Zlib::GzipFile.wrap(args...) {|gz| ... }

 See ((<Zlib::GzipReader.wrap>)) and ((<Zlib::GzipWriter.wrap>)).

and

--- Zlib::GzipReader.wrap(io) {|gz| ... }

 Creates a GzipReader object associated with ((|io|)), and
 executes the block with the newly created GzipReader object,
 just like File::open. The GzipReader 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.

and

--- Zlib::GzipWriter.wrap(io[, level[, strategy]]) {|gz| ... }

 Creates a GzipWriter object associated with ((|io|)), and
 executes the block with the newly created GzipWriter object,
 just like File::open. The GzipWriter 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.

which looks rather like duplication, and might be better if the
description were moved up into GzipFile

I don't mind creating a patch to fix this, but maybe there are good
reasons why this came about, and a better way to fix it, so that the
docs really do end up in the subclasses. But since the C source
doesn't have the rb_define_method calls to create the wrap methods
in the subclasses, I can't see this being picked up correctly by
Rdoc from the subclasses.

Does anyone have any wisdom to share about this before I create a
patch to move the descriptions up GzipFile, and thereby combine them?

     Hugh

=end

[hgs (Hugh Sasse)]

=begin
Forgot to confirm this is still true for ruby_1_8. I have now checked that and it is still true.
=end

[zenspider (Ryan Davis)]

=begin

On Jan 25, 2010, at 11:38 , Hugh Sasse wrote:

Bug [ruby-core:27692] (Closed)" href="/issues/2656">#2656: Inconsistent docs for Zlib. [ruby-core:27692]
http://redmine.ruby-lang.org/issues/show/2656

Please get Hugh a commit bit. He's beyond awesome with patches.

=end

[hgs (Hugh Sasse)]

=begin
On Tue, 26 Jan 2010, Ryan Davis wrote:

On Jan 25, 2010, at 11:38 , Hugh Sasse wrote:

Bug [ruby-core:27692] (Closed)" href="/issues/2656">#2656: Inconsistent docs for Zlib. [ruby-core:27692]
http://redmine.ruby-lang.org/issues/show/2656

Please get Hugh a commit bit. He's beyond awesome with patches.

I very much appreciate the compliment, but I'm not sure I'm ready
to leave my fingerprints all over the core. Things like this
present point need discussion, and community agreement. I could
probably be persuaded, though, because I try to be careful with
other people's work.

     Thank you,
     Hugh

=end

[zenspider (Ryan Davis)]

=begin

On Jan 25, 2010, at 13:07 , Hugh Sasse wrote:

On Tue, 26 Jan 2010, Ryan Davis wrote:

On Jan 25, 2010, at 11:38 , Hugh Sasse wrote:

Bug [ruby-core:27692] (Closed)" href="/issues/2656">#2656: Inconsistent docs for Zlib. [ruby-core:27692]
http://redmine.ruby-lang.org/issues/show/2656

Please get Hugh a commit bit. He's beyond awesome with patches.

I very much appreciate the compliment, but I'm not sure I'm ready
to leave my fingerprints all over the core. Things like this
present point need discussion, and community agreement. I could
probably be persuaded, though, because I try to be careful with
other people's work.

Yeah whatever. :P

core: Ignore him and get him a commit bit! Force it on him if you have to!

=end

[hgs (Hugh Sasse)]

=begin

On Tue, 26 Jan 2010, Ryan Davis wrote:

On Jan 25, 2010, at 13:07 , Hugh Sasse wrote:

On Tue, 26 Jan 2010, Ryan Davis wrote:

On Jan 25, 2010, at 11:38 , Hugh Sasse wrote:

Bug [ruby-core:27692] (Closed)" href="/issues/2656">#2656: Inconsistent docs for Zlib. [ruby-core:27692]
http://redmine.ruby-lang.org/issues/show/2656

Please get Hugh a commit bit. He's beyond awesome with patches.

I very much appreciate the compliment, but I'm not sure I'm ready
to leave my fingerprints all over the core. Things like this
present point need discussion, and community agreement. I could
probably be persuaded, though, because I try to be careful with
other people's work.

Yeah whatever. :P

core: Ignore him and get him a commit bit! Force it on him if you have to!

:-) !!

     Hugh

=end

[naruse (Yui NARUSE)]

• Category set to lib

=begin

Zlib::GzipWriter.open(filename, level=nil, strategy=nil) { |gz| ... }
(snip)
and Zlib::GzipWriter#wrap.
However, there is no wrap method documented on that page.

This typo of Zlib::GzipWriter.wrap.

Zlib::GzipReader.open(filename) {|gz| ... }
(snip)
ZLib::GzipReader.wrap.
Again, there is no wrap method documented on that page.

This is correct.

GzipFile.wrap(...)
See Zlib::GzipReader#wrap and Zlib::GzipWriter#wrap.

This is typo of Zlib::GzipReader.wrap and Zlib::GzipWriter.wrap.
Zlib::GzipReader#wrap and Zlib::GzipWriter#wrap are't exist.

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.

If you create a patch for Ruby's trunk, I'll merge it.
Now this ticket move to Ruby 1.9.

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.
  =end

[hgs (Hugh Sasse)]

=begin

On Tue, 26 Jan 2010, Yui NARUSE wrote:

Issue [ruby-core:27692] (Closed)" href="/issues/2656">#2656 has been updated by Yui NARUSE.

Category set to lib

Zlib::GzipWriter.open(filename, level=nil, strategy=nil) { |gz| ... }
(snip)
and Zlib::GzipWriter#wrap.
However, there is no wrap method documented on that page.

This typo of Zlib::GzipWriter.wrap.

Zlib::GzipReader.open(filename) {|gz| ... }
(snip)
ZLib::GzipReader.wrap.
Again, there is no wrap method documented on that page.

This is correct.

GzipFile.wrap(...)
See Zlib::GzipReader#wrap and Zlib::GzipWriter#wrap.

This is typo of Zlib::GzipReader.wrap and Zlib::GzipWriter.wrap.
Zlib::GzipReader#wrap and Zlib::GzipWriter#wrap are't exist.

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.

     Hugh

=end

[hgs (Hugh Sasse)]

=begin
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 [ruby-core:27692] (Closed)" href="/issues/2656">#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

=end

[naruse (Yui NARUSE)]

• Status changed from Open to Closed

=begin
I merged your doc, thanks.

rd document is thought as obsoleted and removed.

Your point, GzipFile.wrap is actually used as GzipReader.wrap,
seems rdoc's structural problem.
Just left seems correct.
=end

[hgs (Hugh Sasse)]

=begin

On Tue, 26 Jan 2010, Yui NARUSE wrote:

Issue [ruby-core:27692] (Closed)" href="/issues/2656">#2656 has been updated by Yui NARUSE.

Status changed from Open to Closed

I merged your doc, thanks.

Thank you.

rd document is thought as obsoleted and removed.

I've missed some option off svn update to get that reflected in my
copy

Your point, GzipFile.wrap is actually used as GzipReader.wrap,
seems rdoc's structural problem.
Just left seems correct.

Is the following change, to push this more into rdoc style, the
right thing to do? Sorry I forgot this detail before!

Index: ext/zlib/zlib.c

--- ext/zlib/zlib.c (revision 26425)
+++ ext/zlib/zlib.c (working copy)
@@ -2420,12 +2420,14 @@
}

/*

• • Creates a GzipFile object associated with ((|io|)), and

• • call-seq: Zlib::GzipFile.wrap(io) { |gz| ... }
• • 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#finish+ method in the block.
    */
    static VALUE
    rb_gzfile_s_wrap(int argc, VALUE *argv, VALUE klass)

    Thank you,
    Hugh
    

=end