Skip to content

Fix extraction order #424

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

Merged
merged 1 commit into from
Jul 8, 2016
Merged

Fix extraction order #424

merged 1 commit into from
Jul 8, 2016

Conversation

ngrilly
Copy link
Contributor

@ngrilly ngrilly commented Jun 24, 2016

Before Babel 2.3, the input directories were always extracted
in the order specified by the command line arguments.

For example, pybabel extract dir_a dir_b dir_c would process
dir_a first, then dir_b, and finally dir_c.

Since Babel 2.3, the inputs are stored in a Python dictionary,
and we loop over items in the dictionary using items().
This makes the order unspecified, not matching the order
specified on the command line anymore.

Before Babel 2.3, the input directories were always extracted 
in the order specified by the command line arguments.

For example, `pybabel extract dir_a dir_b dir_c` would process 
`dir_a` first, then `dir_b`, and finally `dir_c`.

Since Babel 2.3, the inputs are stored in a Python dictionary, 
and we loop over items in the dictionary using items().
This makes the order unspecified, not matching the order 
specified on the command line anymore.
@codecov-io
Copy link

Current coverage is 90.15%

Merging #424 into master will not change coverage

@@             master       #424   diff @@
==========================================
  Files            24         24          
  Lines          3951       3951          
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
  Hits           3562       3562          
  Misses          389        389          
  Partials          0          0          

Powered by Codecov. Last updated by 5fe694b...d0dfcdb

@buckbaskin
Copy link

@ngrilly Have you considered using a collections.OrderedDict? I believe it does what you are looking for (maintain insertion order). See the first recipe for an example of what I'm thinking about.

@ngrilly
Copy link
Contributor Author

ngrilly commented Jun 25, 2016

@buckbaskin Yes, I've considered it. It even was my first thought :-) But I think using an OrderectDict would be overkill in this case, because the only operation we do over the data structure returned by _get_mappings is to iterate over it, nothing else.

In the gettext philosophy, the default behavior is to store messages in .po files in their extraction order. This preserves message "locality", and keep related messages together, which facilitates the translator's work. This is why I think fixing this is important.

@akx
Copy link
Member

akx commented Jul 8, 2016

Yeah, this looks fine, thanks! (_get_mappings() is an underscore-private API anyway; if someone was depending on it returning a dict, they should be prepared to deal with it changing.)

Thank you for the patch, @ngrilly!

@akx akx merged commit 4321605 into python-babel:master Jul 8, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants