Project

General

Profile

Bug #10928

optparse Switch#summarize code doesn't reflect its documentation

Added by gettalong (Thomas Leitner) over 4 years ago. Updated 8 days ago.

Status:
Closed
Priority:
Normal
Target version:
-
ruby -v:
ruby 2.2.0p0 (2014-12-25 revision 49005) [x86_64-linux]
[ruby-core:68378]

Description

The documentation for Switch#summarize says "+sdone+:: Already summarized short style options keyed hash." for the sdone argument (similar problem with ldone). I.e. it mentions it should be a Hash.

However, the actual method definition line shows otherwise, namely an Array.

def summarize(sdone = [], ldone = [], width = 1, max = width - 1, indent = "")

The OptionParser#summarize command does actually invoke it with Hashes as arguments (line 566): visit(:summarize, {}, {}, width, max, indent, &blk).

So the method definition for Switch#summarize is probably false.


Files

optparse-switch-summarize.patch (1.19 KB) optparse-switch-summarize.patch jeremyevans0 (Jeremy Evans), 07/07/2019 08:56 PM

Associated revisions

Revision 3fcffcea
Added by jeremyevans (Jeremy Evans) 8 days ago

Fix default argument values for OptParse::Switch#summarize

The documentation describes these arguments being hashes, and the method
is called with hashes, so a hash default makes more sense.

The method would fail previously if called without arguments and @short
or @long contained a non-integer value.

Fixes [Bug #10928]

History

Updated by davydov_anton (Anton Davydov) about 4 years ago

Hello,

However, the actual method definition line shows otherwise, namely an Array.

In documentations says that sdone and ldone options take keyed hash. It's mean that this options take array with hash keys.

The OptionParser#summarize command does actually invoke it with Hashes as arguments (line 566): visit(:summarize, {}, {}, width, max, indent, &blk).

It's different methods. In first case it's Switch#summarize and in second case it's OptionParser#summarize.

Updated by jeremyevans0 (Jeremy Evans) 8 days ago

I agree that this is a bug and it should be fixed. You can trigger it by calling OptionParser::Switch#summarize without arguments:

require 'optparse'
o = OptionParser.new
o.on('-c'){}
o.instance_variable_get(:@stack)[2].instance_variable_get(:@short).values.first.summarize{}
# TypeError (no implicit conversion of String into Integer)

Attached is a patch that fixes the issue.

Updated by nobu (Nobuyoshi Nakada) 8 days ago

Thank you, I've missed it.
Commit it please.

#4

Updated by jeremyevans (Jeremy Evans) 8 days ago

  • Status changed from Assigned to Closed

Applied in changeset git|3fcffceafd2bce7186851bf4899484c545a9ace8.


Fix default argument values for OptParse::Switch#summarize

The documentation describes these arguments being hashes, and the method
is called with hashes, so a hash default makes more sense.

The method would fail previously if called without arguments and @short
or @long contained a non-integer value.

Fixes [Bug #10928]

Also available in: Atom PDF