matthewwithanm / django-imagekit

Automated image processing for Django. Currently v4.0
http://django-imagekit.rtfd.org/
BSD 3-Clause "New" or "Revised" License
2.26k stars 276 forks source link

generateimages: fix taking arguments #431

Closed x-yuri closed 7 years ago

x-yuri commented 7 years ago

Feel free to tell me what is to be fixed.

Fixes #430

vstoykov commented 7 years ago

Because the library officially supports pre Django 1.8 versions can you make it compattible?

  1. Let the args attribute stay
  2. Set the argument name for argparse to be the same as before generator_ids (with s at the end)
  3. Check if generator_ids is in options (Django 1.8+) or use args instead.
x-yuri commented 7 years ago

Sorry, it took so long.

Set the argument name for argparse to be the same as before generator_ids (with s at the end)

It's so for a reason. First, that's what documentation suggests:

...
    def add_arguments(self, parser):
        parser.add_argument('poll_id', nargs='+', type=int)

    def handle(self, *args, **options):
        for poll_id in options['poll_id']:
...

This way we have:

usage: manage.py generateimages [-h] [--version] [-v {0,1,2,3}]
                                ...
                                [generator_id [generator_id ...]]

Following your suggestion, it would be:

usage: manage.py generateimages [-h] [--version] [-v {0,1,2,3}]
                                ...
                                [generator_ids [generator_ids ...]]

Just in case I'm leaving here a script I used to test it. It's to be used like so:

./create-django-app-with-image-field.sh django-app-1

NOTE. it does rm -rf django-app-1. Before running you're supposed to change PATH_TO_REPO variable at the beginning of the script. And probably amend it for your environment (Python 3.4 executable name?).

vstoykov commented 7 years ago

Ok tank you for your contribution.

About the help text you are right that it is looking better. It just looked kind of not in place in the code when you are doing:

generator_ids = options['generator_id']

instead of

generator_ids = options['generator_ids']

But at the end help text for the user is more important.