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.
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.