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

Refactor SaxHandler for clarity #32

Merged
merged 2 commits into from
Jun 4, 2012

Conversation

gaffneyc
Copy link
Contributor

@gaffneyc gaffneyc commented Jun 4, 2012

The overall goal was to make the code easier to grok while I was
tracking down the issue with elements having the wrong encoding.

* Tried to reduce the number of instances of respond_to? in the code by
  pulling common uses of it out to methods.
* The parse stack is now composed of simple objects instead of it being
  an array of arrays.
* Now using an identifier for an empty buffer instead of empty string.
* Cleaned up several variables that were not being used.
* Encapsulated stack so it's not being exposed as part of the API.
* #cdata_block is now an alias instead of delegating to characters.

This includes and depends on issue #31, sorry for any duplication.

gaffneyc added 2 commits June 3, 2012 19:45
String.new creates a string with an encoding of ASCII-8BIT which was
causing the elements to also have that encoding. This change uses a
duplicate of the string provided from Nokogiri, which is in the correct
encoding, as the buffer for the element's value.
The overall goal was to make the code easier to grok while I was
tracking down the issue with elements having the wrong encoding.

* Tried to reduce the number of instances of respond_to? in the code by
  pulling common uses of it out to methods.
* The parse stack is now composed of simple objects instead of it being
  an array of arrays.
* Now using an identifier for an empty buffer instead of empty string.
* Cleaned up several variables that were not being used.
* Encapsulated @stack so it's not being exposed as part of the API.
* #cdata_block is now an alias instead of delegating to characters.
ezkl added a commit that referenced this pull request Jun 4, 2012
@ezkl ezkl merged commit 9973288 into pauldix:master Jun 4, 2012
ezkl added a commit that referenced this pull request Jun 4, 2012
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