ktty1220 / cheerio-httpcli

iconvによる文字コード変換とcheerioによるHTMLパースを組み込んだNode.js用HTTPクライアントモジュール
MIT License
262 stars 28 forks source link

HTMLエンティティデコードはclient.loadOptionsで指定できるようにする #9

Closed ikeyan closed 8 years ago

ikeyan commented 8 years ago

今まで、cheerio.loadに { decodeEntities: false } をわたし、cheerio.prototype.{text, html}でHTMLエンティティをデコードしていたため、cheerio.prototype.{attr,data}などではHTMLエンティティがデコードされていませんでした。

このPRでは、 client.loadOptions を追加し、初期値を { decodeEntities: true } として、これを cheerio.load に渡すようにしたため、cheerioの全メソッドでデコード済みの値が取得できるようになり、独自の {text, html} プロトタイプ拡張が不要になります。

マージよろしくお願いします!

ktty1220 commented 8 years ago

PRありがとうございます。

decodeEntitiesをfalseにしているのはちょっとした経緯がありまして、 以前はdecodeEntitiesの指定をしていなかったのですが(結果的にcheerioデフォルトのtrueでパース)、それでパースした$.html()メソッドを実行すると、全部の文字がエンティティ化された状態で返ってきてしまうことが分かりました。

※今回のikeyanさんのPRをマージした状態でexample/google.jsの28行目と30行目の.text().html()にするとそのようになっているのを確認できると思います。

なので、cheerioのdecodeEntitiesは求めているものと違う => decodeEntitiesをfalseにした、という流れがあります。

その副作用として、元からエンティティで書かれている部分をcheerioのtext()html()で取得するとエンティティのまま返ってきてしまうので、その対策として{text,html}のプロトタイプ拡張を仕込んでいる、といった感じです。

なので、今回のPRは一旦見送らせてもらいます。スミマセン:pensive:

それとは別で、{attr,data}などにエンティティが入っているとそのまま取得してしまうのは{text,html}と同じような対策が必要ですが、各メソッド毎にエンティティデコードをかます拡張を仕込むのも何かアレなので、こちらについてはちょっと検討させてください(cheerioでパースする前にbodyに対してエンティティデコードするとか、decodeEntitiesをtrueにしておかしくなるのがhtml()だけなら、decodeEntitiesをfalseにするのはやめて.html()のcheerio拡張は残しておく、とか)。