Page MenuHomePhabricator

Figlet does not properly render apostrophes with some characters
Open, WishlistPublic

Description

Examine this comment:
https://secure.phabricator.com/T9360#143554
At first it appears the comment is dancing, livened by @chad's singing.

It looks like when the apostrophe immediately proceeds an 'm', 'o' or is immediately proceeded by an 's', 'm', or 'o'. There might be other characters it doesn't go well with. I haven't checked capital letters.

___ _ _ _ _ _ |_ _( ) __| | | |__ ___ ___( ) | __ ___ ___ _ __ ___ | ||/ / _` | | '_ \ / _ \ / _ \/| |/ / / _ \ / _ \| '_ \/ __| | | | (_| | | |_) | __/ | (_) | | < | (_) | (_) | |_) \__ \ |___| \__,_| |_.__/ \___| \___/ |_|\_\ \___/ \___/| .__/|___/ |_|
___ _ _ |_ _( ) __ ___ ___ | | __ | ||/ '_ ` _ \ / _ \| |/ / | | | | | | | | | (_) | < |___| |_| |_| |_| \___/|_|\_\
___ _ _ _ _ __ ___ |_ _( ) (_)___ ___ | | __ | '_ ` _ \ | ||/ | / __| / _ \| |/ / | | | | | || | | \__ \ | (_) | < |_| |_| |_|___| |_|___/ \___/|_|\_\
_ _ _ _ ( )__| | ___)_ __ _ __ (_)_ __ __ _ |/ __| |/ / | '_ \| '_ \| | '_ \ / _` | \__ \ <| | |_) | |_) | | | | | (_| | |___/_|\_\_| .__/| .__/|_|_| |_|\__, | |_| |_| |___/
_ _ _ _ ( ) __ ___ (_)___| |_ ___ _ __(_) |/ '_ ` _ \| / __| __/ _ \ '__| | | | | | | | \__ \ |_ __/ | | | |_| |_| |_|_|___/\__\___|_| |_|
_ _ _ _ _ _ _ _ ( ) _ __ ___ (_)___| |_ ___ _ __(_) __ __ ) ___ _ __ ___| |_ ___ _ __ | |__( ) ___ ___ |/ | '_ ` _ \| / __| __/ _ \ '__| | \ \ /\ / // / _ \| '_ \/ __| __/ _ \ '__| | '_ \/ / _ \ / _ \ | | | | | | \__ \ |_ __/ | | | \ V V / | (_) | | | \__ \ |_ __/ | | |_) | | (_) | (_) | |_| |_| |_|_|___/\__\___|_| |_| \_/\_/ \___/|_| |_|___/\__\___|_| |_.__/ \___/ \___/

Event Timeline

cspeckmim updated the task description. (Show Details)
cspeckmim added subscribers: cspeckmim, chad.

I think this is a problem only with the default font, it seems OK in other fonts:


_ _ _ _ __ ___( ) | __ _ __| |_ _ | '_ ` _ \/| |/ _` |/ _` | | | | | | | | | | | | (_| | (_| | |_| | |_| |_| |_| |_|\__,_|\__,_|\__, | |___/
### # # ### # ## ##### # # ## ## # # # # # # # # # ## # # # # # # # # # # # ###### # # # # # # # # # # # # # ###### # # ##### #
o _ /| | | _ _ _ | | __, __| / |/ |/ | |/ / | / | | | | | |_/ |__/\_/|_/\_/|_/ \_/|/ /| \|
+-+-+-+-+-+-+ |m|'|l|a|d|y| +-+-+-+-+-+-+

This could be:

  1. a bug with the font itself; or
  2. a bug with the PHP figlet library's decoding and rendering of the font.

We could distinguish between these by using the same font file with the real figlet binary. If rendering there is correct, that implicates the PHP figlet library. If rendering there is incorrect, that implicates the font. (I think (2) is much more likely.)

If this is a font problem, we could remove the font and choose a different default. If this is a library problem, we could try to fix the library. In either case, we could work around the problem by choosing a different default; this might be much easier than fixing the library.

To move forward:

  • First, we should test rendering of the default font (which should be externals/figlet/fonts/standard.flf) with the normal figlet binary to implicate the font file or the PHP figlet renderer.
  • If the renderer is at fault, we should ideally try to fix the renderer. It's only ~500 lines long with ~300 lines of real logic, and there's a C implementation to compare it to, so maybe this isn't too tricky. However, the C implementation looks like it's more in the ballpark of ~2,500 lines of real figlet logic, so it's possible that the problem with the PHP library is that it's missing 2000 lines of complex layout code.
  • If we can't fix the renderer, we could just pick a different default font.
