Bug 118851 - Add extract-localizable-js-strings and use it for WebInspectorUI
Summary: Add extract-localizable-js-strings and use it for WebInspectorUI
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Timothy Hatcher
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-07-18 08:23 PDT by Timothy Hatcher
Modified: 2013-07-18 15:17 PDT (History)
4 users (show)

See Also:


Attachments
Patch (9.16 KB, patch)
2013-07-18 08:27 PDT, Timothy Hatcher
joepeck: review+
joepeck: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Timothy Hatcher 2013-07-18 08:23:21 PDT
Update update-webkit-localizable-strings to use extract-localizable-js-strings for the WebInspectorUI strings.
Comment 1 Timothy Hatcher 2013-07-18 08:27:05 PDT
Created attachment 206992 [details]
Patch
Comment 2 Joseph Pecoraro 2013-07-18 10:59:39 PDT
Comment on attachment 206992 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=206992&action=review

r=me

> Tools/Scripts/extract-localizable-js-strings:59
> +    $file =~ s-^./--;

I have never seen "-" used as the s/// delimiter. It is kind of confusing. Typically I see / or | used.

Also, is this trying to detect a literal '.' or a single character directory? Seems like this should be

    $file =~ s|^\./||;

> Tools/Scripts/extract-localizable-strings:46
> +no warnings 'deprecated';

Why is this needed and not needed before?

> Tools/Scripts/update-webkit-localizable-strings:42
> +my $webInspectorUIFileToUpdate = "../OpenSource/Source/WebInspectorUI/Localizations/en.lproj/localizedStrings.js";

I suspect the "../OpenSource/" part of this path should be removed.
Comment 3 Timothy Hatcher 2013-07-18 11:06:16 PDT
(In reply to comment #2)
> (From update of attachment 206992 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=206992&action=review
> 
> r=me
> 
> > Tools/Scripts/extract-localizable-js-strings:59
> > +    $file =~ s-^./--;
> 
> I have never seen "-" used as the s/// delimiter. It is kind of confusing. Typically I see / or | used.
> 
> Also, is this trying to detect a literal '.' or a single character directory? Seems like this should be
> 
>     $file =~ s|^\./||;

Matches extract-localizable-strings and works, so I'll keep it as-is.

> > Tools/Scripts/extract-localizable-strings:46
> > +no warnings 'deprecated';
> 
> Why is this needed and not needed before?

Prevents error spew hen running update-webkit-localizable-strings that we were just tolerating. So I did this drive-by fix.

> > Tools/Scripts/update-webkit-localizable-strings:42
> > +my $webInspectorUIFileToUpdate = "../OpenSource/Source/WebInspectorUI/Localizations/en.lproj/localizedStrings.js";
> 
> I suspect the "../OpenSource/" part of this path should be removed.

Yep, fixed.
Comment 4 Timothy Hatcher 2013-07-18 15:17:01 PDT
http://trac.webkit.org/changeset/152870