-
Notifications
You must be signed in to change notification settings - Fork 44
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
feat(body): add entities formatting support #250
Conversation
cdb4644
to
27ee597
Compare
Hi @EdJoPaTo ! Sorry, but I struggle to fix this Can you help me or point me in the right direction? |
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.
I like the simplicity of the change!
I'm not sure about the readonly part (see comment on the TextBody
) but otherwise I like the addition.
(Side note: I will squash the PR into a single commit later on so adding commits to the PR is easier to review than having force pushes which need to be diffed, loosing a bit of the reasoning of the changes while the PR is ongoing)
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.
Can you add a simple test case where some input entity ends up in the sendMessage / sendPhoto method argument just making sure its passed through correctly?
Otherwise its good to merge!
Tests added, I hope they're in line with your standards. |
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.
I like the added FAQ entry. A few nitpicks but I already like the current state of the PR!
Thanks a lot for your feedback, I updated the documentation |
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.
Wanted to hit the approve and merge, but I have more nitpicks… Sorry for that! 😇
Hahaha! Don't hesitate! |
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.
I'll merge and release later when I have more time, so I don't introduce mistakes from trying to get it done in a hurry.
Thank you for the addition! It's released as 9.1.0 ✨ |
Nice! |
Hi !
This pull request adds the support to entities on text messages sent.
This allows:
FormattedString
objects created with grammyjs/parse-mode.Example