Skip to content

Conversation

@PradeepFSTdhane123
Copy link

@PradeepFSTdhane123 PradeepFSTdhane123 commented Feb 9, 2025

Closes #6794

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.

@codecov
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).
Report is 19 commits behind head on master.

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 _(String) dgettext("data.table", String)
// NB: flip argument order to match that of R's ngettext()
#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.

@MichaelChirico MichaelChirico changed the title Issue - #6794 Set up extraction of plural strings from C Feb 24, 2025
@MichaelChirico
Copy link
Member

I will close here for now. As noted, this issue in particular doesn't have much to do on the {data.table} side -- almost all of the work will happen in {potools}.

If you'd like to contribute there, awesome!

But if you're mainly interested in contributing directly to {data.table}, there may be other issues more worth your time.

Thanks!

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.

Failed to extract Pl_() strings to .pot

3 participants