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

Block parameters for Parser.new and Encoder.new #139

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

padde
Copy link

@padde padde commented Jun 19, 2014

Just wanted to sprinkle some syntactic sugar on the Parser and Encoder classes. Whereas now you would have to write

parser = Yajl::Parser.new
parser.on_parse_complete = lambda { |hash|
  # do something with hash
}

and

encoder = Yajl::Encoder.new
encoder.on_progress_callback = lambda { |str|
  # do something with str
}

this branch allows you to write

parser = Yajl::Parser.new do |hash|
  # do something with hash
end

and

encoder = Yajl::Encoder.new do |str|
  # do something with str
end

While I was at it, I

  • fixed some typos and indentation in the comments
  • found and fixed a bug that rendered on_progress_callback practically useless

The bug occurs because Encoder#encode takes a block parameter which sets on_progress_callback in the background. However, later on the block is called directly instead of referring to the on_progress_callback. This means that setting on_progress_callback before calling encode had no effect. This probably went unnoticed because there was no spec for that case but there is now. Check out padde@9ab456e.

Please comment on my branch and even if you don't like the block parameters, consider cherry-picking the other fixes.

Cheers

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.

None yet

1 participant