More Vim Arabic patch comments

[digitect at mindspring dot com]

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