Differential D25435

Fix rendering of cowsay sheep.cow
ClosedPublic
Actions

Authored by Sten on Sep 11 2023, 10:12.

Details

Reviewers

avivey

Group Reviewers

O1: Blessed Committers

Commits

rARC35e127da57a8: Fix rendering of cowsay sheep.cow

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 created this revision.Sep 11 2023, 10:12

Owners added a reviewer: O1: Blessed Committers.Sep 11 2023, 10:12

Herald added subscribers: Cigaryno, Matthew, valerio.bozzolan, tobiaswiese. · View Herald TranscriptSep 11 2023, 10:12

Sten requested review of this revision.Sep 11 2023, 10:12

Harbormaster completed remote builds in B837: Diff 1379.Sep 11 2023, 10:12

Sten edited the summary of this revision. (Show Details)Sep 11 2023, 10:13

Can you add a couple of tests to https://we.phorge.it/source/arcanist/browse/master/src/utils/__tests__/cowsay/ ?

valerio.bozzolan added inline comments.Sep 11 2023, 10:48

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