-
Notifications
You must be signed in to change notification settings - Fork 112
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
fix(ts-wrappers): fix TypeScript wrappers generation for messages with single quote #1106
fix(ts-wrappers): fix TypeScript wrappers generation for messages with single quote #1106
Conversation
…h single quote Fixes tact-lang#972
Thank you kindly for the contribution!
That would be nice, but there are no immediate plans (ok, now there are: #1107). We have a good-to-have plans to make the wrapper generation system a bit more flexible to allow making wrappers for multiple languages, but those are a bit far-fetched now that the team is focusing more efforts on many other areas of the compiler. @i582 Could you please add an entry to the CHANGELOG.md? |
@novusnota done |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, but let's wait for the input from @verytactical, @anton-trunov and others :)
@i582 Hi, thanks for the contribution. The original PR template had some pointers on our PR merging policy and it requires adding tests to make sure this bug won't creep in later after some refactoring, for instance. |
…s-with-single-quote
Yeah, I saw that, that's why I asked about tests. However, since there are no tests for wrappers yet (as I understand), what tests could I add, @anton-trunov? |
@i582 you can use the example from the linked issue, e.g. by including an apostrophe into one of the string receiver's in the end-to-end tests (see |
Thank you, @anton-trunov! I added test for different messages |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Hey, this particular bug is not that important, of course, but it's still a great option to dig into the project :)
I haven't found any test cases for TS wrappers. Does Tact plan to create (or have) one?
Fixes #972