Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support Lam-Alef ligatures #4

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

michaeldiscala
Copy link

We use the arabic-letter-connector gem with Prawn to render PDFs. One of our clients reported that the content we generated for them was very difficult to read because we were not correctly joining the Lam Alef characters. We've sent new proofs to the client for review (they'll be better able to determine if we're rendering the ligatures correctly), but I figured in the meantime I could see if this is a patch that you're interested in. Definitely open to feedback (especially since I don't speak Arabic).

--Mike
P.S. Love the Gem -- thanks so much for putting this together

@michaeldiscala
Copy link
Author

Based on feedback from our client, I made a few changes in 2 additional commits:

  1. lam-alef characters are no longer marked as allowing connections
  2. Diacritic characters are now supported (they are not replaced at all -- just marked as allowing connections)
  3. Standalone hamza characters no longer cause the preceding character to be in the medial form (hamzas seem to be an exception and do not allow connections from previous characters)

@michaeldiscala
Copy link
Author

After reviewing with our client, I realized that the hamza character change was not always correct. I have added a commit to revert that change.

I'm fairly confident in the patch at this point, with its final feature set being:

  1. support for diacritical marks
  2. support for lam-alef ligatures

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant