Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#19877 closed New feature (fixed)

Allowing running management commands without colorized output

Reported by: josh@… Owned by: jose
Component: Core (Management commands) Version: master
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Since the output style for a command is set as BaseCommand.style, and from call_command() you can only pass arguments into execute(), which does not expose any kind of style argument.
So as far as I can tell, there is no possible way to set style to django.core.management.color.no_style, and all of the SQL I am getting has color codes, making it fail to execute.
The only other alternative I can think of is to strip out the color codes after the fact, but that doesn't really seem like a clean option.

Maybe an option could be added for sqlflush? Or a style could be added to BaseCommand's default options similar to how it handles stdout and stdin?

Change History (11)

comment:1 Changed 2 years ago by claudep

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

I think that's a good suggestion, and we should probably add it as a common option to BaseCommand.

comment:2 Changed 2 years ago by jose

  • Owner changed from nobody to jose
  • Status changed from new to assigned

comment:3 Changed 2 years ago by jose

Hello,

I think I've got this fixed. Now, a new default option --no-style is available in BaseCommand, so you can pass it to avoid using the color_style().

At this point, I've fixed some regression tests in admin_script tests module, but not sure if this is enough or I should write a new unit test for this option. If someone can tell me something about this, it will be great. For now, I ran the whole tests and there are no fails or warnings concerning my code.

You can view all that in my Django fork in Github https://github.com/jose-lpa/django

Should I make a pull request now? (Sorry, this would be my very first contribution to Django code, so not sure about how to proceed).

comment:4 Changed 2 years ago by timo

  • Has patch set

The next step is for someone to review the code. It seems like a new unit test would be helpful, but I'm not sure where it should live or how it would work exactly (presumably testing the difference between colored and non-colored output).

The new option should also be mentioned in the release notes.

https://github.com/django/django/pull/843

comment:5 Changed 2 years ago by jose

Sorry for the late answer and thanks for replying. I think you're right about the needing of a unit test to check the difference between outputs. I was wondering why this should be tested, but I didn't found any approach to do it. I will try to take a look closer.

About the mention of the new option, I also updated the related Django documentation to include a description of the option. That can be seen in the latest commit of my pull request.

comment:6 Changed 2 years ago by anonymous

Since I cannot find differences in the output string of a management command with and without styling, I've just wrote a unit test for the command argument --no-style to check that it is correctly handled when passed into a management command.

If someone can point me how we can test the string output, that will be very helpful.

comment:7 Changed 2 years ago by timo

  • Summary changed from call_command provides no method to set output style to Allowing running management commands without colorized output
  • Version changed from 1.4 to master

I was able to write a test and updated the PR to merge cleanly. I think it may be more intuitive to call the option "no-color" rather than "no-style", but I'll let others chime in before committing this.

comment:8 Changed 2 years ago by jose

Thanks, Tim, for your attention. I will say that seems a good idea for me to call the option --no-color, since it only prevents colorizing the output.

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

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

In 7b57e575c94691e40533e84d84235a0c6c90ab7b:

Fixed #19877 -- Added --no-color option to BaseCommand to avoid using output styles.

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

In dffda2ba4e55d4567b3a829d410dae794636ebdd:

Fixed a test that depended on the DB backend; refs #19877. Thanks Loic.

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

In 8550df869bef11d176aa926aa056df9f3566a304:

Removed part of a test that doesn't work on Jenkins; refs #19877.

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