Comment 17 for bug 212491

Revision history for this message
Colin Watson (cjwatson) wrote : Re: [Bug 212491] Re: Live CD, first screen, Hebrew not correctly displayed

This looks pretty nice, and I'm minded to apply something similar.
Thanks! A few comments, though:

 * Could you 'bzr branch
   lp:~ubuntu-core-dev/gfxboot-theme-ubuntu/mainline bidi' (or whatever
   branch name you choose), commit your changes there, push them to a
   branch on Launchpad, and create a merge proposal? That way you can be
   properly credited in revision control history, and we can do patch
   review more easily.

 * Please use the form "(LP: #nnnnnn)" to close Launchpad bugs, and use
   the correct bug number too. :-) Forms such as "closes: Malone
   #nnnnnn" have been deprecated for several years.

 * Are you sure that this works as well for Arabic as it does for
   Hebrew? I don't read or write either language, but I've worked with
   people who do, and I'm told that Arabic script needs a shaping
   algorithm to apply connections between letters. Without shaping, my
   understanding is that it looks amateurish at best and is likely to be
   difficult to read.

 * The string equality operator in Perl is 'eq', not '==' which is for
   numeric equality.

     $ perl -le 'print "hello" if "ar" == "he"'
     hello

 * It would be better to use safe opens rather than unsafe shell
   constructs; not that I actively distrust translators or anything, but
   I don't think it's generally a good idea to pass translations through
   the shell where a misplaced double quote could cause disaster.
   Instead of messing around escaping shell metacharacters, I think it
   would be better to do something like this (completely untested):

     use IPC::Open2; # at top of program

     my $pid = open2(\*BIDI_OUT, \*BIDI_IN, 'fribidi', '--nopad', '--nobreak');
     print BIDI_IN $txt;
     close BIDI_IN;
     { local $/ = undef; $txt = <BIDI_OUT>; } # might have to fiddle with trailing newlines
     waitpid $pid, 0;

Could you adjust your patch along these lines? Thanks again.