Add t.me links for User, Chat and Message if available and update User.mention_* by jonowo · Pull Request #1092 · python-telegram-bot/python-telegram-bot

jonowo · 2018-04-27T15:33:59Z

User.link (t.me link of a user if the user has a username)
Chat.link (t.me link of a supergroup if it has a username)
Message.link (t.me link of a message if it is in a supergroup with a username)

Updated User.mention_markdown and User.mention_html so that the default value of name is User.full_name instead of User.name. If a user has a username, User.name returns the user's username prefixed with "@". It makes no sense to markdown a username, since the username already mentions the user.

Add User.link and update User.mention_*

Add Chat.link

Add Message.link

jsmnbom

I like the idea a lot! Not sure how the other feel yet though. Please see attached comments though.

jsmnbom · 2018-04-30T11:13:00Z

telegram/chat.py

+        :attr:`username`, returns a t.me link of the chat.
+        """
+        if self.type == "supergroup" and self.username:
+            return "t.me/{}".format(self.usermame)


Should this default to something? (if it's not a supergroup)

jsmnbom · 2018-04-30T11:13:03Z

telegram/chat.py

+        :obj:`str`: Convenience property. If the chat is a supergroup and has a 
+        :attr:`username`, returns a t.me link of the chat.
+        """
+        if self.type == "supergroup" and self.username:


Please use the Chat.SUPERGROUP attribute here.
(comment applies elsewhere too)

jsmnbom · 2018-04-30T11:15:12Z

telegram/user.py

-        if not name:
-            return util_mention_markdown(self.id, self.name)
-        else:
+        if name:


This change is out of scope... we're more likely to merge simple self contained pull requests...
IMO this is fine this time though :)

Will separate into 2 PRs next time.

Eldinnie

Basically the changes look good. Some small comments on docstrings. However tests are missing. Can you please modify/change the tests so they won't fail and diff coverage is 100%
After that we can merge

Eldinnie · 2018-04-30T14:20:45Z

