Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix a bug #8

Closed
wants to merge 2 commits into from
Closed

fix a bug #8

wants to merge 2 commits into from

Conversation

ice1000
Copy link

@ice1000 ice1000 commented Apr 18, 2017

@@ -111,7 +111,7 @@ def __conv_item_keyonly_opts(id, keys)
keys2
end

def itemconfig_hash_kv(id, keys, enc_mode = nil, conf = nil)
def itemconfig_hash_kv(id, keys, enc_mode = [], conf = )
Copy link
Member

Choose a reason for hiding this comment

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

Why conf parameter is removed?

Copy link
Author

Choose a reason for hiding this comment

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

Well that's a mistake, I'm sorry. It should be like

def itemconfig_hash_kv(id, keys, enc_mode = [], conf = [])

@rhenium
Copy link
Member

rhenium commented Apr 19, 2017

I believe this is fixed differently by r54575 (ruby/ruby@0318d35).

@ice1000
Copy link
Author

ice1000 commented Apr 19, 2017

@rhenium So why there's such a bug like this?
I believe that before this bug was fixed, you cannot even run the samples.

@rhenium
Copy link
Member

rhenium commented Apr 19, 2017

@ice1000 Because it was fixed after Ruby 2.3 release and the patch didn't make into ruby_2_3 branch.

@rhenium
Copy link
Member

rhenium commented Apr 19, 2017

I created a backport request at bugs.ruby-lang.org: https://bugs.ruby-lang.org/issues/13484

I'm closing this PR since I could confirm the code on the SO question successfully runs with tk gem 0.1.2.

Side note: if you use Ruby 2.3 and want to use the tk gem, you need to explicitly activate the gem by calling gem "tk" before require "tk" in order to prevent loading the standard library one (which still has the bug).

@rhenium rhenium closed this Apr 19, 2017
@ice1000
Copy link
Author

ice1000 commented Apr 19, 2017

Thank you

@ice1000 ice1000 deleted the patch-1 branch April 19, 2017 17:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants