Opened 2 years ago

Closed 16 months ago

Last modified 11 months ago

#20422 closed Bug (fixed)

makemessages ignores "ignore patterns"

Reported by: nnseva Owned by: claudep
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 nnseva 2 years ago.
patch for django-1.5.1-final

Download all attachments as: .zip

Change History (10)

Changed 2 years ago by nnseva

patch for django-1.5.1-final

comment:1 Changed 2 years ago by bmispelon

  • Cc bmispelon@… added
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

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 2 years ago by claudep

  • Owner changed from nobody to claudep
  • Status changed from new to assigned
  • Triage Stage changed from Accepted to Ready for checkin

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

  • Resolution set to fixed
  • Status changed from assigned to closed

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 16 months ago by fdemmer

  • Patch needs improvement set
  • Resolution fixed deleted
  • Status changed from closed to new
  • Version changed from 1.5 to 1.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 16 months ago by fdemmer (previous) (diff)

comment:5 Changed 16 months ago by timo

  • Patch needs improvement unset
  • Version changed from 1.6 to 1.5

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

comment:6 Changed 16 months ago by timo

  • Resolution set to fixed
  • Status changed from new to closed

comment:7 Changed 16 months ago by infraredgirl

I opened a new ticket for this regression:

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

comment:8 Changed 11 months 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 11 months 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