FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

add `caption`, `parse_mode`, `caption_entities`, `disable_web_page_preview` parameters to `Bot._send_message` by lemontree210 · Pull Request #3385 · python-telegram-bot/python-telegram-bot · GitHub

/ python-telegram-bot Public

add caption, parse_mode, caption_entities, disable_web_page_preview parameters to Bot._send_message - #3385

Merged
Bibo-Joshi merged 7 commits into
masterfrom
caption-params-for-send-message-3372
Nov 26, 2022
ConversationCommits (7)ChecksFiles changed
Merged

add caption, parse_mode, caption_entities, disable_web_page_preview parameters to Bot._send_message#3385
Bibo-Joshi merged 7 commits into
masterfrom
caption-params-for-send-message-3372

Conversation

lemontree210 commented Nov 24, 2022 •
edited
Loading

Copy link
Copy Markdown
Member

closes #3372

Not sure I need to add tests (I think in this case I just needed to make sure existing tests don't fail).

lemontree210 added the 🛠 refactor change type: refactor label Nov 24, 2022
lemontree210 self-assigned this Nov 24, 2022
Bibo-Joshi requested changes Nov 24, 2022
View reviewed changes
Comment thread telegram/_bot.py Outdated
refactor: add docstring to ._send_message
a2370bc 
closes #3372
add doctstring and move comment there
lemontree210 added enhancement and removed 🛠 refactor change type: refactor labels Nov 24, 2022
harshil21 reviewed Nov 24, 2022
View reviewed changes

harshil21 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

copy_message and edit_message_caption still have those if clauses left to remove.

Comment thread telegram/_bot.py Outdated
Bibo-Joshi reviewed Nov 24, 2022
View reviewed changes
Comment thread telegram/_bot.py Outdated

lemontree210 commented Nov 24, 2022

Copy link
Copy Markdown
Member Author

copy_message and edit_message_caption still have those if clauses left to remove.

Yes, I agree on edit_message_caption(), but copy_message() calls Bot._post(), not Bot._send_message(), so not sure this change is applicable for copy_message()

lemontree210 commented Nov 25, 2022

Copy link
Copy Markdown
Member Author

BTW a (maybe too) philosophical question. Can the fact that the generic ._send_message() method knows about other methods having parameters such as .caption (which pertain not to any message but to a specific type of message) be considered as breaking LoD? Maybe it's OK in this context because all of these params are part of official Telegram API.

Bibo-Joshi commented Nov 25, 2022

Copy link
Copy Markdown
Member

fact that the generic ._send_message() method knows about other methods having parameters such as .caption

IMO this is not the case. _send_message just offers that parameter, but it doesn't really know if any other method actually uses it.

add parse_mode to args of Bot._send_message()
be5cfef 
change all methods except for `.copy_message()`,
which doesn't call `._send_message()`

lemontree210 commented Nov 25, 2022

Copy link
Copy Markdown
Member Author

Added parse_mode as mentioned in this comment for #3372

lemontree210 changed the title refactor: add caption, caption_entities parameters to Bot._send_message refactor: add caption, parse_mode, caption_entities parameters to Bot._send_message Nov 25, 2022
lemontree210 changed the title refactor: add caption, parse_mode, caption_entities parameters to Bot._send_message add caption, parse_mode, caption_entities parameters to Bot._send_message Nov 25, 2022
Bibo-Joshi reviewed Nov 25, 2022
View reviewed changes
Comment thread telegram/_bot.py Outdated
lemontree210 changed the title add caption, parse_mode, caption_entities parameters to Bot._send_message add caption, parse_mode, caption_entities, disable_web_page_preview parameters to Bot._send_message Nov 25, 2022
Bibo-Joshi approved these changes Nov 25, 2022
View reviewed changes

Bibo-Joshi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM except for the comment below :)

Comment thread telegram/ext/_extbot.py
Bibo-Joshi merged commit 637cc57 into master Nov 26, 2022
Bibo-Joshi deleted the caption-params-for-send-message-3372 branch November 26, 2022 18:05
github-actions Bot locked and limited conversation to collaborators Dec 4, 2022
Bibo-Joshi added 🔌 enhancement pr description: enhancement and removed enhancement labels Nov 3, 2024
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

Bibo-Joshi Bibo-Joshi approved these changes
harshil21 harshil21 approved these changes

Assignees

lemontree210

Labels

🔌 enhancement pr description: enhancement

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add caption, caption_entities as parameters to Bot._send_message

3 participants

Back | FazBrowse Home | New Git URL