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

NFTDescriptor: escape quotes #104

Merged
merged 5 commits into from Apr 22, 2021
Merged

NFTDescriptor: escape quotes #104

merged 5 commits into from Apr 22, 2021

Conversation

ewilz
Copy link
Member

@ewilz ewilz commented Apr 21, 2021

closes #56

struct DecimalStringParams {
// significant figures of decimal
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

prettier's gone rogue...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

think i ran into this once before, look for an issue in the solidity prettier plugin repo

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ugh, I'm so sorry about this. I just opened a PR that fixes this: prettier-solidity/prettier-plugin-solidity#480 I'll try to have it merged and released today.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, Franco, that was so quick!! thank you! cc @fvictorio

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be fixed in prettier-plugin-solidity@1.0.0-beta.10. Let us know if it's not!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yess, that did the trick 💯 happy to be able to throw you weird edgecases as you guys get closer to a stable release 😂

@ewilz ewilz mentioned this pull request Apr 21, 2021
4 tasks
@@ -113,43 +113,35 @@ library NFTDescriptor {
}

function formatTokenSymbol(string memory symbol) internal pure returns (string memory) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems this method should be more like escapeQuotes(string) returns (string) and could use some fuzz testing (it may not work right multibyte characters, not sure)

@ewilz ewilz merged commit 8b4ab2c into main Apr 22, 2021
@ewilz ewilz deleted the quotes branch April 22, 2021 16:15
@PaulRBerg
Copy link

PaulRBerg commented May 28, 2023

Interesting edge case!

I'm curious: how many ERC-20 tokens out there have quotes in their symbols (is there any particular example you could share)? Shouldn't this be very rare?

ChatGPT failed to find any example of such a token. However, I managed to find one with an apostrophe ('), i.e. JPEG.

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

Successfully merging this pull request may close these issues.

NFT Metadata improvements
5 participants