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

Numerus form not applied for trip summary in en-GB #3256

Closed
1 of 4 tasks
schtibal opened this issue May 27, 2021 · 13 comments
Closed
1 of 4 tasks

Numerus form not applied for trip summary in en-GB #3256

schtibal opened this issue May 27, 2021 · 13 comments

Comments

@schtibal
Copy link

Describe the issue:

  • Bug
  • Change request
  • New feature request
  • Discussion request

Issue long description:

The summary line for dive trip shows "(# dive(s))" instead of using the numerus form from translation file (ie: "1 dive" vs "2 dives")

Operating system:

Linux and Windows

Subsurface version:

v5.0.1

Steps to reproduce:

  1. Select two or more dives from the dive list
  2. Right-click, select "Create new trip above"

Current behavior:

The trip summary line is shown as "<title>, (x dive(s))"

Expected behavior:

Trip summary line should show "<title>, (x dives)" if multiple dives are in the trip
or
"<title>, (1 dive)" if only one dive is in the trip

Additional information:

Mentions:

trip_numerus

@dirkhh
Copy link
Collaborator

dirkhh commented May 27, 2021

So our translations come from Transifex, where these plurals need to be handled in the slightly awkward UI. It's a bit hard to search for them, but searching for dive(s) is a good start.

You can see which ones are wrong using this command on the Subsurface sources:

grep numerus translations/subsurface_en_GB.ts | grep '(s)'

So this is the first part - just fix those translations on Transifex (and let me know once you've done this so I can pull the new translations into Subsurface sources).

But the second part is more interesting. There are more texts that aren't identified as numerus forms in the sources. I believe that in most cases this is intentional (because at the time the string is set, we don't know how many items there are / will be), but a quick hunt through the sources seems to indicate that there are some where it might be worth while fixing that. You can get those via

git grep '(s)' | grep 'tr(' | grep -v '%n'

I'm not saying that you should figure out how to fix all of them - I'm just trying to make sure you understand that not all of the cases where we have a (s) for both singular and plural are already marked for translation as numerus form.

@schtibal
Copy link
Author

So this is the first part - just fix those translations on Transifex (and let me know once you've done this so I can pull the new translations into Subsurface sources).

Yes, the Transifex UI is a little bit awkward. It looks like I have access to suggest edits, but not to confirm them. I've started with adding suggestions for 2002 and 2533. I assume you will need to confirm those edits and then pull them back into Subsurface?

@dirkhh
Copy link
Collaborator

dirkhh commented May 28, 2021

No, we don't use the 'review' part of their UI. We trust that people work with good intentions...
If you made changes, they should show up the next time I pull the strings (long complex story but that won't happen before the weekend)

@schtibal
Copy link
Author

I'm not able to save an edit, I can only save as a suggestion:

image

@dirkhh
Copy link
Collaborator

dirkhh commented May 28, 2021

Have you joined the Subsurface team?

@schtibal
Copy link
Author

schtibal commented May 28, 2021

Have you joined the Subsurface team?

Yep. And I'm listed as one of the members translating to English (United Kingdom).

I've gone into the translations for the project:
image

Then I can save an edit as a suggestion:
image

If I leave an go back into that string, I can select to "Use this" translation from the 1 or Other suggestions, at which stage in the suggestion list it shows as "used":
image

But there doesn't appear to be a way to commit the change - the "Save Changes" option stays greyed out. I either don't have the correct permissions for something, or there is a severe PEBCAK going on here!

@dirkhh
Copy link
Collaborator

dirkhh commented May 28, 2021

I wonder if this was because another translator marked those strings as 'reviewed'...
I removed that tag from all of the strings. Can you try again, please?

@schtibal
Copy link
Author

schtibal commented May 28, 2021

Yes, that appeared to work. I've saved the text for contexts DiveTripModelBase (qt-models/divetripmodel.cpp:76) and gettextFromC (core/qthelper.cpp:1054)

@schtibal
Copy link
Author

So the dive trip summary text was already numerus, and just required the language file to be updated.
The dive list context menu was not numerus, and I've updated divelistview.cpp for numerus translations - I assume now that I hold off making the Transifex updates until a PR has been approved?

Speaking of which .. can be added as a collaborator so I can push my branch up?

image

@schtibal
Copy link
Author

schtibal commented Jun 7, 2021

@dirkhh now that you've merged my code changes into the master, I've gone to add the numerus forms into Transifex, but I can't find (or don't have permission) to add a numerus form to an existing string. If the string already has a numerus form. I can only edit the singular/plural versions if they already exist.

Edit: or is there a push/sync process from the source to transifex which does this?

@dirkhh
Copy link
Collaborator

dirkhh commented Jun 7, 2021

I have to push the new strings. Sorry - I am not being nearly as responsive as I should be regarding your changes.
This will happen in the next couple of hours

@dirkhh
Copy link
Collaborator

dirkhh commented Jun 7, 2021

this has happened I can see the strings in Transifex now

@schtibal
Copy link
Author

schtibal commented Jun 8, 2021

this has happened I can see the strings in Transifex now

And translations are done.

Edit. Oops, translations are done for en-GB. Will roll out to en-US when I get a chance.

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 a pull request may close this issue.

2 participants