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

Made all alerts in QSController.m localisable. #568

Merged
merged 5 commits into from May 15, 2012

Conversation

pjrobertson
Copy link
Member

@pjrobertson pjrobertson commented Nov 11, 2011

Localizable.strings just needs editing for each language

Please note: If I have wrapped a string in NSLocalizedString(@"Some String",nil) BUT @"Some String" isn't in Localised.strings then Quicksilver will most likely crash.

I have checked twice that I have done this all properly, but a third check from another would be appreciated.

Localizable.strings just needs editing for each language
@skurfer
Copy link
Member

@skurfer skurfer commented Nov 14, 2011

Please note: If I have wrapped a string in NSLocalizedString(@"Some String",nil) BUT @"Some String" isn't in Localised.strings then Quicksilver will most likely crash.

So if you use something other than English, is that a guaranteed crash?

@pjrobertson
Copy link
Member Author

@pjrobertson pjrobertson commented Nov 14, 2011

So if you use something other than English, is that a guaranteed crash?

No. QS's default is to always drop back to the English if other languages
don't exist. If there's no English though it doesn't know what to drop back
to... crash! (although the crash shouldn't actually happen, I think that's
a bug)

On 14 November 2011 04:29, Rob McBroom <
reply@reply.github.com

wrote:

Please note: If I have wrapped a string in NSLocalizedString(@"Some
String",nil) BUT @"Some String" isn't in Localised.strings then Quicksilver
will most likely crash.

So if you use something other than English, is that a guaranteed crash?


Reply to this email directly or view it on GitHub:
#568 (comment)

@HenningJ
Copy link
Contributor

@HenningJ HenningJ commented Nov 14, 2011

Please note: If I have wrapped a string in NSLocalizedString(@"Some String",nil) BUT @"Some String" isn't in Localised.strings then Quicksilver will most likely crash.

This might be my fault. Last time I fiddled with Localization, I changed a lot of stuff.
I think I found the problem. In NSBundle_BLTRExtensions.m Line 19 the line

[[NSBundle mainBundle] safeLocalizedStringForKey:(key) value:nil table:nil]

should be changed to

[[NSBundle mainBundle] safeLocalizedStringForKey:(key) value:(key) table:nil]

That way, if all localization attempts fail, the original, non-localized string (@"Some String" in this case) will be used.
The same goes for line 23 and 27. Just replace value:nil with value:(key).

I can't make the changes myself right now. So, @pjrobertson, will you just include them in this pull request?

@pjrobertson
Copy link
Member Author

@pjrobertson pjrobertson commented Nov 14, 2011

Yeah I definitely thought it was a but.

I don't think it's just a bug with other languages (I get crashes when just
using English)
Seems to be that if no string if found, 'nil' is being returned as opposed
to a placeholder such as @""
That's what really should be done.

On 14 November 2011 12:43, Henning Jungkurth <
reply@reply.github.com

wrote:

Please note: If I have wrapped a string in NSLocalizedString(@"Some
String",nil) BUT @"Some String" isn't in Localised.strings then Quicksilver
will most likely crash.

This might be my fault. Last time I fiddled with Localization, I changed a
lot of stuff.
I think I found the problem. In NSBundle_BLTRExtensions.m Line 19
the line

[[NSBundle mainBundle] safeLocalizedStringForKey:(key) value:nil
table:nil]

should be changed to

[[NSBundle mainBundle] safeLocalizedStringForKey:(key) value:(key)
table:nil]

That way, if all localization attempts fail, the original, non-localized
string (@"Some String" in this case) will be used.
The same goes for line 23 and 27. Just replace value:nil with
value:(key).

I can't make the changes myself right now. So, @pjrobertson, will you just
include them in this pull request?


Reply to this email directly or view it on GitHub:
#568 (comment)

@HenningJ
Copy link
Contributor

@HenningJ HenningJ commented Nov 14, 2011

With the changes I proposed, it would return the original string you put in, instead of nil.

If you'd rather return @"", you should change NSBundle_BLTRExtensions.m#L77 instead. From:

return defaultValue;

to something like:

if (defaultValue) {
    return defaultValue;
}
return @"<String Not Found>";

This way, it would return the default value if you provided one (using NSLocalizedStringWithDefaultValue), otherwise it would return @"".

@@ -695,6 +696,72 @@ - (void)checkForFirstRun {
}

- (void)checkForCrash {
<<<<<<< HEAD
Copy link
Member

@skurfer skurfer May 14, 2012

Choose a reason for hiding this comment

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

I don’t think this is valid Objective-C. ;-)

skurfer added a commit that referenced this issue May 15, 2012
Made all alerts in QSController.m localisable.
@skurfer skurfer merged commit 6b8c551 into quicksilver:master May 15, 2012
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 this pull request may close these issues.

None yet

3 participants