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

Message fixes for the C code #6504

Merged
merged 20 commits into from
Sep 20, 2024
Merged

Message fixes for the C code #6504

merged 20 commits into from
Sep 20, 2024

Conversation

aitap
Copy link
Contributor

@aitap aitap commented Sep 17, 2024

This solves some of the obvious issues in #6503. I can expand on the more tough or less obvious ones when we decide which ones to fix and how.

  • regenerate po/data.table.pot

Otherwise it's very hard to translate without pgettext() in a language
where "there is no <x>" is translated as "<x> is absent"
This will avoid potential word order issues.
.Last.value is a variable in the global environment. .Last.updated belongs to the data.table package.
Copy link

github-actions bot commented Sep 17, 2024

Comparison Plot

Generated via commit 384fe62

Download link for the artifact containing the test results: ↓ atime-results.zip

Time taken to finish the standard R installation steps: 3 minutes and 22 seconds

Time taken to run atime::atime_pkg on the tests: 6 minutes and 53 seconds

@aitap aitap requested a review from tdhock as a code owner September 17, 2024 13:10
src/fsort.c Outdated Show resolved Hide resolved
Copy link
Member

@MichaelChirico MichaelChirico left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! A few small questions. Fixes basically look good.

@MichaelChirico MichaelChirico added the translation issues/PRs related to message translation projects label Sep 17, 2024
src/fread.c Outdated Show resolved Hide resolved
src/fmelt.c Outdated Show resolved Hide resolved
src/fmelt.c Outdated Show resolved Hide resolved
src/fmelt.c Outdated Show resolved Hide resolved
src/fsort.c Outdated Show resolved Hide resolved
src/fmelt.c Outdated Show resolved Hide resolved
aitap and others added 4 commits September 18, 2024 17:23
Hopefully the compiler will be smart enough to inline it back.

Co-authored-by: Michael Chirico <[email protected]>
@MichaelChirico
Copy link
Member

Don't regenerate the .pot file in this PR, save it for a follow-up

src/fsort.c Outdated Show resolved Hide resolved
src/fsort.c Outdated Show resolved Hide resolved
}
MSBsize = shrinkMSB(MSBsize, msbCounts, order, verbose);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might have assumed this helper would be by-reference on MSBsize; OTOH, it's a single size_t and this is only done once in fsort(). The impact should not be noticed.

Copy link
Member

@MichaelChirico MichaelChirico left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work + progress! I think this is ready to merge. We can always do more PRs with more fixes :)

@MichaelChirico MichaelChirico merged commit 0e257a8 into master Sep 20, 2024
7 of 8 checks passed
@MichaelChirico MichaelChirico deleted the message-fixes branch September 20, 2024 03:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
translation issues/PRs related to message translation projects
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants