Page MenuHomePhabricator

Changed the rendering of the `priority` column color based on the `priorityColor` property
ClosedPublic

Authored by bwinterton on Apr 9 2014, 6:11 PM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Apr 24, 10:06 PM
Unknown Object (File)
Sun, Apr 21, 7:50 AM
Unknown Object (File)
Fri, Apr 19, 11:18 AM
Unknown Object (File)
Mon, Apr 15, 3:38 AM
Unknown Object (File)
Mon, Apr 15, 3:38 AM
Unknown Object (File)
Mon, Apr 15, 3:38 AM
Unknown Object (File)
Mon, Apr 15, 3:20 AM
Unknown Object (File)
Mon, Apr 15, 3:02 AM
Subscribers

Details

Summary

I changed the rendering of the bar color for the priority column when running arc tasks to match the priorityColor property.

If the priorityColor property is one of the basic colors already supported by ansi, then the bar is set to that color, otherwise it is set to white.

This will allow the user to customize maniphest priorities and then set their own colors and have those colors display correctly when running arc tasks

Fixed some linting errors

Test Plan

Run arc tasks and ensure that:

  • the priority column is displayed with a colored bar
  • the priority column bar is the correct color (or white if it is an unsupported color)

Diff Detail

Repository
rARC Arcanist
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

bwinterton retitled this revision from to Changed the rendering of the `priority` column color based on the `priorityColor` property.
bwinterton updated this object.
bwinterton edited the test plan for this revision. (Show Details)
bwinterton added a reviewer: epriestley.

Currently this does not allow for users to define any colors other than the basics:

  • magenta
  • red
  • yellow
  • green
  • blue
  • cyan

and have them show up correctly in the terminal. Another option (which I feel would be better and would also allow us to continue supporting the default configuration) would be to add a terminalColor property to the maniphest.priorities config and allowing the user to define a color that will be displayed on phabricator and a color that arc tasks will display.

If you think this is a better option, let me know and I will close this diff and make another one with those changes instead. Thanks for your help!

I think doing terminalColor is probably overkill, but maybe it's worth it to map the web colors to terminal colors? Specifically, this is my best effort at producing such a map offhand:

$web_to_terminal = array(
  'red' => 'red',
  'orange' => 'orange', // Or red? Is orange a terminal color?
  'yellow' => 'yellow',
  'green' => 'green', 
  'blue' => 'blue',
  'sky' => 'cyan',
  'indigo' => 'magenta',
  'violet' => 'magenta',
);

Then send anything else to white? Does that seem reasonable? I think that should be the best we can do in all reasonable cases, without requiring additional configuration.

Actually I like that idea a lot. I can see how terminalColor could be overkill. But I like the web color map. Would you like to to just map the default configuration colors, or would you like me to map basic ones as well? I will get to work on that right now.

The web config should only support a subset of colors -- basically the ones here, under the "Colors" header:

https://secure.phabricator.com/uiexample/view/PHUIColorPalletteExample/

It doesn't (IIRC) support arbitrary web/HTML colors, so there's no need to map anything not on the palette page. I think the most-ambitious version of this patch would just map the "light" colors, too.

bwinterton edited edge metadata.
  • Changed the priority_accepted_colors to web_to_terminal_colors map and mapped all colors found in the palette
epriestley edited edge metadata.

Perfect, thanks!

This revision is now accepted and ready to land.Apr 9 2014, 7:12 PM
epriestley updated this revision to Diff 20722.

Closed by commit rARCac528ab3a83b (authored by @epriestley).