Allow the makemessages command to accept a directory to operate on like xgettext does

[Terence Honles]

PR: ​https://github.com/django/django/pull/10422

Use case:

REPO_ROOT
|-- manage.py
|-- app/
|   `-- ...
`-- .venv/
    `-- ...

When ".venv" or another adjacent directory has a bad utf-8 marker or code that would cause xgettext to fail the management command cannot succeed. The workaround right now is to "cd" into the django_app and run "../mange.py" so the current directory is now the django_app and makemessages will properly exclude the other directories. Gettext has an options "-D" or "--directory" which changes the directory to recurse from the current directory to the specified directory. The patch linked adds that option and walks that directory instead of the current directory from the makemessages command.

[Claude Paroz]

Mmmh, not convinced yet. You have at least four possible workarounds, the one you mentioned in the ticket description, moving the .venv to a superior level, using the --ignore flag or create your own makemessages command subclass and complete the xgettext_options attribute.
I think this should be enough.

[Carlton Gibson]

Yep, I agree with Claude. We closed #29734 for the same reason. (See ​Customising the `makemessages` command to pass additional options to xgettext.)

[Terence Honles]

Replying to Claude Paroz:

Mmmh, not convinced yet. You have at least four possible workarounds, the one you mentioned in the ticket description, moving the .venv to a superior level, using the --ignore flag or create your own makemessages command subclass and complete the xgettext_options attribute.
I think this should be enough.

Adding to xgettext_options is not applicable because the list of files to process is enumerated by this command and not xgettext which is why I said this is allowing the command to emulate xgettext. Yes, I could override the command and replace find_files to ignore the value passed to it and hardcode the value I would pass for -D but having a custom command that is making find_files ignore the only argument passed to it seems silly and prone to error. The function already takes a variable, why isn't it being used?

Using --ignore is heavy handed because it's only a ​basename check

Moving the venv to a superior level is not acceptable because it is already at the root of the repository.

Of course I can continue doing what I am doing, but I don't understand why this very simple change which is built in to xgettext is not acceptable. Please just look at the diff! It's 6 lines of code that changed (4 of the 6 are just to take the command line argument) and makes the function find_files actually useful. Why was it written to take an argument but that argument always "."?

[Claude Paroz]

Replying to Terence Honles:

Using --ignore is heavy handed because it's only a ​basename check

The ignore function just below the line you mention shows that it ignores either on the basename or on the path. Please can you try that again?

[Terence Honles]

Replying to Claude Paroz:

Replying to Terence Honles:

Using --ignore is heavy handed because it's only a ​basename check

The ignore function just below the line you mention shows that it ignores either on the basename or on the path. Please can you try that again?

You're right, I had familiarized myself more with collectstatic and it handles things differently. Looking at the code more closely my issue with collectstatic may have been the path prefix. I am able to ignore directories properly with --ignore, however I still don't see what is wrong with being able to set the root for find_files.

The code I am working with is very largely inherited and there are many directories that need to be excluded so a single argument is preferred over multiple that may need to be updated as files which break xgettext are added via other tooling. It is nice to see I was incorrect about --ignore, but for maintainability changing the current working directory to exclude all the other files is still more bulletproof than having to update an --ignore list. Even better would be defining the root because it's more obvious to others than a cd dance.

[Claude Paroz]

Replying to Terence Honles:

I am able to ignore directories properly with --ignore, however I still don't see what is wrong with being able to set the root for find_files.

First I think that makemessages has already lots of options, and I don't like seeing the list growing version after version.
But probably the most compelling reason is that for options that are meant to always been set for some project (typically in your case), adding options seems not the way to go in my opinion. Options are good for real options, that is an optional choice the runner of the command may activate or not depending on some preference. Typically, the locale to generate messages for (with --locale). I'm aware that with this rule, several existing options should go away, too!

So to come back to your case, I would be open to make the find_files root more configurable, not through an option, but with a command attribute, so you could add your own makemessages command subclass in your project with something like this:

from django.core.management.commands.makemessages import Command as MakeMessagesCommand

class Command(MakeMessagesCommand):
    files_root = 'django_app'

Would you find that satisfactory for your use case?

[Terence Honles]

Replying to Claude Paroz:

Replying to Terence Honles:

I am able to ignore directories properly with --ignore, however I still don't see what is wrong with being able to set the root for find_files.

First I think that makemessages has already lots of options, and I don't like seeing the list growing version after version.
But probably the most compelling reason is that for options that are meant to always been set for some project (typically in your case), adding options seems not the way to go in my opinion. Options are good for real options, that is an optional choice the runner of the command may activate or not depending on some preference. Typically, the locale to generate messages for (with --locale). I'm aware that with this rule, several existing options should go away, too!

So to come back to your case, I would be open to make the find_files root more configurable, not through an option, but with a command attribute, so you could add your own makemessages command subclass in your project with something like this:

from django.core.management.commands.makemessages import Command as MakeMessagesCommand

class Command(MakeMessagesCommand):
    files_root = 'django_app'

Would you find that satisfactory for your use case?

Well I disagree about this not being a "real option" and I would prefer not to have to create a custom command just to configure the flag, but if it was easier to customize as you are suggesting I probably would have just created the custom command rather than creating the PR (so this is better than nothing). I still think extensibility should be provided by options so things like "make" could hold the flags rather than having a bunch of custom commands, but I understand in world without build tools you would want to make sure every user used the same options. Obviously this is a discussion about where your configuration lives. I would prefer it to be in my build tooling and others may prefer to create commands so it's "build in" to the manage.py <subcommand>. For me the preference is my config will live in one file rather than distributed across the filesystem.

I obviously have stated my preference, but thank you for discussing this with me.

[Carlton Gibson]

Thanks for the thoughtful discussion all.

I'm going to close again. I think Claude's points cover what we'll ship in core.

It'd be a bit more work for your use-case but I expect writing a wrapper script to take the options you want would not be infeasible.
Sorry if that's not perfect for you Terence.