search

ChrisWren / grunt-link-checker

Run node-simple-crawler to discover broken links on your website
MIT License
33 stars 9 forks source link

Correct bug when checking for ids with dots #32

Closed OXINARF closed 8 years ago

OXINARF commented 8 years ago

If an id has dots (for example fig4.5.1) then cheerio doesn't work correctly. This makes the crawler fail although the anchor does exist.

greggman commented 8 years ago

Oh, haha. Doh! I just posted a fix for the same issue. I suppose I should have checked. Maybe you like the other PR

OXINARF commented 8 years ago

@greggman I like that your PR adds the dot tests, but your way to fix it isn't the appropriate way according to the discussion in cheerio that I already posted above (https://github.com/cheeriojs/cheerio/issues/115)

greggman commented 8 years ago

Sorry, I'm not understanding why my PR doesn't work according to that discussion. Escaping the periods works in jquery, in querySelector, and in cheerio. But in any case whatever fixes the issue would be great

OXINARF commented 8 years ago

It does work, but it seems to be because of the way an internal library works (which means that a change to the library can break your fix): https://github.com/cheeriojs/cheerio/issues/115#issuecomment-9629782 If you read all the discussion the advised way is to use something like [id="foo.bar"], which is what my fix does.

Anyway, this repo doesn't seem to be updated anymore...

OXINARF commented 8 years ago

So the PR with the wrong way to fix the problem was merged, without even a word in this previous one.

Thanks @ChrisWren! I won't certainly open any other PR on anything you're involved.