Page MenuHomePhorge

Fix rendering of cowsay sheep.cow
ClosedPublic

Authored by Sten on Sep 11 2023, 10:12.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Jul 21, 17:23
Unknown Object (File)
Wed, Jul 17, 23:58
Unknown Object (File)
Sat, Jul 13, 17:45
Unknown Object (File)
Sat, Jul 13, 11:26
Unknown Object (File)
Sat, Jul 13, 09:54
Unknown Object (File)
Sat, Jul 13, 09:54
Unknown Object (File)
Sat, Jul 13, 09:54
Unknown Object (File)
Wed, Jul 10, 23:17

Details

Summary

In the templates of the external cowsay library, most replaceable tokens are specified as $var_name.
However, the sheep.cow and flaming-sheep.cow use the ${eyes} syntax instead. This is not recognised by PhutilCowsay.php resulting in incorrect rendering of the template.

This change updates PhutilCowsay.php to handle ${var_name} tokens as well as $var_name

___________________ < My eyes, my eyes! > ------------------- \ \ __ UooU\.'@@@@@@`. \__/(@@@@@@@@@@) (@@@@@@@@) `YY~~~~YY' || ||
Test Plan

In a Remarkup comment or document, add

cowsay(cow='sheep'){{{How do my eyes look now?}}}

When testing in differential, you don't even need to submit the comment.

Diff Detail

Repository
rARC Arcanist
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Sten requested review of this revision.Sep 11 2023, 10:12
src/utils/PhutilCowsay.php
76–86

Thanks. Personally I would prefer to have these regexes in a dedicated variables, like:

$patterns = array(
  ...,
  ...,
);
foreach($patterns as $token_pattern) {

}

So the foreach is a bit more readable and it can be expanded in the future without the 80 chars limit

Updates as per reviews

Sten marked an inline comment as done.Sep 11 2023, 12:39
This revision is now accepted and ready to land.Sep 11 2023, 19:04
This revision was automatically updated to reflect the committed changes.