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
Add caption_(html|markdown)(_urled)? #1013
Conversation
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 it how you used
from html import escape
and gaining fromfuture
.
However, inhelpers.py
we still use the approach. Maybe fix there too? - I had some small nitpicks on the code (look on the comments).
- Other than that, looks good :)
telegram/message.py
Outdated
entities = self.parse_entities() | ||
message_text = self.text | ||
def _parse_html(self, text, entities, urled=False): | ||
message_text = text |
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.
Why not get message_text
as one of the function arguments?
/me asking because you never use text
but to initialize message_text
; afterwards at the first line of the for
loop you override text
.
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.
This is where I was being an idiot with double named parameters before. My fix is bad too :D. Will change
telegram/message.py
Outdated
return self._parse_html(self.caption, self.parse_caption_entities(), urled=True) | ||
|
||
def _parse_markdown(self, text, entities, urled=False): | ||
message_text = text |
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.
Why not get message_text
as one of the function arguments?
/me asking because you never use text
but to initialize message_text
; afterwards at the first line of the for
loop you override text
.
telegram/message.py
Outdated
""" | ||
return self._parse_html(self.caption, self.parse_caption_entities(), urled=True) | ||
|
||
def _parse_markdown(self, text, entities, urled=False): |
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 change this to be @staticmethod
?
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.
Why?
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.
as discussed, because this method needs nothing from the instance which makes it a good enough reason to be static.
telegram/message.py
Outdated
def _text_html(self, urled=False): | ||
entities = self.parse_entities() | ||
message_text = self.text | ||
def _parse_html(self, text, entities, urled=False): |
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 change this to be @staticmethod
?
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.
why again?
Closes #1010