Re: [putty]Bidi + PuTTY - done

[Simon Tatham <anakin at pobox dot com>]

"ahmad khalifa" <ahmadkhalifa at hotmail dot com> wrote:

> patch is now OK. (thanks nadim)
> http://cvs.arabeyes.org/cvsweb/projects/external/putty/

Thanks. This seems more workable; the only thing I had to do to get
it to compile was to add arabic.c and minibidi.c to `Recipe' so that
the makefiles remembered to build them.

I've made some immediate changes to the patch, some to make it fit
with the PuTTY design principles and some because they simply
appeared to be mistakes:

 - Memory allocation is now done through the `snew', `sresize',
   `sfree' macros like the rest of PuTTY.

 - Removed all // comments, since we can't guarantee that all
   compilers ever used to build PuTTY will support them in C.

 - Re-indented the code in accordance with the PuTTY coding
   standard. (Sorry :-/ )

 - The variables `wcFrom', `wcTo' and `ltemp' are now part of the
   Terminal structure, so that when we port to platforms that expect
   to run more than one terminal per process, they will be
   replicated correctly.

 - do_arabic_textout() has been moved into window.c, because it is
   specific to the Windows front end. (Otherwise, arabic.c would not
   compile under Linux. Linux does not need do_arabic_textout()
   since the GTK text functions I'm using are admirably stupid and
   only do exactly what they're told.) Also I've renamed it
   `exact_textout' because it isn't about doing Arabic things - it's
   precisely about _avoiding_ doing anything Arabic-specific :-)

 - The line `memset(buffer, 0, sizeof(buffer))' in exact_textout()
   is wrong, since sizeof(buffer) is always 4 and has nothing to do
   with the size you just allocated. Fixed.

 - I've tried to do a better job of the ldisc.c hack, to preserve
   the previous hexadecimal behaviour where it's needed. I'm not
   _entirely_ sure it worked, so please let me know if I've missed
   any cases.

 - do_bidi() and do_short_shape() are now declared in putty.h. In
   particular, this caused a compile error in terminal.c, since
   do_bidi() is defined with three arguments and terminal.c was only
   passing in two! The third argument appears to be unused as yet,
   though, so I've added `NULL' to the argument list in terminal.c
   and that seems to work so far.

 - You defined two help contexts in winhelp.h without writing
   sections of the manual for them to refer to! I've added the two
   relevant sections to doc/config.but; let me know if you think
   anything is wrong about them.

The resulting binary appears to run fine on Windows, and it looks as
if it's doing _something_ on Unix too (but I don't have any Arabic X
fonts installed so it's difficult to tell).

The revised patch is available at

  http://www.tartarus.org/~simon/patch.putty.arabic

Arabeyes people: please check my version of the patch to make sure
it still works and I haven't broken anything obvious. In particular,
I might have broken ldisc.c, and my suggested documentation might be
wrong.

Owen: can you give your opinion of the functionality?

Also, a couple more things:

 - I'd prefer to find a nicer way to implement doMirror(), but the
   comment at the top of the function suggests that this isn't news
   to you :-)

 - It currently seems to me that bidi and shaping do not happen at
   all if you're using a non-UTF-8 character set such as Win1256. I
   imagine this is because the characters appear in the PuTTY
   terminal structures as D8xx rather than proper Unicode, so the
   bidi and shaping algorithms don't notice them. Does this happen
   for you as well? Does it need to be fixed before checkin?

 - Arabeyes people: for this amount of code, you probably deserve to
   be credited in the PuTTY copyright notice and licence. Who should
   I be crediting, and precisely how?

Cheers,
Simon
-- 
Simon Tatham         "loop, infinite _see_ infinite loop"
<anakin at pobox dot com>     - Index, Borland Pascal Language Guide