epriestley triaged this task as Wishlist priority.Dec 4 2015, 12:37 PM
epriestley added a project: Remarkup.
epriestley renamed this task from Figlet does not properly render apostrophe's with some characters to Figlet doe's not properly render apostrophe's with some character's.Dec 4 2015, 1:03 PM

From a first glance it looks like the renderer may be the issue (tested the font in phabricator with figlet installed via homebrew):

figlet-test.png (740×1 px, 375 KB)

PhabricatorRemarkupFigletBlockInterpreter is a remarkup rule for rendering figlet fonts, invoked like this:

figlet {{{ The quick brown fox... }}}

This produces:

_____ _ _ _ _ __ |_ _| |__ ___ __ _ _ _(_) ___| | __ | |__ _ __ _____ ___ __ / _| _____ __ | | | '_ \ / _ \ / _` | | | | |/ __| |/ / | '_ \| '__/ _ \ \ /\ / / '_ \ | |_ / _ \ \/ / | | | | | | __/ | (_| | |_| | | (__| < | |_) | | | (_) \ V V /| | | | | _| (_) > < _ _ _ |_| |_| |_|\___| \__, |\__,_|_|\___|_|\_\ |_.__/|_| \___/ \_/\_/ |_| |_| |_| \___/_/\_(_)_)_) |_|

A while ago we implemented this by just running figlet, but it turns out that this is pretty hard to make safe (see T9408), so we swapped it to a PHP implementation (in externals/pear-figlet/). However, this implementation appears to have a bug with rendering apostrophes:

figlet {{{ C'thun }}}

...produces:

____ _ _ _ / ___( ) |_| |__ _ _ _ __ | | |/| __| '_ \| | | | '_ \ | |___ | |_| | | | |_| | | | | \____| \__|_| |_|\__,_|_| |_|

Uhhh, that one works fine.

_ _ _ | | | ( ) __ ___ _ __ ___ _ __ ___ | | | |/ '_ ` _ \| '_ ` _ \| '_ ` _ \ | |_| | | | | | | | | | | | | | | | | | \___/ |_| |_| |_|_| |_| |_|_| |_| |_|

Okay, that one's kinda broken. See original description for some more examples.

Either:

  • Spend a little time trying to figure this out (or trying to figure out how involved figuring this out is, or whatever else). I haven't made an attempt at this so it may be unreasonably difficult, although the total size of the implementation is not particularly large so it's conceivable that the fix is fairly easy.
  • Or, give up and close this task as not worth fixing.

If we give up, we may want to change the default $font in PhabricatorRemarkupFigletBlockInterpreter to a different one. We currently use standard, but could pick anything from externals/figlet/fonts/.

Alright I've been able to narrow this down a little bit more. There's some error with how external/pear-figlet/Text/Figlet.php is smushing characters. Specifically lines 339-362 as well as the _rep($r) function. The equivalent function in the C code is here. I haven't been able to find an obvious discrepancy, but I suspect there's a subtle difference in how one of the regex functions is handling things or an error in one of the regexes themselves. This error also occurs with backticks and double-quotes:

pear-figletfigletnotes
pasted_file (89×154 px, 2 KB)
pasted_file (99×158 px, 2 KB)
pear-figlet smushes "/I" into just / and replaces )_ with just )
pasted_file (95×172 px, 2 KB)
pasted_file (106×166 px, 2 KB)
The _( gets smushed into just _. Also starting on line 4, it seems like pear-figlet starts smushing the whitespace below the quotes
pasted_file (95×163 px, 1 KB)
pasted_file (97×165 px, 2 KB)
Both of the issues that the above two have combined into one. Pretty much, I don't think pear-figlet is capable of rendering _( or )_ (as well as other combinations). It needs to do that sometimes though.

Also notable, b'<ANY CHARACTER> always renders messed up. There are some other letters like that as well.

Anyway I think this is a problem that requires beer so I'm going to look more into it this weekend.

chad renamed this task from Figlet doe's not properly render apostrophe's with some character's to Figlet does not properly render apostrophe's with some character's.Mar 27 2017, 3:56 PM
jcox removed jcox as the assignee of this task.Mar 27 2017, 4:36 PM
jcox added a subscriber: jcox.

HoYp1cA.gif (375×500 px, 220 KB)

CodeMouse92 renamed this task from Figlet does not properly render apostrophe's with some character's to Figlet does not properly render apostrophes with some characters.Mar 27 2017, 4:39 PM

THANKS FOR RUINING THE JOKE EVERYONE

is srs bsness mode taking over? i'm in a panic :[