Skip to content

Fix typo: missing n in unknown #1344

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

Merged
merged 1 commit into from
Oct 11, 2023
Merged

Fix typo: missing n in unknown #1344

merged 1 commit into from
Oct 11, 2023

Conversation

noamzaks
Copy link
Contributor

No description provided.

@dyeroshenko dyeroshenko merged commit 4eebe43 into google:main Oct 11, 2023
@mgeisler
Copy link
Collaborator

Hey all! A small note: you should never edit the msgid fields of the PO files. It is only msgmerge that is allowed to update that field. This is mentioned briefly in our documentation.

The problem is that the translations are not using the current English text. Instead they're using the English text as it looked at the time mentioned in the POT-Creation-Date field at the top of the PO file. This was implemented in #1243.

We should revert the edits to the PO files to fix this.

mgeisler added a commit that referenced this pull request Oct 11, 2023
mgeisler added a commit that referenced this pull request Oct 11, 2023
sakex pushed a commit that referenced this pull request Oct 11, 2023
Reverts #1344.

The edit to the Markdown file has been kept since we _do_ want to fix
the typo!
@mgeisler
Copy link
Collaborator

I created #1347 to remove the edits to the PO files.

Perhaps we can create a check somehow which catches this in the future? We've have a few PRs like this where people do the natural thing and fix a typo in all files where it occurs.

@noamzaks
Copy link
Contributor Author

Perhaps we can create a check somehow which catches this in the future? We've have a few PRs like this where people do the natural thing and fix a typo in all files where it occurs.

Do we want to search for any edits of PO files, or specific changes of anything inbetween the msgid and msgstr?

#: src/exercises/day-3/safe-ffi-wrapper.md:26
msgid ""
"`*const i8` to `&CStr`: you need something which can find the trailing `\\0` "
"character"
msgstr ""

I'm assuming only everything between msgid and msgstr?

@mgeisler
Copy link
Collaborator

Perhaps we can create a check somehow which catches this in the future? We've have a few PRs like this where people do the natural thing and fix a typo in all files where it occurs.

Do we want to search for any edits of PO files, or specific changes of anything inbetween the msgid and msgstr?

Good question... The tricky thing is that we do change the msgid fields from time to time — as mentioned in the TRANSLATION file.

#: src/exercises/day-3/safe-ffi-wrapper.md:26
msgid ""
"`*const i8` to `&CStr`: you need something which can find the trailing `\\0` "
"character"
msgstr ""

I'm assuming only everything between msgid and msgstr?

Yes, correct: the text there is the original English text. It has to match the text in the Markdown for us to translate it correctly (via mdbook-i18n-helpers).

Saying this, I get an idea : I think we can spot correct edits of the msgid fields by looking for a chance in the POT-Creation-Date field. When someone runs msgmerge to add in new English strings, the field will change (it is copied from the messages.pot file).

mgeisler pushed a commit that referenced this pull request Oct 11, 2023
See discussion in #1344.

This checks that all `.po` files changed in pull requests either contain
changes in POT-Creation-Date (i.e. are run by the specific tool which
should change msgid's), or don't change msgid's (by checking that each
line changed is NOT between msgid and msgstr)

I have tested this on my fork, see [valid
changes](https://github.com/noamzaks/comprehensive-rust/pull/1/files)
where the action passes, and [invalid
changes](https://github.com/noamzaks/comprehensive-rust/pull/2/files)
where it doesn't.

The action also has output for failing/passing:

[passing](https://github.com/noamzaks/comprehensive-rust/actions/runs/6488171290/job/17620037473?pr=1)

[failing](https://github.com/noamzaks/comprehensive-rust/actions/runs/6488175171/job/17620050716?pr=2)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants