Skip to content

bpo-29755: Fixed the lgettext() family of functions in the gettext module.#2266

Merged
serhiy-storchaka merged 4 commits intopython:masterfrom
serhiy-storchaka:lgettext
Jun 20, 2017
Merged

bpo-29755: Fixed the lgettext() family of functions in the gettext module.#2266
serhiy-storchaka merged 4 commits intopython:masterfrom
serhiy-storchaka:lgettext

Conversation

@serhiy-storchaka
Copy link
Member

They now always return bytes.

@mention-bot
Copy link

@serhiy-storchaka, thanks for your PR! By analyzing the history of the files in this pull request, we identified @birkenfeld, @warsaw and @loewis to be potential reviewers.

@warsaw
Copy link
Member

warsaw commented Jun 18, 2017

LGTM, and thanks for submitting this! Some thoughts:

  • I think the docs should explicitly discourage the l*() methods. I agree with your comment in the bpo-29755 that there's probably almost no utility for them in Python 3. I'm not sure we should go as far as deprecate them, but we should strong steer people clear of using them in favor of gettext() and Unicodes all the way down.
  • These methods should document that exceptions are now possible if the strings can't be encoded to bytes in the given codeset.

Copy link
Member

@warsaw warsaw left a comment

Choose a reason for hiding this comment

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

A couple of additional suggestions were given, but I know you'll DTRT and what you have is already good, so I'll approve it in advance.

@serhiy-storchaka
Copy link
Member Author

I'm interesting in your suggestions. In any case I'm going to rewrite the docs (group the l*() functions for avoiding duplications).

@warsaw
Copy link
Member

warsaw commented Jun 19, 2017

Grouping the l* methods sounds like a great idea. I'd probably just put a reST admonition at the top of that section. Draft text:

.. warning:
   These `l*()` methods should be avoided in Python 3, because they return encoded bytes.
   It's much better to use alternatives which return Unicode strings instead, since most Python
   applications will want to manipulate human readable text as strings instead of bytes.  Further,
   it's possible that you may get unexpected Unicode-related exceptions if there are encoding
   problems with the translated strings.  It is possible that the `l*()` methods will be deprecated
   in future Python versions due to their inherent problems and limitations.

@serhiy-storchaka
Copy link
Member Author

Thank you for your review @warsaw. Please look at the updated documentation since I'm not sure about my English.

@warsaw
Copy link
Member

warsaw commented Jun 20, 2017

Looks great, thanks!

@serhiy-storchaka serhiy-storchaka merged commit 26cb465 into python:master Jun 20, 2017
@serhiy-storchaka serhiy-storchaka deleted the lgettext branch June 20, 2017 14:13
serhiy-storchaka added a commit to serhiy-storchaka/cpython that referenced this pull request Jun 20, 2017
…ext module. (pythonGH-2266)

They now always return bytes.

Updated the gettext documentation..
(cherry picked from commit 26cb465)
@bedevere-bot
Copy link

GH-2297 is a backport of this pull request to the 3.6 branch.

serhiy-storchaka added a commit to serhiy-storchaka/cpython that referenced this pull request Jun 20, 2017
…ext module. (pythonGH-2266)

They now always return bytes.

Updated the gettext documentation.
(cherry picked from commit 26cb465)
@bedevere-bot
Copy link

GH-2298 is a backport of this pull request to the 3.5 branch.

serhiy-storchaka added a commit that referenced this pull request Jun 20, 2017
…ext module. (GH-2266) (#2297)

They now always return bytes.

Updated the gettext documentation..
(cherry picked from commit 26cb465)
serhiy-storchaka added a commit that referenced this pull request Jun 20, 2017
…ext module. (GH-2266) (#2298)

They now always return bytes.

Updated the gettext documentation.
(cherry picked from commit 26cb465)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type-bug An unexpected behavior, bug, or error

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants