Azoy / Sword

Discord library for Swift
https://azoy.github.io/Sword
MIT License
179 stars 52 forks source link

addReaction, deleteReaction, and getReaction percent encode emoji if necessary #23

Closed TellowKrinkle closed 7 years ago

TellowKrinkle commented 7 years ago

The strings used by those functions need to be URL encoded to work, but it would be confusing (or was for me at least) to find that message.addReaction("šŸ”µ") fails. This way forces them to create an Emoji object using Emoji("šŸ”µ") which will get converted to a properly URL-encoded tag before the request is run.

Azoy commented 7 years ago

Why not just keep the string argument, and use the new emoji initializer to get the tag?

TellowKrinkle commented 7 years ago

To me at least, the string argument makes it look like you could just put an emoji there (which you can't, since the emoji won't be URL encoded and will make a bad URL). Since you'll always need to put an emoji through an emoji object to URL encode it anyways, you might as well have the function take an emoji object to have one less step for the person using the API while also making it less confusing.

Azoy commented 7 years ago

What Iā€™m saying is, make it possible to put an emoji in the string argument. Initialize an emoji object from the string, and pass the tag to the request.

TellowKrinkle commented 7 years ago

So make addReaction take a string and then internally convert it to an emoji and get its tag?

Azoy commented 7 years ago

Bingo, and for custom emojis they can just pass emoji.tag. Check if the string is a tag vs a Unicode emoji, and initialize an emoji for Unicode.

TellowKrinkle commented 7 years ago

How's this?