[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: More Vim Arabic patch comments
- To: digitect at mindspring dot com
- Subject: Re: More Vim Arabic patch comments
- From: Nadim Shaikli <shaikli at yahoo dot com>
- Date: Wed, 5 Feb 2003 16:28:12 -0800 (PST)
- Cc: Arabeyes Developers <developer at arabeyes dot org>
--- 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).
Do you Yahoo!?
Yahoo! Mail Plus - Powerful. Affordable. Sign up now.