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

Fix in telegram.Message #1042

Merged
merged 2 commits into from Mar 16, 2018

Conversation

Projects
None yet
2 participants
@Eldinnie
Copy link
Member

Eldinnie commented Mar 12, 2018

The _parse_(html|markdown) methods now properly return None on an empty Message.text or an empty Message.caption

Fix in telegram.Message
The `_parse_(html|markdown)` methods now properly return `None` on an empty `Message.text` or an empty `Message.caption`
@jh0ker

This comment has been minimized.

Copy link
Member

jh0ker commented on tests/test_message.py in 345f637 Mar 12, 2018

Maybe change this to assert message.text_html is None ? Same for below

def test_text_html_empty(self, message):
message.text = None
message.caption = "test"
assert not message.text_html

This comment has been minimized.

@jh0ker

jh0ker Mar 12, 2018

Member

Perhaps change to assert message.text_html is None ?

This comment has been minimized.

@jh0ker

jh0ker Mar 12, 2018

Member

same for the other tests of course

This comment has been minimized.

@Eldinnie

Eldinnie Mar 12, 2018

Author Member

I could, but why?

This comment has been minimized.

@jh0ker

jh0ker Mar 12, 2018

Member

Because it would properly test the feature. We don't want to allow empty strings, right?

This comment has been minimized.

@Eldinnie

Eldinnie Mar 12, 2018

Author Member

I don't see why not, actually I was think of doing:

if not message_text:
  return message_text
@jh0ker

jh0ker approved these changes Mar 12, 2018

@jh0ker jh0ker merged commit e182046 into master Mar 16, 2018

5 checks passed

codecov/patch 100% of diff hit (target 92.48%)
Details
codecov/project Absolute coverage decreased by -0.18% but relative coverage increased by +7.51% compared to 2b221da
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
hound No violations found. Woof!

@Eldinnie Eldinnie deleted the Fix_parse_html-markdown branch Apr 17, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.