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

Unify %s should be TRUE or FALSE messages #6066

Closed
MichaelChirico opened this issue Apr 10, 2024 · 4 comments · Fixed by #6075
Closed

Unify %s should be TRUE or FALSE messages #6066

MichaelChirico opened this issue Apr 10, 2024 · 4 comments · Fixed by #6075
Assignees
Labels
beginner-task translation issues/PRs related to message translation projects

Comments

@MichaelChirico
Copy link
Member

          This is a pretty common error, better for translation would be `stopf("%s should be TRUE or FALSE", "skip_absent")`, but that's out of scope for this PR.

Originally posted by @MichaelChirico in #6044 (comment)

Currently:

grep -Fr "should be TRUE or FALSE" po/*.pot
po/data.table.pot:msgid "narm should be TRUE or FALSE"
po/data.table.pot:msgid "fill= should be TRUE or FALSE"
@MichaelChirico MichaelChirico added translation issues/PRs related to message translation projects beginner-task labels Apr 10, 2024
@joshhwuu
Copy link
Member

joshhwuu commented Apr 11, 2024

Should I also include this error message:

grep -Fr "should be TRUE" po/*.pot
po/data.table.pot:msgid "use.names= should be TRUE, FALSE, or not used (\"check\" by default)"

Taking a look at po/R-data.table.pot and po/data.table.pot, there are a lot of other warning messages at the R and C level that can be refactored this way, such as

"When by and keyby are both provided, keyby must be TRUE or FALSE"
"'fromLast' must be TRUE or FALSE"
"Internal error in coalesce.c: argument 'inplaceArg' must be TRUE or FALSE"

Should I do a mass refactoring of all error messages with "TRUE or FALSE"? Some have "should be" and some have "must be", or should I just stick to the ones listed above?

@joshhwuu joshhwuu self-assigned this Apr 11, 2024
@MichaelChirico
Copy link
Member Author

the 'fromLast' one can be included, the other two have subtle differences that mean they can't be easily collapsed. If you think they're identical (or close to it), feel free to include in a PR; worst case we'll just revert those during review.

@ben-schwen
Copy link
Member

Out of curiosity, what's the advantage of having %s over putting the argument in the string?

@MichaelChirico
Copy link
Member Author

Out of curiosity, what's the advantage of having %s over putting the argument in the string?

DRY principle for translation. different strings might wind up translated differently even though the English is the same.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beginner-task translation issues/PRs related to message translation projects
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants