davidchambers / tutor

JavaScript interface for the Gatherer card database
https://gatherer.wizards.com/
Do What The F*ck You Want To Public License
149 stars 18 forks source link

Update check for "No Results" when searching by name #95

Closed CBXZero closed 9 years ago

CBXZero commented 9 years ago

This should fix issue #93 that I opened. I check now for the div that encapsulates the full sized image (which is only found on the page when a single card is returned).

Also, sorry for the 2nd pull request. Messed up my first attempt at fixing it. Tested it extensively in my bot this time.

CBXZero commented 9 years ago

Just realized my mistake here, So the cardImage div won't work, looking to see if there's a better thing to use to fix this.

davidchambers commented 9 years ago

So the cardImage div won't work, looking to see if there's a better thing to use to fix this.

Okay. Let me know if you discover a solution. :)

CBXZero commented 9 years ago

There we go, I got it figured out and I also wrote a test around it (haven't used coffeescript or mocha, so it took me a bit to figure out).

The issue ended up being that if you use a fun word like "test" as the name, then it would return multiple cards back (like here http://gatherer.wizards.com/Pages/Search/Default.aspx?name=+[test]). I decided to also check for the "Additional Sort:" text that appears on these pages to weed them out from providing false positives.

CBXZero commented 9 years ago

It look like my machine did some edits to the gatherer.js file when it updated coffeescript, do you want me to redo it with the old version of coffeescript, or is it alright?

davidchambers commented 9 years ago

You should run make setup to install dependencies, then make fixtures to generate the fixtures, then make test to run the unit tests.

You should update the CoffeeScript files rather than the JavaScript files (see #92, for example). You shouldn't use your globally installed coffee to generate the JavaScript files – simply run make.

All this talk of Make assumes you're on OS X or some flavour of Linux. Is this the case?

CBXZero commented 9 years ago

Yeah, I'm running ubuntu of some variant on my laptop, I'll give that a shot, thanks :)

CBXZero commented 9 years ago

Fixed the coffeescript file and all the tests passed, and make setup made life about 10 times easier. Are there any other finer details I may be missing?

davidchambers commented 9 years ago

This is looking good. Please remove the lib changes from the pull request. The generated files are automatically updated as part of the release process, so needn't be updated with each commit.

Also, please squash the commits. :)

CBXZero commented 9 years ago

somehow I just blew those up, let me fix whatever I have going on...

CBXZero commented 9 years ago

Somehow when I attempted to squash, I blew up everything in my branch and then some, I'm going to start fresh taking the notes that we have here and fixing this real quick. If I make another pull request for that I apologize in advance, first time contributing outside of team assignments at school.

CBXZero commented 9 years ago

Looks like it didn't want to update when I purged and redid this, So I'll have to move this into a new request, apologies for my Version Control mishap!

davidchambers commented 9 years ago

Superseded by #96