Re: More Vim Arabic patch comments

[Nadim Shaikli <shaikli at yahoo dot com>]

--- digitect mindspring com wrote:
> 
> Nadim, below are some additional (final?) Vim Arabic patch comments
> and clarifications from those previous.
> 
> As a side note, I'm not a programmer--you certainly do not want my
> help making any adjustments to your patch! (Or I would.) I was hoping
> yourself or other Arabeyes developers can react to these, as I'm only
> able to be a liaison between the patch and those that include it. ;)
> 
> 
> Steve Hall  [ <digitect>at<mindspring>dot<com> ]
> 
> ---------------------------------------------------------------------
> > > From: Steve Hall <digitect at mindspring dot com>
> > >
> > > 1. FEAT_RIGHTLEFT combined with FEAT_ARABIC:  not a problem
> > > 2. header file or forward declare cleanup:    minor problem
> > > 3. :set noarabic broken:                      will look into
> > > 4. documentation:                             will do
> >
> > I'll add a few notes below.  I don't think I communicated the issue
> > for #1 quite clearly.
> > 
> > > Nadim Shaikli wrote:
> > >
> > > > > > Another thing is making the features more self-contained,
> > > > > > when possible.  I was looking through the patch and most
> > > > > > of the changes it makes are bounded by #ifdef
> > > > > > FEAT_RIGHTLEFT.  Since FEAT_RIGHTLEFT was already being used
> > > > > > with Farsi and Hebrew, then it seems the changes made by the
> > > > > > Arabic patch can potentially affect the other two as well.
> > > > > > If the changes are only supposed to help with Arabic
> > > > > > support, it seems like they should be bounded by #ifdef
> > > > > > FEAT_ARABIC instead.  Changes that are to enhance rightleft
> > > > > > support in general can be left as-is, but perhaps submitted
> > > > > > as a separate patch, which could be included / tested
> > > > > > independently of the Arabic support.
> > > >
> > > > Arabic requires RIGHTLEFT support and as such it was needed to
> > > > ensure that Arabic is only available if RIGHTLEFT is
> > > > compiled-in. The fact that Farsi (I'm not sure that anyone
> > > > really uses this - Farsi is very close to Arabic and their
> > > > implementation of farsi seems very lacking) and hebrew use
> > > > RIGHTLEFT has no baring on anything really.  In short, its a
> > > > requirement they all share which they all require.
> > 
> > Right, I understand that RIGHTLEFT is required for Arabic.  What I
> > was trying to say is, if someone compiles Vim with FEAT_RIGHTLEFT
> > but not FEAT_ARABIC, they are still going to be affected by the code
> > you added that is surrounded by #ifdef FEAT_RIGHTLEFT.  If this code
> > is meant to enhance RIGHTLEFT mode in general, it could be submitted
> > as a separate patch from the #ifdef FEAT_ARABIC code.  This would
> > allow it to be tested by anyone using RIGHTLEFT mode, regardless of
> > whether they use FEAT_ARABIC or not.  On the other hand, if the new
> > FEAT_RIGHTLEFT code is only necessary for the Arabic stuff, perhaps
> > it should be delimited by #ifdef FEAT_ARABIC instead.
> > 
> > This isn't so much a problem, just a perception thing that may help
> > get parts of the patch some more testing.  Since it appears that
> > anytime FEAT_RIGHTLEFT is defined, FEAT_ARABIC is too (unless
> > someone edits feature.h), there probably isn't much use testing them
> > separately anyway.

OK - will keep this in mind.

> > > > > > There is still a bit of cleanup work to do.  I noticed when
> > > > > > compiling under MSVC++ that I got warnings about undefined
> > > > > > references to arabic_combine() and shape_arabic(), so it
> > > > > > assumed extern returning int.  This is just a header file or
> > > > > > forward declare issue that should be easy to resolve.  Also,
> > > > > > so variables they add were reported as 'declared but unused'
> > > > > > when compiling the console version, which means they need to
> > > > > > be wrapped in an appropriate #ifdef.
> > > >
> > > > I will look into this - but it is a simple enough thing to
> > > > fix/clean-up.  Steve feel free to do it or encourage this person
> > > > to do so as he sees fit (let me know what the changes were for
> > > > me to add them on if need be along with any other system
> > > > requirements).
> > 
> > As a small style note, to be consistent with the rest of the Vim
> > code, you may want to change groups like
> > 
> > #ifdef FEAT_ARABIC
> > #ifdef FEAT_GUI
> > ...
> > #endif
> > #endif
> > 
> > To something a little more compact, like
> > 
> > #if defined(FEAT_ARABIC) && defined(FEAT_GUI)
> > ...
> > #endif
> > 

Actually I followed the VIM code and its formating - what you note might
be readable to me and you, but I opted to attempt to follow the style
I saw in the files.

> > > > I do remember it was a short-cut for involving rightleft mode 
> > > > > (and possibly something else - donn't remember) - I'll add it
> > > > to my list of things to look into (ie. the noarabic part).
> > 
> > I was playing with it some more Friday, actually getting Arabic text
> > (using instructions I saw in the arabeyes developer archive) and
> > noticed a small problem under Win32.  When I do a search, the /
> > starts out on the right side of the screen, but after typing a few
> > characters, it jumps to the left and acts like left-to-right mode.
> > The search still finds the word correctly, but visually it is like
> > typing /word to match 'drow'.  I know you said the project is mainly
> > for Unix Arabic support, but it should really be cleaned up in Win32
> > if possible as well.

This doesn't happen on unix/linux - sorry can't help you with win32.
If you see issues that require win32 cleanup, feel free to let me know
and I'll role them in.

> > > > True. I will spend some time adding the necessary short
> > > > documentation. You know how it is when it comes to
> > > > documentation, but if this is the one serious complaint/comment
> > > > so far - this is great.
> > 
> > You have done a very nice job.  And remember, these weren't meant as
> > complaints.  They were suggestions to hopefully help speed adoption
> > of the patch into the main source.  I saw that you got some other
> > input from Tony, which is good.  If you release a new patch based
> > on his or my notes, you may want to post a message to the vim-dev
> > (and perhaps even the vim) lists pointing to it.  Steve and Jeremy
> > are a big help creating binaries including it, but many people
> > (especially developers) compile their own, and would have to include
> > the patch themselves.

Thanks for your comments - I was on the vim list for a very long time
to no avail and have since been swamped with lots of other work (and
thus my un-subscription).

Regards,

 - Nadim


__________________________________________________
Do you Yahoo!?
Yahoo! Mail Plus - Powerful. Affordable. Sign up now.
http://mailplus.yahoo.com