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

Issue - #6794 #6805

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

Conversation

PradeepFSTdhane123
Copy link

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.

Copy link

codecov bot commented Feb 9, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.64%. Comparing base (b7f2106) to head (39317bb).

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.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@aitap aitap left a 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.

Copy link
Contributor

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
Copy link
Contributor

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.

Copy link
Author

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.

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.

2 participants