telegram/user.py

        """
        Shortcut for::

-                bot.get_user_profile_photos(update.message.from_user.id, *args, **kwargs)


Revert this indent please. Otherwise docs will build weirdly

Eldinnie · 2018-04-30T14:22:03Z

telegram/user.py

        Args:
-            name (:obj:`str`): If provided, will overwrite the user's name.
-
+            name (:obj:`str`): If provided, will overwrite the user's full name.


Add the blank line between the headers again. Without it docs will mess up.

Eldinnie · 2018-04-30T14:22:20Z

telegram/user.py

        Args:
-            name (:obj:`str`): If provided, will overwrite the user's name.
-
+            name (:obj:`str`): If provided, will overwrite the user's full name.


Add the blank line between the headers again. Without it docs will mess up.

jonowo · 2018-04-30T15:53:51Z

Fixed docstrings. Now working on tests.

tsnoam · 2018-05-07T22:28:51Z

@Tr-Jono
what is the status of this?

jonowo · 2018-05-07T23:57:54Z

Modified tests for user and chat. Will work on test_message.py later.

jonowo · 2018-05-08T23:54:39Z

Fixed mistakes, waiting for the checks. I could only work on mobile...

…p for its link, message links should be available for channels too, fix typos, add more tests for links, confirmed tests passed

Fixed and added more tests, minor change on chat/message link functions

fix docstrings blank lines

jonowo · 2018-05-09T05:33:03Z

@jeffffc helped fixed the tests & docstrings, and include links for channels. Should be fine now. ☺

Eldinnie · 2018-05-09T06:06:28Z

Hey @Tr-Jono . Did you install the pre-commit hook like explained in our Contributing guide

Flake8 found a problem which makes CI fail:

telegram/user.py:152:1: W293 blank line contains whitespace

jeffffc · 2018-05-09T06:46:08Z

It was my bad, missed the pre-commit line when I set up a new venv. Will push again a while later, sorry for the silly mistakes we made.

Eldinnie · 2018-05-09T06:46:53Z

No problem :D

fix docstrings whitespaces in blank lines

jeffffc · 2018-05-09T08:42:04Z

Good to go!

Some docstring formatting. Removed some redundant tests

Eldinnie · 2018-05-09T09:14:09Z

i pushed some last fixes and it's good to go with that IMO. The patch fix being 75% is due to missing tests for mention_html and `mention_markdown in the first place. Will fix that in another PR.

Eldinnie · 2018-05-09T09:25:08Z

@Tr-Jono one last thing. If you like you can add yourself to the contributers list.
Just commit or reply here with the entry you want in there and I'll add it when I merge.

EDIT:
NVm you're already on there :D

Eldinnie · 2018-05-09T09:42:22Z

Thank you for your contribution

…r.mention_* (#1092) * Add User.link and update User.mention_* * Add Chat.link * Add Message.link * Link returns None on default * Add test link

jonowo added 9 commits April 27, 2018 22:59

Add User.link and update User.mention_*

54ea342

Add Chat.link

0f4f695

Add Message.link

466eaa2

Merge pull request #1 from Tr-Jono/Tr-Jono-patch-1

7986c74

Add User.link and update User.mention_*

Tiny modification

820c0e2

Merge pull request #2 from Tr-Jono/Tr-Jono-patch-1-1

5069b0e

Add Chat.link

Merge pull request #3 from Tr-Jono/Tr-Jono-patch-1-2

b584bba

Add Message.link

Fixed nub mistakes I made

80a140e

Fixed mistakes I made when I was fixing mistakes

aca6eda

jsmnbom requested changes Apr 30, 2018

View reviewed changes

jsmnbom added the 📋 pending-reply work status: pending-reply label Apr 30, 2018

jonowo added 6 commits April 30, 2018 11:29

Link returns None on default

ed52057

Link returns None on default

af8e909

Link returns None on default

27cbdbf

Update chat.py

e18d076

Fixed link and it returns None on default

a2247b6

Ooooooops

aff64c3

Eldinnie requested changes Apr 30, 2018

View reviewed changes

jonowo added 4 commits April 30, 2018 22:39

Reverted Indent

2016e75

Update user.py

5d45047

Update message.py

41f697e

Add the blank lines

1d622fe

jonowo added 5 commits May 8, 2018 07:19

Add https

dfae43b

Update chat.py

38a1c12

Add https

86df641

Add test link

3db087a

Add test link

c844562

jonowo added 5 commits May 8, 2018 10:22

Mistakes fixed

3915bc5

Update test_chat.py

c5e8576

Fixed "trailing whitespace"

6a49537

Fixed "line too long"

f0c45fc

Fixed whitespace mistakes

03aa149

jonowo closed this May 8, 2018

jonowo reopened this May 8, 2018

jeffffc and others added 4 commits May 9, 2018 12:21

Chat could be private/channel too so should not force check supergrou…

0689616

…p for its link, message links should be available for channels too, fix typos, add more tests for links, confirmed tests passed

Merge pull request #4 from jeffffc/jono2

9f336db

Fixed and added more tests, minor change on chat/message link functions

fix docstrings blank lines

a8d8ac0

Merge pull request #5 from jeffffc/jono2

90d31ea

fix docstrings blank lines

fix docstrings whitespaces in blank lines

c0a254e

Merge pull request #6 from jeffffc/jono2

e2bc04b

fix docstrings whitespaces in blank lines

Eldinnie added 2 commits May 9, 2018 10:55

Merge branch 'master' into 1092master

ac13a76

Some small fixes

e2b4424

Some docstring formatting. Removed some redundant tests

Eldinnie approved these changes May 9, 2018

View reviewed changes

jsmnbom approved these changes May 9, 2018

View reviewed changes

Eldinnie merged commit 94abf16 into python-telegram-bot:master May 9, 2018

github-actions bot locked and limited conversation to collaborators Aug 21, 2020

Conversation

jonowo commented Apr 27, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jsmnbom left a comment

Choose a reason for hiding this comment

Uh oh!

jsmnbom Apr 30, 2018

Choose a reason for hiding this comment

Uh oh!

jsmnbom Apr 30, 2018

Choose a reason for hiding this comment

Uh oh!

jsmnbom Apr 30, 2018

Choose a reason for hiding this comment

Uh oh!

jonowo Apr 30, 2018

Choose a reason for hiding this comment

Uh oh!

Eldinnie left a comment

Choose a reason for hiding this comment

Uh oh!

Eldinnie Apr 30, 2018

Choose a reason for hiding this comment

Uh oh!

Eldinnie Apr 30, 2018

Choose a reason for hiding this comment

Uh oh!

Eldinnie Apr 30, 2018

Choose a reason for hiding this comment

Uh oh!

jonowo commented Apr 30, 2018

Uh oh!

tsnoam commented May 7, 2018

Uh oh!

jonowo commented May 7, 2018

Uh oh!

jonowo commented May 8, 2018

Uh oh!

jonowo commented May 9, 2018

Uh oh!

Eldinnie commented May 9, 2018

Uh oh!

jeffffc commented May 9, 2018

Uh oh!

Eldinnie commented May 9, 2018

Uh oh!

jeffffc commented May 9, 2018

Uh oh!

Eldinnie commented May 9, 2018

Uh oh!

Eldinnie commented May 9, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Eldinnie commented May 9, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

jonowo commented Apr 27, 2018 •

edited

Loading

Eldinnie commented May 9, 2018 •

edited

Loading