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

add_translation_dirs documentation is incorrect in regards to priorities #1473

Closed
rbu opened this issue Nov 25, 2014 · 5 comments
Closed

add_translation_dirs documentation is incorrect in regards to priorities #1473

rbu opened this issue Nov 25, 2014 · 5 comments
Labels

Comments

@rbu
Copy link
Contributor

rbu commented Nov 25, 2014

From pyramid/config/i18n.py:

    @action_method
    def add_translation_dirs(self, *specs):
...
        Later calls to ``add_translation_dir`` insert directories into the
        beginning of the list of translation directories created by earlier
        calls.  This means that the same translation found in a directory
        added later in the configuration process will be found before one
        added earlier in the configuration process.  However, if multiple
        specs are provided in a single call to ``add_translation_dirs``, the
        directories will be inserted into the beginning of the directory list
        in the order they're provided in the ``*specs`` list argument (items
        earlier in the list trump ones later in the list).

This description is not in sync with the behaviour of Pyramid and gettext. In the list of locale directories, if two translations provide the same translation, the last in the list overrides the earlier ones.
That means this documentation is wrong in two aspects:

  1. In one call providing multiple *specs, later dirs override earlier dirs
  2. In two calls, earlier calls override later calls.

This issue is about fixing the documentation, I will add another issue about changing the implementation to be more sane.

rbu added a commit to rbu/pyramid-i18n-reproducer-1473 that referenced this issue Nov 25, 2014
@rbu
Copy link
Contributor Author

rbu commented Nov 25, 2014

I have created a minimal reproducer that illustrates this problem. Somewhat quoting from the helloworld.py:

    config.add_translation_dirs('locale', 'locale_override')
    # override wins

    config.add_translation_dirs('locale_override', 'locale')
    # override loses

    config.add_translation_dirs('locale_override')
    config.add_translation_dirs('locale')
    # override wins

@piotr-dobrogost
Copy link

In the list of locale directories, if two translations provide the same translation, the last in the list overrides the earlier ones.

This is effect of

def merge(self, translations):

@mmerickel
Copy link
Member

The issues described here seem legitimate, however I'm personally have a big difficulty understanding how things are supposed to actually work with regards to 1) extra locale directories 2) more specific dialects 3) combinations of 1 and 2. Can anyone provide a few examples for me?

From how override_assets works I agree that the last example given by @rbu makes no sense and we should try to fix it, but unless someone is submitting a PR with lots of tests I need to understand the corner cases here.

@mmerickel mmerickel added the bugs label Dec 15, 2016
@mmerickel
Copy link
Member

mmerickel commented Dec 16, 2016

So the docstring is just 100% wrong right? Or can I not read?

  1. Currently, last spec wins in a single call. Not the first spec as it says ("items earlier in the list trump items laster in the list").
  2. Currently, the first add_translation_dirs call wins. Not the last spec as it says.

Can someone confirm that what I said currently happens is actually what currently happens, and that it is counter to the docstring?

@piotr-dobrogost
Copy link

piotr-dobrogost commented Dec 16, 2016

So the docstring is just 100% wrong right?

Yes, you are right in that the docstring is 100% wrong. That the docstring is 100% wrong comes from the fact that its author must have been convinced that "search path" in case of gettext and Translations class works the same as searching in "normal" search path (PATH env var for instance).

Can someone confirm that what I said currently happens is actually what currently happens, and that it is counter to the docstring?

That it happens was already confirmed by @rbu's tests in #1473 (comment)
That it is counter to the docstring follows directly from these tests.

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

No branches or pull requests

3 participants