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

Strings Format: Use context as key #1105

Merged
merged 4 commits into from
May 1, 2020
Merged

Strings Format: Use context as key #1105

merged 4 commits into from
May 1, 2020

Conversation

akirk
Copy link
Member

@akirk akirk commented May 1, 2020

When importing from a .strings file we read the lines as follows (which is correct):

/* $comment */
"$context" = "$singular";

But when exporting, we export like this:

/* $comment */
"$singular" = "$translation";

when it should be

/* $comment */
"$context" = "$translation";

This patch fixes this inconsistency.

@akirk akirk added the [Type] Bug An existing feature is broken. label May 1, 2020
@ocean90 ocean90 added this to the 3.0 milestone May 1, 2020
Copy link
Member

@ocean90 ocean90 left a comment

Choose a reason for hiding this comment

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

Thanks @akirk! I guess this wasn't noticed because in our test file the original and context is always the same? Can we add one which covers this issue?

gp-includes/formats/format-strings.php Outdated Show resolved Hide resolved
@ocean90 ocean90 self-requested a review May 1, 2020 15:40
@AliSoftware
Copy link

Hello @akirk @ocean90 👋 It seems that this PR had the unfortunate consequence of reintroducing #921 – which was fixed by #922 back then, and is where the use of the singular for the key instead of context was introduced.

As a matter of fact, I recently encountered the issue mentioned in #921 (and it took me a while to understand why I had it given that I found #922 that was supposed to have fixed it) for one of our keys that is longer than 255 characters.

@akirk
Copy link
Member Author

akirk commented Jan 4, 2021

Please take a look at my response on that ticket. Short version is: there is no need to use the English text as a context, you can use anything shorter, you just need to specify an "English translation."

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Bug An existing feature is broken.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants