Skip to content

Print default in help when option type is :boolean and default is false#849

Merged
rafaelfranca merged 2 commits into
rails:mainfrom
nevesenin:bool_false_default
Jun 20, 2023
Merged

Print default in help when option type is :boolean and default is false#849
rafaelfranca merged 2 commits into
rails:mainfrom
nevesenin:bool_false_default

Conversation

@nevesenin

Copy link
Copy Markdown
Contributor

🌈 I just learned, that there is an open issue #819 already, which will be solved by this PR.

Comment thread lib/thor/parser/option.rb
Comment on lines +107 to +109
def show_default?
super || [TrueClass, FalseClass].any? { |c| default.is_a?(c) }
end

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
def show_default?
super || [TrueClass, FalseClass].any? { |c| default.is_a?(c) }
end
def show_default?
case default
when TrueClass, FalseClass
true
else
super
end
end

Comment thread spec/parser/option_spec.rb Outdated

describe "#print_default" do
it "prints boolean with true default value" do
expect(option(:foo, {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
expect(option(:foo, {
expect(option(:foo, {

Comment thread spec/parser/option_spec.rb Outdated
}).print_default).to eq(true)
end
it "prints boolean with false default value" do
expect(option(:foo, {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
expect(option(:foo, {
expect(option(:foo, {

@rafaelfranca rafaelfranca merged commit 5fb6206 into rails:main Jun 20, 2023
@nevesenin nevesenin deleted the bool_false_default branch July 6, 2023 08:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants