Opened 3 years ago

Closed 3 years ago

Last modified 2 years ago

#20422 closed Bug (fixed)

makemessages ignores "ignore patterns"

Reported by: Vsevolod Novikov Owned by: Claude Paroz
Component: Core (Management commands) Version: 1.5
Severity: Normal Keywords: ignore, pattern
Cc: bmispelon@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

The default ignore pattern set contains '.*','*~', and 'CVS' patterns. As I was expecting, it should mean that any file having file NAME like these patterns, should be ignored.

Really, the makemessages management command ignores only project root files having file names like these.

As a result, the makemessages makes such unexpected things like walking through all .svn directories except root, or trying to analyze binary Mac OS X - specific file descriptors like '._adminoverride.js'

The reason for such inconsistency is a function is_ignored() defined in the makemessages.py.

While the django 1.5 declares this function in a module context, the current trunk defines it in a context of the find_files() function (it is not important, but makes creating ready-to-use patch harder).

Anyway, the only line required to be fixed is:

        if fnmatch.fnmatchcase(path, pattern):

which should be replaced by:

        if fnmatch.fnmatchcase(os.path.split(path)[-1], pattern):

This simple change makes makemessages command working with ignore patterns as expected.

The applied patch has been created for django version 1.5.1-final:

>>> import django
>>> django.VERSION
(1, 5, 1, 'final', 0)

Attachments (1)

django-makemessages-ignore-patterns-1.5.patch (488 bytes) - added by Vsevolod Novikov 3 years ago.
patch for django-1.5.1-final

Download all attachments as: .zip

Change History (10)

Changed 3 years ago by Vsevolod Novikov

patch for django-1.5.1-final

comment:1 Changed 3 years ago by Baptiste Mispelon

Cc: bmispelon@… added
Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset
Triage Stage: UnreviewedAccepted

Hi,

I added some tests and rewrote the is_ignored function a bit (using the builtin any instead of the loop).
I made a pull request out of it: https://github.com/django/django/pull/1076

Thank you for the report and the original patch.

comment:2 Changed 3 years ago by Claude Paroz

Owner: changed from nobody to Claude Paroz
Status: newassigned
Triage Stage: AcceptedReady for checkin

comment:3 Changed 3 years ago by Claude Paroz <claude@…>

Resolution: fixed
Status: assignedclosed

In 9012a9e2000020493b881a5b79cc801c62180796:

Fixed #20422 -- Applied makemessage's --ignore patterns to full path

Fix makemessage's --ignore patterns being applied to the full path
instead of the file name. Thanks to nnseva for the report and the
original patch.

comment:4 Changed 3 years ago by Florian Demmer

Patch needs improvement: set
Resolution: fixed
Status: closednew
Version: 1.51.6

Unfortunately the assumption that "file NAME like these patterns, should be ignored" was not correct. According to the documentation the --ignore pattern should "ignore files or directories matching" (https://docs.djangoproject.com/en/1.6/ref/django-admin/#django-admin-option---ignore)

The example given in the docs actually uses patterns with paths:

django-admin.py makemessages --locale=en_US --ignore=apps/* --ignore=secret/*.html

The applied patch breaks this behaviour by using filename = os.path.basename(path)

Being able to exclude paths is important to eg. easily exclude certain apps in a project from translation.

EDIT:

I looked at the code and I see now that as usual things are not that simple and why the tests still worked after that patch.

There is a test checking for ignore_dir/* which works, but there is none like ignore_dir/*.html.

My usecase (that broke with 1.6) was a pattern like apps/*/admin.py to ignore messages in admin (of all apps) when generating po files for customers, who are only supposed to translate messages from views and templates. Using the full path for pattern matching makes that work again, but maybe i am not seeing the complete picture here.

Last edited 3 years ago by Florian Demmer (previous) (diff)

comment:5 Changed 3 years ago by Tim Graham

Patch needs improvement: unset
Version: 1.61.5

Could you please open a new ticket as the fix for this one has already been released? Thanks.

comment:6 Changed 3 years ago by Tim Graham

Resolution: fixed
Status: newclosed

comment:7 Changed 3 years ago by Ana Krivokapic

I opened a new ticket for this regression:

https://code.djangoproject.com/ticket/22336

comment:8 Changed 2 years ago by Ramiro Morales <ramiro@…>

In b4dce7c37ab7666b023ac791d2c46dad6c8aa637:

Fixed #23298 -- Made makemessages actually ignore specified dirs on Windows.

This was detected by two failures in the i18n.test_extraction of our
test suite.

Refs #20422, refs #22336

comment:9 Changed 2 years ago by Tim Graham <timograham@…>

In 126606c5b8499830cd56cbe632f6af17bc7471ea:

[1.7.x] Fixed #23298 -- Made makemessages actually ignore specified dirs on Windows.

This was detected by two failures in the i18n.test_extraction of our
test suite.

Refs #20422, refs #22336

Backport of b4dce7c37a from master

Note: See TracTickets for help on using tickets.
Back to Top