[Bug 7150] Implement Arabic shaping

[wine-bugs at winehq.org]

http://bugs.winehq.org/show_bug.cgi?id=7150





--- Comment #15 from Alexander Scott-Johns <alexander.scott.johns+winebug at googlemail.com>  2009-10-26 14:25:54 ---
(In reply to comment #13)
> > Muayyad, wine developers don't grab patches from bugzilla, see
> my beloved brother, as you know I send patches to several project (anaconda,
> publican, gnome-xsl, packagekit, pulseaudio, fedora, ...) and they all
> accept my patches through bugzilla
> 
> could you please please continue with them if I were too busy.

I think the maintainer has Emacs hooked up to the wine-patches mailing list,
and doesn't want to change his workflow...

> > * Please keep the spacing of your code consistent with the rest of the
> > file(4 space indentation, spaces around binary operators, etc).
> 
> my patches only add lines so put the code I added into a file then call a
> indent tool over it with the right arguments winehq wants, that's not a big
> deal  :-) 

Wine is very careful about maintaining authorship info; changing other people's
patches and submitting them isn't liked that much.

> BTW: what are those argument (so that I would do this step if I send
> patches to winehq in the future)
> 
[snip]
> > * Can you please use slightly more descriptive variable names than p, c,
> > cn, ch, jx, jcn, jpc, j, i and ix?
> 
> it's a complicated optimized algorithm, in which variables could be reused,
> but since those variables are locally used within my function only they
> don't worth commenting or being long (as in kernel coding style)

Yes, but this isn't the kernel...
Short variable names are good, but it should be obvious what they mean.

> but don't worry here are the meanings
[snip]
> 
> > * Is it possible to write conformance tests for this? (If so, please
> > include some, as the patch will be much more likely to get in.)
> 
> I'm a native speaker and it works, we ship the patched version in our distro
> (ojuba linux) and we had offered several patched version in parallel to all
> wine updates in fedora 11
> 

Conformance tests are also useful as regression tests, in case someone else
breaks your code later on. Also remember that Wine doesn't try to do things
_correctly_, but to do them the same way Windows does.

> we have not received bug reports related to this patch
[snip]
> shaping will be skipped.
> 
> > * You may find it easier to manage and generate your patches with Git.
> this patch only add lines that are independent of the code, I'm certain
> that can be applied cleanly as-is without any extra effort.
> 
> please try it, and inform me if it did not.

I'm sure it does apply cleanly. It's just that using a CMS (like git) would
make it much easier to rebase (update) from WineHQ when the maintainer applies
patches.

PS: I can't speak/read Arabic, nor do I have any Arabic Windows programs, so I
can't really test the patch or comment on how it works (just how it looks).

-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.