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

Support <cards> and <cs> #12

Closed
NilsEnevoldsen opened this issue Nov 15, 2017 · 13 comments
Closed

Support <cards> and <cs> #12

NilsEnevoldsen opened this issue Nov 15, 2017 · 13 comments

Comments

@NilsEnevoldsen
Copy link
Owner

NilsEnevoldsen commented Nov 15, 2017

For backwards compatibility. Not sure what expected behavior is, tbh. Maybe we don't need to support this?

https://mtg.gamepedia.com/Help:Card_tags

@applehat
Copy link
Contributor

#21 resolves this.

@NilsEnevoldsen
Copy link
Owner Author

Hm, why isn't this working on the test wiki? Probably a dumb mistake I'm making.

@applehat
Copy link
Contributor

Intend use case is something like this:

<cs>
Black Lotus
Ancestral Recall
</cs>

@NilsEnevoldsen
Copy link
Owner Author

Right, but it's not working here? http://ec2-34-226-122-61.compute-1.amazonaws.com/Time_Spiral

@applehat
Copy link
Contributor

Did you merge in my changes from #21?

screen shot 2018-03-13 at 4 41 55 pm

Its working on my local wiki running my branch.

@applehat
Copy link
Contributor

Now that Im looking over the code for MTGSCards, I think my PR may not be 100% correct intended functionality either. Let me go look over my PR again.

@NilsEnevoldsen
Copy link
Owner Author

Oh, I found my dumb mistake. I was updating the extension in the wrong way. It works now. But I'll wait to merge until you give me the go-ahead.

@NilsEnevoldsen
Copy link
Owner Author

Can you also check that you're not adding an extra line break at the end?

When you're ready, I'll merge your PR, fix some of my own Grunt warnings, bump the version, and ask Scryfall (again) for a final green light, and then we should be good to go as far as I'm concerned.

@applehat
Copy link
Contributor

Updated.

There is some edge-case stuff that may not be covered still -- but the MTGSCards extension is so old and undocumented that its sort of hard to figure out exactly what some stuff is trying to accomplish.

I think this is a good enough implementation to cover 99% of use cases.

@NilsEnevoldsen
Copy link
Owner Author

Closed by #21.

@NilsEnevoldsen
Copy link
Owner Author

You have the green light from Scryfall, @applehat. Do you have a Discord handle?

@NilsEnevoldsen
Copy link
Owner Author

Acutally, Scryfall asks that I change the utm_source parameter for this deployment. I'll do that now.

@NilsEnevoldsen
Copy link
Owner Author

And that's done in 1.0.1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants