Updating file path support for Windows 7 64 bit #263
Conversation
|
Same change needs to be made for the cordova repo as well @bhiggins advise please Sent from my BlackBerry 10 smartphone. From: Erik Johnson You can merge this Pull Request by running git pull https://github.com/blackberry-webworks/BB10-Webworks-Packager erik/i18n Or view, comment on, or merge it at: Commit Summary
File Changes
Patch Links:
This transmission (including any attachments) may contain confidential information, privileged material (including material protected by the solicitor-client or other applicable privileges), or constitute non-public information. Any use of this information by anyone other than the intended recipient is prohibited. If you have received this transmission in error, please immediately reply to the sender and delete this information from your system. Use, dissemination, distribution, or reproduction of this transmission by unintended recipients is not authorized and may be unlawful. |
|
Tested on Windows 7 and XP. Both the icon and the splash screen change after changing the language. |
|
patch applied to cordova repo |
|
tested on winxp and mac: icon and splash screen worked well when changing the language. |
| @@ -127,7 +127,7 @@ function generateLocalizedMetadata(session, config, xmlObject, key) { | |||
| if (pkgrUtils.isWindows()) { | |||
|
|
|||
| localeFiles.forEach(function (file) { | |||
| file = path.relative(path.resolve(session.sourceDir, "locales"), file).replace(/\\/g, "/"); | |||
There was a problem hiding this comment.
Are we sure we aren't breaking something here? Seems odd that we would go to the trouble of resolving the locales directory and then determining the relative path from it when it's not required. Do we really just need to replace slashes here? @rwmtse Were you the original implementor of this code? Are we breaking some use cases here? Thoughts?
There was a problem hiding this comment.
Doesn't path.normalize change the slashes appropriately? It would seem better to use that instead of find and replace.
There was a problem hiding this comment.
FYI, I remember seeing some weird behavior with path.normalize in that it would convert forward slashes to backslashes for Windows, but not the other way around.
There was a problem hiding this comment.
@erikjohnzon and I tested and it converted to windows fine, but it seemed to do double blackslashes which may be the root of the issue.
There was a problem hiding this comment.
If @rwmtse has no other input and it passes all of our tests no reason not to go ahead with it.
|
bump. |
|
@jkeshavarzi @jeffheifetz are there still concerns with this? it has been outstanding for a while now |
|
@jkeshavarzi @jeffheifetz ? |
|
Should this be closed now, or pulled in? |
No description provided.