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

Contributions Managers now show specific titles (like "Mode Manager", "Tool Manager", etc.) in the user's preferred language #2777

Merged
merged 1 commit into from Aug 19, 2014

Conversation

Projects
None yet
3 participants
@joelmoniz
Copy link
Member

joelmoniz commented Aug 11, 2014

There was no contributions field in the PDE.properties, which caused a MissingResourceException to be thrown each time a Tool/Mode/Library Manager was started. Further, we may need the title String, since each of the managers will need their entire title specified (since concatenating the type.getTitle() with " Manager" would no longer work, since, for example, in French, it would be written the other way around).

@federicobond

This comment has been minimized.

Copy link
Contributor

federicobond commented Aug 11, 2014

I attempted to fix this in #2770. If your PR gets merged first, I can do a rebase.

@joelmoniz

This comment has been minimized.

Copy link
Member

joelmoniz commented Aug 11, 2014

Oh my. Sorry. I didn't notice. I'll close this.

Would adding fields for the individual contribution managers be helpful, so that the title bars read "Tool Manager", "Mode Manager", etc.? If so, I'll either submit a PR onto the branch which you've filed the PR with, or I'll wait till your PR gets accepted.

Thanks!

@benfry

This comment has been minimized.

Copy link
Member

benfry commented Aug 12, 2014

Where are you getting a MissingResourceException? I thought I added code to work around that and return the default string?

@federicobond

This comment has been minimized.

Copy link
Contributor

federicobond commented Aug 12, 2014

The exception is probably thrown in the bundle.getString(text) method, so you never reach the null check in Language.text. By default string you mean the key or the english translation? I could submit a pull request to fix this.

@benfry

This comment has been minimized.

Copy link
Member

benfry commented Aug 12, 2014

The intent was to pass back the key (since that would help us debug), for cases where the English version wasn't even present. It had been throwing a NullPointerException before, so I did a null-check instead, so the MissingResourceException is news.

@federicobond

This comment has been minimized.

Copy link
Contributor

federicobond commented Aug 12, 2014

I rather like the MissingResourceException. It gives you the key and fails loudly, which I think is good for missing English language keys.

One other thing, regarding @joelmoniz's comment. Many language strings in the codebase attempt to interpolate the contribution type inside. This can be a problem for some languages if the words are of different grammatical gender. For example, in english you can talk about the mode or the tool, but in spanish you need to use el modo and la herramienta.

This is why you will find that the spanish translation usually talks about la contribución and ignores the interpolation argument.

@benfry

This comment has been minimized.

Copy link
Member

benfry commented Aug 12, 2014

But the exception has caused problems with the PDE actually running, which is unacceptable.

@joelmoniz

This comment has been minimized.

Copy link
Member

joelmoniz commented Aug 13, 2014

So, would it be better to have separate fields for Mode Manager, Tool Manager, Library Manager, etc., in which case I'll rebase this? The rest of this PR has already been implemented in PR #2270.

@federicobond

This comment has been minimized.

Copy link
Contributor

federicobond commented Aug 13, 2014

Yes, in this case where the distinction is important I believe we should use separate keys.

@joelmoniz

This comment has been minimized.

Copy link
Member

joelmoniz commented Aug 13, 2014

Thanks.

I've rebased it now to have the separate keys, and updated the issue title accordingly.

@joelmoniz joelmoniz changed the title Fixed Contributions Managers not showing due to MissingResourceException Contributions Managers now show specific titles (like "Mode Manager", "Tool Manager", etc.) in the local language Aug 13, 2014

@joelmoniz joelmoniz changed the title Contributions Managers now show specific titles (like "Mode Manager", "Tool Manager", etc.) in the local language Contributions Managers now show specific titles (like "Mode Manager", "Tool Manager", etc.) in the user's preferred language Aug 13, 2014

benfry added a commit that referenced this pull request Aug 19, 2014

Merge pull request #2777 from joelmoniz/fixContribManagersNotSh
Contributions Managers now show specific titles (like "Mode Manager", "Tool Manager", etc.) in the user's preferred language

@benfry benfry merged commit 3cc7b6d into processing:master Aug 19, 2014

@joelmoniz joelmoniz deleted the joelmoniz:fixContribManagersNotSh branch Dec 14, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment