Skip to content

Convenience full_name method for User - #949

Merged
tsnoam merged 1 commit into
python-telegram-bot:masterfrom
graynk:user-full-name
Dec 30, 2017
Merged

Convenience full_name method for User#949
tsnoam merged 1 commit into
python-telegram-bot:masterfrom
graynk:user-full-name

Conversation

@graynk

@graynk graynk commented Dec 24, 2017

Copy link
Copy Markdown
Contributor

Returns full name (first + last) if last name is present or just the first name if not present.

@robert-cody

Copy link
Copy Markdown

In many locales format should be "surname name" (which is BTW more useful for sorting), so it's better to make locale as incoming optional parameter and return full_name depending on locale.

@codecov

codecov Bot commented Dec 24, 2017

Copy link
Copy Markdown

Codecov Report

Merging #949 into master will decrease coverage by 0.11%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #949      +/-   ##
==========================================
- Coverage    91.8%   91.69%   -0.12%     
==========================================
  Files         103      103              
  Lines        4040     4044       +4     
  Branches      638      639       +1     
==========================================
- Hits         3709     3708       -1     
- Misses        193      197       +4     
- Partials      138      139       +1
Impacted Files Coverage Δ
telegram/user.py 86.95% <100%> (+1.24%) ⬆️
telegram/utils/request.py 66.96% <0%> (-0.9%) ⬇️
telegram/bot.py 87.51% <0%> (-0.5%) ⬇️

@graynk

graynk commented Dec 24, 2017

Copy link
Copy Markdown
Contributor Author

I don't think this information is in python locale module though?
It seems to have been in CLDR at some point, but not anymore, if I understand it correctly. In any case, I view this as just a simpler version of User.name method that works exactly the same way, only ensuring that returned names are consistent in style (I feel that mentioning some users via "@username" and others via "name" is unintuitive and doesn't look all that well in group chats).

@robert-cody

Copy link
Copy Markdown

@graynk you're right, for some strange reason, there is no such info in Python's locale module.

@Eldinnie Eldinnie self-assigned this Dec 24, 2017
@Eldinnie Eldinnie added the 📋 pending-merge work status: pending-merge label Dec 24, 2017
@Eldinnie

Copy link
Copy Markdown
Member

Looks good to me. Closes #943

@tsnoam

tsnoam commented Dec 30, 2017

Copy link
Copy Markdown
Member

thank you for your contribution

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

📋 pending-merge work status: pending-merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants