Skip to content

Preserve options#437

Merged
matthewmueller merged 12 commits into
masterfrom
preserve_options
Apr 8, 2014
Merged

Preserve options#437
matthewmueller merged 12 commits into
masterfrom
preserve_options

Conversation

@fb55

@fb55 fb55 commented Apr 8, 2014

Copy link
Copy Markdown
Member

Fixes #273, #256.
Necessary for #261, #248, #275

This is intended to start some progress towards maintaining an options object. evaluate now always receives an options object, but there still seem to be some bugs - the newly-added test-cases are failing (not anymore).

As this contains a lot of changes, I previously merged all PRs that seemed reasonable to me.

I tried to include everything discussed in #274. If I forgot something, please mention it. Also, feel free to send PRs to the preserve_options branch :)

Torthu and others added 9 commits September 19, 2013 17:12
Cheerio.prototype.options are now overwritten if Cheerio is
instantiated with an options object.

cheerioinstace.options is now passed to parser when adding new dom
nodes to an existing Cheerio instance.
as proposed by @jugglinmike in #274, but using `this.options` instead
of the prototype to enable `Cheerio.call` calls to work properly
fb55 added 2 commits April 8, 2014 16:02
caused failure of test cases (htmlparser2's behavior depends on the
existence of the property)

as a reminder for future optimizations, I've added the `decodeEntities`
option instead
I'm not sure the behavior is the same (it's now equal to the code above)
@stevenvachon

Copy link
Copy Markdown
Contributor

+1

@fb55

fb55 commented Apr 8, 2014

Copy link
Copy Markdown
Member Author

As far as I can tell, this should be feature-complete. Much less of a rewrite than originally expected.

@matthewmueller

Copy link
Copy Markdown
Member

hmm, is it possible to get away with just using this.options and using that inside the parser and renderer? It seems like there's a ton of option objects being passed around and in the future it'll be easy to miss it when we're pulling in PRs.

@fb55

fb55 commented Apr 8, 2014

Copy link
Copy Markdown
Member Author

Moving makeDomArray to cheerio's prototype will remove much of the passing in manipulation.js. That leaves some calls in the constructor, which won't get worse.

@matthewmueller

Copy link
Copy Markdown
Member

yah, i think we should attach it to the prototype then

@fb55

fb55 commented Apr 8, 2014

Copy link
Copy Markdown
Member Author

@matthewmueller done

matthewmueller added a commit that referenced this pull request Apr 8, 2014
@matthewmueller matthewmueller merged commit e478de2 into master Apr 8, 2014
@matthewmueller matthewmueller deleted the preserve_options branch April 8, 2014 16:38
@matthewmueller

Copy link
Copy Markdown
Member

@fb55 thanks for fixing this awful bug, haha.

@davidchambers

Copy link
Copy Markdown
Contributor

🎉 🍰

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.

Cheerio does not keep/use options from instantiation

5 participants