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

Add ordinal adverb support #63

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jvarn
Copy link
Contributor

@jvarn jvarn commented Jul 6, 2023

By adding a new number order option (arNumberOrder=3) with default gender (arNumberFeminine=1) this adds support for ordinal adverbs (i.e. firstly, secondly, thirdly).

(Existing) ordinal numbers with arNumberOrder=2

  • الأول
  • الثاني
  • الثالث
  • الرابع
  • الخامس
  • السادس
  • السابع
  • الثامن
  • التاسع
  • العاشر
  • الحادي عشر
  • الثاني عشر
  • الثالث عشر
  • الرابع عشر
  • الخامس عشر
  • السادس عشر

(New) ordinal adverbs with arNumberOrder=3

  • أولاً
  • ثانياً
  • ثالثاً
  • رابعاً
  • خامساً
  • سادساً
  • سابعاً
  • ثامناً
  • تاسعاً
  • عاشراً
  • الحادي عشر
  • الثاني عشر
  • الثالث عشر
  • الرابع عشر
  • الخامس عشر
  • السادس عشر

Ordinal adverbs (firstly, secondly, thirdly) using arNumberOrder=3
@khaled-alshamaa
Copy link
Owner

Hi @jvarn, I am sorry to be late in reviewing your pull request! Your idea is valid and has added value, but I need to review the processing logic a little within the coming couple of days. The first impression made me hesitate about the robustness and completeness of handling all possible scenarios in a clear way.

@jvarn
Copy link
Contributor Author

jvarn commented Jul 31, 2023

Thank you, Khaled. I agree, this implementation isn't particularly future-proof or user-friendly. Using 1,2 for a binary option is fair enough, but 1,2,3 is inelegant, to say the least. I endeavoured to implement the required logic based upon existing code without any refactoring or major additions, and couldn't think of a better way to do it :-)

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

2 participants