Opened 14 years ago

Last modified 13 years ago

#13291 new New feature

Allow `color_style()` and `parse_color_setting()` to be used with custom colour palettes.

Reported by: Tai Lee Owned by: nobody
Component: Core (Management commands) Version: dev
Severity: Normal Keywords: management command customize color roles
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

I want to add new roles and change the default terminal colours that ship with Django, for use in my own project's management commands, but I want to retain the ability for these to be overridden with a DJANGO_COLORS environment variable. I also don't want to repeat myself by copying the entire color_style() and parse_color_setting() functions, which are hard coded to use django.utils.termcolor.PALETTES and which ignore any roles specified in DJANGO_COLORS which are not also present in django.utils.termcolors.PALETTES[NOCOLOR_PALETTE].

It turns out that it's fairly trivial to add a palettes argument to both functions, which will allow me to simply define my own dictionary of palettes somewhere (or import and extend the ones that ship with Django) and then call color_style(my_palettes) to get a style object with roles and default colours that are appropriate for my project, and also retain the ability for users to customise the colours for my roles with a DJANGO_COLORS environment variable.

Attachments (1)

13291-custom-color-palettes-r12937.diff (7.0 KB ) - added by Tai Lee 14 years ago.

Download all attachments as: .zip

Change History (8)

comment:1 by Tai Lee, 14 years ago

Component: Uncategorizeddjango-admin.py
Keywords: management command customize color roles added
Version: 1.1SVN

comment:2 by Tai Lee, 14 years ago

Has patch: set

Patch with tests and docs attached. Also fixed a related bug when calling style.ROLE(integer) which tried to concatonate the integer with a string, rather than use string formatting to convert the integer to string.

comment:3 by Russell Keith-Magee, 14 years ago

Triage Stage: UnreviewedAccepted

comment:4 by Carl Meyer, 13 years ago

Patch needs improvement: set

Some notes on the patch:

I think the documentation addition is misplaced; rather than being in the django-admin.py documentation, it should go in the "writing custom management commands" howto, as it's only relevant to someone who's writing a custom command, not all users of django-admin.py.

I think it also needs more context; there currently is no documentation at all for using django's style/colors system in your own custom management command. If we're going to bother adding the customizability this patch offers, we should document it, and that means full documentation that would actually allow someone to use it, including how to actually do the styled output, etc.

Also, as currently written, the docs show updating PALETTES directly, which monkeypatches the built-in palettes. Actually, even the .copy() method used in the tests, presumably to avoid that, is not effective: only a shallow copy of the outer dictionary is performed, so the inner palette dictionaries are still referring to the same actual dictionary, and updating them _still_ in fact monkeypatches the built-in palettes.

All of which leads me in a roundabout way to my only issue with the code patch itself: it suggests a usage pattern that is too easy to misuse, as demonstrated by the fact that both the provided test and the provided docs misuse it! I think perhaps a better option for the API would be to pass in palette_overrides rather than completely new palettes; this would make the usage look more like palette_overrides={'dark': {'WARNING': {'fg': 'yellow'}}}. Or, perhaps even better, to reuse the more compact DJANGO_COLORS string syntax, so you would just pass in palette_overrides="warning=yellow".

comment:5 by Julien Phalip, 13 years ago

Severity: Normal
Type: New feature

comment:6 by Aymeric Augustin, 12 years ago

UI/UX: unset

Change UI/UX from NULL to False.

comment:7 by Aymeric Augustin, 12 years ago

Easy pickings: unset

Change Easy pickings from NULL to False.

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