Page MenuHomePhorge

Add PhutilRemarkupHexColorCodeRule, a new remarkup rule to format color codes
ClosedPublic

Authored by 20after4 on Feb 25 2024, 04:10.
Tags
None
Referenced Files
F2205347: D25540.1716838429.diff
Sun, May 26, 19:33
Unknown Object (File)
Sat, May 25, 04:49
Unknown Object (File)
Tue, May 21, 20:50
Unknown Object (File)
Mon, May 20, 18:44
Unknown Object (File)
Mon, May 20, 18:29
Unknown Object (File)
Mon, May 20, 09:02
Unknown Object (File)
Mon, May 20, 08:43
Unknown Object (File)
Mon, May 20, 08:41

Details

Summary

Implements a remarkup rule that formats the background color to match the value of inline hex color codes.

The format for this rule looks like {#RRGGBB} and it will be formatted the same as monospace text, so #RRGGBB with the background set to the specified color.

Test Plan
  • run arc unit and see the tests pass
  • create some markup with {#ff0000} color codes for all of your favorite colors {#ee33ee}

Diff Detail

Repository
rP Phorge
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

This is not yet tested.

Interesting. What's the use case you have in mind?

Interesting, I agree. Note that also #bbb is valid but I see that 6 is minimum characters.

Added contrasting color calculation

Now foreground will always contrast with the background color.

I went down a rabbit hole while trying to add unit tests for this. Our core remarkup unit tests are pretty thorough but they only seem to cover the "core" remarkup engine and syntax. When I added a new test case for this it would pass but always caused a different test case to fail in weird ways. I haven't yet figured out what part of the unit test or engine is stateful such that my unit test alters the state in a way that breaks a later test. If I change the order that tests run in, it just causes another different test case to fail.

About your Remarkup unit tests, try to rebase. Maybe related to D25559.

  • Fixed a logic bug.
  • Added passing unit tests.
20after4 retitled this revision from WIP: add PhutilRemarkupHexColorCodeRule to Add PhutilRemarkupHexColorCodeRule, a new remarkup rule to format color codes.Apr 1 2024, 18:10
20after4 edited the summary of this revision. (Show Details)
20after4 edited the test plan for this revision. (Show Details)

About your Remarkup unit tests, try to rebase. Maybe related to D25559.

Thanks, it does seem to work now!

avivey subscribed.

looks good to me - couple of small inlines....

src/infrastructure/markup/markuprule/PhutilRemarkupHexColorCodeRule.php
28

can the repeat syntax say "exactly 6 times"

46

I think we use {$foo}

This revision is now accepted and ready to land.Apr 1 2024, 19:41
  1. Are we interested in #fff?
  1. I've updated and restarted my webserver but I cannot test this 🤔 ideas? Thanks again :)
src/infrastructure/markup/markuprule/PhutilRemarkupHexColorCodeRule.php
11

oh NO extra newline here (nuclear alarm sound)

25

Maybe we can move public methods above, so to have protected methods below

src/infrastructure/markup/remarkup/__tests__/PhutilRemarkupEngineTestCase.php
101

(can we restore this newline? My entire infrastructure is based on it!1!)

20after4 marked 4 inline comments as done.

Address review feedback:

  • match exactly 6 digits, not 6 or more
  • Add support for 3 digit color codes
  • wrap variable references in braces
  • updated unit test data:
    • add 3 digit color
    • make sure invalid color codes do not, in fact, match.
src/infrastructure/markup/markuprule/PhutilRemarkupHexColorCodeRule.php
28

Good catch.

src/infrastructure/markup/markuprule/PhutilRemarkupHexColorCodeRule.php
14

repeat exactly 3 times for the 3 digit codes, then repeat that sequence exactly 1 or 2 times for 3 or 6 digits.

I do not understand why it does not work to me.

  • I can pass the unit test
  • I have the class PhutilRemarkupHexColorCodeRule
  • I have arc liberate up to date
  • I have restarted my local webserver

Possible cause:

I start with this Remarkup:

some red, grey and white for you: {#ff0000} {#999999} {#ffffff} Also green: {#0F0} but not this: {#0000000} or this: {#0000} asdasdasd asdsdsssss sdss

And I see that the apply($text) function receives a PhutilSafeHTML object that, casted to string, contains this value:

some red, grey and white for you: 1Z 2Z 3Z Also green: 4Z but not this: 5Z or this: 6Z asdasdasd asdsdss

So it never matches to me.

But this is strange since, again, the unit test works 🤔

LOL I only ever tested the unit test case. I forgot to add the rule to PhabricatorMarkupEngine:

PhabricatorMarkupEngine.php
$rules[] = new PhutilRemarkupHexColorCodeRule();

I do not understand why it does not work to me.

  • I can pass the unit test
  • I have the class PhutilRemarkupHexColorCodeRule
  • I have arc liberate up to date
  • I have restarted my local webserver

Possible cause:

I start with this Remarkup:

some red, grey and white for you: {#ff0000} {#999999} {#ffffff} Also green: {#0F0} but not this: {#0000000} or this: {#0000} asdasdasd asdsdsssss sdss

And I see that the apply($text) function receives a PhutilSafeHTML object that, casted to string, contains this value:

some red, grey and white for you: 1Z 2Z 3Z Also green: 4Z but not this: 5Z or this: 6Z asdasdasd asdsdss

So it never matches to me.

But this is strange since, again, the unit test works 🤔

Actually add the rule to the remarkup engine.

It's kind of a shame that remarkup wasn't named Pharkup.

...or Pharkdown :D

Anyway it still does not work to me in real life. Tested on a Task description or on a comment 🧐 Any idea why?

Doesn't actually work for me either after arc patch:

image.png (317×429 px, 19 KB)

The test does run and pass, so something strange is up.

It's something to do with the priority - it works if the priority is 150.
Probably one of the other similar rules is interrupting (icon? Object? Maybe it thinks this is a mention of a project?)

man I didn't even consider that you could actually have a valid project named #0dba11 ... maybe the syntax should be something different. #{00ffff} perhaps?

Also apologies for not testing this locally in the phab ui, I was far too confident in my TDD.

A weird but... expandable, fast, easy possibility is introducing this syntax:

color {{{#00ffff}}}

or something like that ↑

That uses the already existing cowsay-like syntax. THANKS FOR ASKING, it's this one (see raw remarkup asd):

cowsay {{{ asd }}}

_______ < asd > ------- \ ^__^ \ (oo)\_______ (__)\ )\/\ ||----w | || ||

figlet {{{ asd }}}

_ __ _ ___ __| | / _` / __|/ _` | | (_| \__ \ (_| | \__,_|___/\__,_|

I think the current syntax should be ok because it isn't normal to use {} around a project name. And my regex only matches exactly 3 or 6 hex digits, as demonstrated in the hex-color-code.txt test file.

I'm on latest master, and I'm still not able to try this 🤔 No errors. Just plaintext remarkup. Test case:

Yeah please set {#ff0000} asdasd

Yeah yeah {#ee33ee}

{#ee33e1}

I probably missed the train, so… can you add the docs for this? :-P