-
Notifications
You must be signed in to change notification settings - Fork 997
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
Issue - #6794 #6805
base: master
Are you sure you want to change the base?
Issue - #6794 #6805
Conversation
issue no-6794. solving this issue
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #6805 +/- ##
=======================================
Coverage 98.64% 98.64%
=======================================
Files 79 79
Lines 14639 14639
=======================================
Hits 14440 14440
Misses 199 199 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this pull request complete? If not, it's best to mark it as draft to avoid wasting reviewer time.
Instead of mentioning the issue number in the title, please mention it in the text of the pull request. GitHub doesn't do much with issue numbers in the pull request titles, but it will gladly link the two together if you move it into the comment section.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please don't commit extraneous files (produced by your editor?). When contributing to an open source project, try to minimise your changes as much as possible. This file should be removed from the pull request (but feel free to keep it locally, uncommitted).
#define Pl_(n, String1, StringPlural) dngettext("data.table", String1, StringPlural, n) | ||
#define Pl_Extract(String1, StringPlural) (String1 "\0" StringPlural) // Helps extract both strings |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please explain your plan for making this work? As submitted, this doesn't help potools::po_extract
actually find the strings marked using the Pl_
macro.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@aitap Sorry for this mistake. I appreciate your feedback and will make the necessary improvements.
The macro adds the plural string by concatenating it with the singular form using "\0".
This ensures both forms are present in the source code for translation tools like xgettext.
It does not affect runtime behavior but helps extract both singular and plural strings.
When used, only the singular appears in printf(), but both forms exist in memory.