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

Addressing problems raised in sphinx_issues #10104 #61

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

hoangduytranuk
Copy link

Added sort_output=False and sort_by_message_location=False so these flags can be controlled by users when dumping the po files manually and making use of the internal sort_output (sort the msgid alphabetically when writing to file) or sort_by_message_location (internally known as sort_by_file)

Copy link
Member

@shimizukawa shimizukawa left a comment

Choose a reason for hiding this comment

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

@hoangduytranuk please check my comment and update docstring (or code-comment).

@@ -37,7 +37,7 @@ def dump_po(filename, catalog, line_width=76):

# Because babel automatically encode strings, file should be open as binary mode.
with io.open(filename, 'wb') as f:
pofile.write_po(f, catalog, line_width)
pofile.write_po(f, catalog, line_width, sort_output=sort_output, sort_by_file=sort_by_message_location)
Copy link
Member

Choose a reason for hiding this comment

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

Memo

https://babel.pocoo.org/en/latest/api/messages/pofile.html#babel.messages.pofile.write_po

  • sort_output – whether to sort the messages in the output by msgid
  • sort_by_file – whether to sort the messages in the output by their locations

defaults are False: sort_output=False, sort_by_file=False

@@ -23,7 +23,7 @@ def load_po(filename):
return pofile.read_po(f, charset=charset)


def dump_po(filename, catalog, line_width=76):
def dump_po(filename, catalog, line_width=76, sort_output=False, sort_by_message_location=False):
Copy link
Member

Choose a reason for hiding this comment

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

Please add param lines on docstring for new 2 parameters.

These two arguments are not set to True within this tool and may be removed as unnecessary arguments. As a remedy, please describe the purpose for which they are arguments in the docstring or code comment.

UPDATED: please fix linter error.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants