[GitHub] cordova-plugin-file pull request: CB-11305 Enable cdvfile: assets ...
Github user jasongin commented on the pull request: https://github.com/apache/cordova-plugin-file/pull/182#issuecomment-222021395 :shipit: --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-plugin-file pull request: CB-11305 Enable cdvfile: assets ...
Github user jasongin commented on a diff in the pull request:
https://github.com/apache/cordova-plugin-file/pull/182#discussion_r64836957
--- Diff: tests/tests.js ---
@@ -3441,6 +3441,27 @@ exports.defineAutoTests = function () {
}
});
});
+describe('resolveLocalFileSystemURL on cdvfile://', function () {
+it("file.spec.147 should be able to resolve cdvfile
applicationDirectory fs root", function(done) {
+var cdvfileApplicationDirectoryFsRootName;
+if (cordova.platformId === 'android') {
+cdvfileApplicationDirectoryFsRootName = 'assets';
+} else if (cordova.platformId === 'ios') {
+cdvfileApplicationDirectoryFsRootName = 'bundle';
+} else {
--- End diff --
I guess since "assets" is already defined and working with
resolveLocalFileSystemURL we can't change it.
---
In reply to:
[64836116](https://github.com/apache/cordova-plugin-file/pull/182#discussion_r64836116)
[](ancestors = 64836116)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-plugin-file pull request: CB-11305 Enable cdvfile: assets ...
Github user jasongin commented on a diff in the pull request:
https://github.com/apache/cordova-plugin-file/pull/182#discussion_r64836116
--- Diff: tests/tests.js ---
@@ -3441,6 +3441,27 @@ exports.defineAutoTests = function () {
}
});
});
+describe('resolveLocalFileSystemURL on cdvfile://', function () {
+it("file.spec.147 should be able to resolve cdvfile
applicationDirectory fs root", function(done) {
+var cdvfileApplicationDirectoryFsRootName;
+if (cordova.platformId === 'android') {
+cdvfileApplicationDirectoryFsRootName = 'assets';
+} else if (cordova.platformId === 'ios') {
+cdvfileApplicationDirectoryFsRootName = 'bundle';
+} else {
--- End diff --
Should we consider using the same root name all platforms to represent the
application root installation directory? Otherwise developers are going to have
to use similar if-else conditions in their code.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-windows pull request: CB-11117: Optimize prepare for windo...
Github user jasongin commented on a diff in the pull request:
https://github.com/apache/cordova-windows/pull/174#discussion_r64834967
--- Diff: template/cordova/lib/prepare.js ---
@@ -425,19 +451,39 @@ module.exports.prepare = function (cordovaProject) {
AppxManifest.purgeCache();
// Update own www dir with project's www assets and plugins' assets
and js-files
-return Q.when(updateWwwFrom(cordovaProject, this.locations))
+return Q.when(updateWww(cordovaProject, this.locations))
.then(function () {
// update project according to config.xml changes.
return updateProjectAccordingTo(self._config, self.locations);
})
.then(function () {
-copyImages(cordovaProject.projectConfig, self.root);
+copyImages(cordovaProject, self.locations);
})
.then(function () {
events.emit('verbose', 'Prepared windows project successfully');
});
};
+module.exports.clean = function (options) {
+// A cordovaProject isn't passed into the clean() function, because it
might have
+// been called from the platform shell script rather than the CLI.
Check for the
+// noPrepare option passed in by the non-CLI clean script. If that's
present, or if
+// there's no config.xml found at the project root, then don't clean
prepared files.
+var projectRoot = path.resolve(this.root, '../..');
+var projectConfigFile = path.join(projectRoot, 'config.xml');
--- End diff --
I updated the PRs for all 3 platforms to use the platform's config.xml for
cleaning.
They still check for the presence of config.xml at the project root,
because I want to be extra careful not to do this clean operation in a
platform-centric workflow.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-windows pull request: CB-11117: Optimize prepare for windo...
Github user jasongin commented on a diff in the pull request:
https://github.com/apache/cordova-windows/pull/174#discussion_r64688561
--- Diff: template/cordova/lib/prepare.js ---
@@ -425,19 +451,39 @@ module.exports.prepare = function (cordovaProject) {
AppxManifest.purgeCache();
// Update own www dir with project's www assets and plugins' assets
and js-files
-return Q.when(updateWwwFrom(cordovaProject, this.locations))
+return Q.when(updateWww(cordovaProject, this.locations))
.then(function () {
// update project according to config.xml changes.
return updateProjectAccordingTo(self._config, self.locations);
})
.then(function () {
-copyImages(cordovaProject.projectConfig, self.root);
+copyImages(cordovaProject, self.locations);
})
.then(function () {
events.emit('verbose', 'Prepared windows project successfully');
});
};
+module.exports.clean = function (options) {
+// A cordovaProject isn't passed into the clean() function, because it
might have
+// been called from the platform shell script rather than the CLI.
Check for the
+// noPrepare option passed in by the non-CLI clean script. If that's
present, or if
+// there's no config.xml found at the project root, then don't clean
prepared files.
+var projectRoot = path.resolve(this.root, '../..');
+var projectConfigFile = path.join(projectRoot, 'config.xml');
--- End diff --
That's a reasonable question. I didn't want the clean function to depend on
a previous prepare having completed successfully (so that the platform
config.xml was merged). But I'm not sure that's best, and maybe it would be
cleaner just to use that file.
I did this the same way for the other 2 platforms, so this discussion would
apply there also.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-android pull request: Improving Android build output and v...
Github user jasongin commented on the pull request: https://github.com/apache/cordova-android/pull/305#issuecomment-219825870 LGTM --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-windows pull request: Improving Windows build output and v...
Github user jasongin commented on the pull request: https://github.com/apache/cordova-windows/pull/176#issuecomment-219552867 LGTM --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-windows pull request: Improving Windows build output and v...
Github user jasongin commented on a diff in the pull request:
https://github.com/apache/cordova-windows/pull/176#discussion_r63426276
--- Diff: bin/lib/create.js ---
@@ -46,15 +47,15 @@ module.exports.create = function (destinationDir,
config, options) {
var root = path.join(__dirname, '..', '..');
events.emit('log', 'Creating Cordova Windows Project:');
-events.emit('log', '\tApp Name : ' + appName);
-events.emit('log', '\tNamespace : ' + packageName);
-events.emit('log', '\tPath : ' + projectPath);
+events.emit('log', '\tPath: ' + path.relative(process.cwd(),
(projectPath || 'CordovaExample')));
--- End diff --
'CordovaExample' will never be substituted here because projectPath must be
truthy after the checks at the top of this function.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-ios pull request: Improving iOS build output and verbose l...
Github user jasongin commented on the pull request: https://github.com/apache/cordova-ios/pull/223#issuecomment-219494395 LGTM --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-lib pull request: Improving prepare and build output
Github user jasongin commented on the pull request: https://github.com/apache/cordova-lib/pull/439#issuecomment-218885704 :shipit: --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-windows pull request: Improving Windows build output and v...
Github user jasongin commented on the pull request: https://github.com/apache/cordova-windows/pull/176#issuecomment-218882039 :shipit: --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-windows pull request: Improving Windows build output and v...
Github user jasongin commented on a diff in the pull request:
https://github.com/apache/cordova-windows/pull/176#discussion_r63094515
--- Diff: template/cordova/lib/build.js ---
@@ -171,11 +171,11 @@ function parseAndValidateArgs(options) {
// Validate args
if (options.debug && options.release) {
-throw new CordovaError('Only one of "debug"/"release" options
should be specified');
+throw new CordovaError('Cannot specify "debug" and "release"
options together. Please only use one');
--- End diff --
Nit: Error messages generally shouldn't say "please". I think you can omit
this sentence anyway.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-android pull request: Improving Android build output and v...
Github user jasongin commented on the pull request: https://github.com/apache/cordova-android/pull/305#issuecomment-218880614 :shipit: --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-lib pull request: Improving prepare and build output
Github user jasongin commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/439#discussion_r63092520
--- Diff: cordova-lib/src/cordova/plugin.js ---
@@ -286,7 +286,7 @@ module.exports = function plugin(command, targets,
opts) {
var configXml = new
ConfigParser(configPath);
configXml.removePlugin(target);
configXml.write();
-events.emit('results', 'config.xml
entry for ' +target+ ' is removed');
+events.emit('results', 'config.xml
entry for plugin ' + target + ' was removed');
--- End diff --
What is the 'results' channel??
There is a similar log line for removing a platform from config.xml, only
it comes before the change.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-windows pull request: CB-11117: Optimize prepare for windo...
Github user jasongin commented on the pull request: https://github.com/apache/cordova-windows/pull/174#issuecomment-218324502 Oh, yeah I'll update those tests. --- In reply to: [218305527](https://github.com/apache/cordova-windows/pull/174#issuecomment-218305527) [](ancestors = 218305527) --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-ios pull request: CB-11117: Optimize prepare for ios platf...
Github user jasongin commented on the pull request: https://github.com/apache/cordova-ios/pull/220#issuecomment-218005495 @shazron I know - a cordova-common 1.3 release is in progress. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-lib pull request: CB-11117: Add FileUpdater module to cord...
Github user jasongin commented on the pull request: https://github.com/apache/cordova-lib/pull/429#issuecomment-217281613 As discussed, I tested this with symbolic links both at the root and in a subdirectory. It works fine without any special symlink handling in the code. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-lib pull request: CB-11194 Improve cordova load time
Github user jasongin commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/434#discussion_r62252220
--- Diff: cordova-common/src/superspawn.js ---
@@ -22,20 +22,20 @@ var fs = require('fs');
var path = require('path');
var _ = require('underscore');
var Q = require('q');
-var shell = require('shelljs');
var events = require('./events');
var iswin32 = process.platform == 'win32';
// On Windows, spawn() for batch files requires absolute path & having the
extension.
function resolveWindowsExe(cmd) {
+var which = require('shelljs/src/which');
var winExtensions = ['.exe', '.bat', '.cmd', '.js', '.vbs'];
function isValidExe(c) {
return winExtensions.indexOf(path.extname(c)) !== -1 &&
fs.existsSync(c);
}
if (isValidExe(cmd)) {
return cmd;
}
-cmd = shell.which(cmd) || cmd;
+cmd = which({}, cmd) || cmd;
if (!isValidExe(cmd)) {
--- End diff --
I think this won't work now. The non-wrapped `which()` returns a
`ShellString` object, then `path.extname()` throws if `(typeof path !==
'string')`.
Did you test on Windows?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-lib pull request: CB-11117: Add FileUpdater module to cord...
Github user jasongin commented on the pull request: https://github.com/apache/cordova-lib/pull/429#issuecomment-217236642 @nikhilkh waiting on you. Thanks. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-docs pull request: CB-11196: Converting mark elements in d...
Github user jasongin commented on the pull request: https://github.com/apache/cordova-docs/pull/593#issuecomment-217040413 ð --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-lib pull request: CB-11194 Improve cordova load time
Github user jasongin commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/434#discussion_r62068625
--- Diff: cordova-lib/cordova-lib.js ---
@@ -18,19 +18,35 @@
*/
// For now expose plugman and cordova just as they were in the old repos
+
+
+function addProperty(obj, property, modulePath) {
--- End diff --
Nice, that should definitely be consolidated. It will have to be moved into
cordova-common though.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-android pull request: CB-11198 Skip android target sdk che...
Github user jasongin commented on the pull request: https://github.com/apache/cordova-android/pull/303#issuecomment-216705549 :+1: --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-android pull request: CB-11198 Skip android target sdk che...
Github user jasongin commented on a diff in the pull request:
https://github.com/apache/cordova-android/pull/303#discussion_r61976209
--- Diff: bin/templates/cordova/lib/builders/GradleBuilder.js ---
@@ -203,6 +203,11 @@ GradleBuilder.prototype.build = function(opts) {
} else {
process.stdout.write(stdio.stdout);
}
+}).catch(function (error) {
+if (error.toString().indexOf('failed to find target with hash
string') >= 0) {
+return check_reqs.check_android_target(error);
--- End diff --
Would it be possible for check_android_target to actually find the target
and then swallow this error?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-android pull request: CB-11198 Skip android target sdk che...
Github user jasongin commented on a diff in the pull request:
https://github.com/apache/cordova-android/pull/303#discussion_r61975714
--- Diff: bin/lib/check_reqs.js ---
@@ -238,13 +236,13 @@ module.exports.getAbsoluteAndroidCmd = function () {
return cmd.replace(/(\s)/g, '\\$1');
};
-module.exports.check_android_target = function(valid_target) {
+module.exports.check_android_target = function(originalError) {
--- End diff --
I don't see where any value for originalError is ever passed in by a caller.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-android pull request: CB-11198 Skip android target sdk che...
Github user jasongin commented on a diff in the pull request:
https://github.com/apache/cordova-android/pull/303#discussion_r61975678
--- Diff: bin/lib/check_reqs.js ---
@@ -254,18 +252,22 @@ module.exports.check_android_target =
function(valid_target) {
}
var androidCmd = module.exports.getAbsoluteAndroidCmd();
-throw new CordovaError('Please install Android target: "' +
valid_target + '".\n\n' +
+var msg = 'Please install Android target: "' + valid_target +
'".\n\n' +
'Hint: Open the SDK manager by running: ' + androidCmd + '\n' +
'You will require:\n' +
'1. "SDK Platform" for ' + valid_target + '\n' +
'2. "Android SDK Platform-tools (latest)\n' +
-'3. "Android SDK Build-tools" (latest)');
+'3. "Android SDK Build-tools" (latest)'
--- End diff --
Missing semicolon.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-windows pull request: CB-11117: Optimize prepare for windo...
GitHub user jasongin opened a pull request: https://github.com/apache/cordova-windows/pull/174 CB-7: Optimize prepare for windows platform, clean prepared files This uses the `FileUpdater` module added in apache/cordova-lib#429 to skip copying files that didn't change. Some refactoring was required because previously the target directories would just be wiped before copying; now we need to map out the source and target directories so the `FileUpdater` has the necessary information to determine the optimal set of file operations. And I'm making it so that `cordova clean windows` also cleans the prepared files (but `clean.bat` does not). You can merge this pull request into a Git repository by running: $ git pull https://github.com/jasongin/cordova-windows CB-7 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/cordova-windows/pull/174.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #174 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-ios pull request: CB-11117: Optimize prepare for ios platf...
GitHub user jasongin opened a pull request: https://github.com/apache/cordova-ios/pull/220 CB-7: Optimize prepare for ios platform, clean prepared files This uses the `FileUpdater` module added in apache/cordova-lib#429 to skip copying files that didn't change. Some refactoring was required because previously the target directories would just be wiped before copying; now we need to map out the source and target directories so the `FileUpdater` has the necessary information to determine the optimal set of file operations. And I'm making it so that `cordova clean ios` also cleans the prepared files (but the `clean` shell script does not). You can merge this pull request into a Git repository by running: $ git pull https://github.com/jasongin/cordova-ios CB-7 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/cordova-ios/pull/220.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #220 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-docs pull request: CB-11196: Converting mark elements in d...
Github user jasongin commented on the pull request: https://github.com/apache/cordova-docs/pull/593#issuecomment-21788 Do the icons have title text? If so I can't find it in the code. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-docs pull request: CB-11196: Converting mark elements in d...
Github user jasongin commented on the pull request: https://github.com/apache/cordova-docs/pull/593#issuecomment-216665227 > The Apple logo is now used to indicate the OS X platform. This is confusing to me, given that I never think about using Cordova to develop apps for OS X, and my iPhone has a prominent Apple logo on the back. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-lib pull request: CB-11117: Add FileUpdater module to cord...
Github user jasongin commented on the pull request: https://github.com/apache/cordova-lib/pull/429#issuecomment-216624907 If you're talking about a source file, then that would change the last-modified timestamp of the source file, causing it to be newer than the target file so it will be copied on the next prepare. If you save changes to a source file at precisely the wrong time _while a build is in progress_ (and the change doesn't affect the size of the file), then it's possible the change could go undetected if a hook modifies the corresponding target file to make it newer than the changed source file. But even good old 'make' has this issue. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-lib pull request: CB-11117: Add FileUpdater module to cord...
Github user jasongin commented on the pull request: https://github.com/apache/cordova-lib/pull/429#issuecomment-216618237 BTW, I didn't mention above the reason I had to abandon the approach of checking for exactly-equal modified times. The problem is the OS X filesystem doesn't support sub-second timestamps, so it would be possible for a file to be modified again in the same second while still appearing to have the same timestamp. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-lib pull request: CB-11117: Add FileUpdater module to cord...
Github user jasongin commented on the pull request: https://github.com/apache/cordova-lib/pull/429#issuecomment-216617718 @purplecabbage If a hook fails because it expected a file would be refreshed but it wasn't, then I'd expect the hook to print an error (or stack trace), so that should be easy to find. Is there another scenario you're concerned about? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-lib pull request: CB-11117: Add FileUpdater module to cord...
Github user jasongin commented on the pull request: https://github.com/apache/cordova-lib/pull/429#issuecomment-216611405 All right, I think this is the best we can do. The latest logic copies the file if the source's last-modified time is >= the target's last-modified time, or if the file sizes are different. At the time the file gets copied, the target's last-modified time gets set to the current time, so it is greater than the source's time until the source gets modified again. This should work fine with hooks, because a hook that modifies a file is likely to change the size, or if it doesn't change the size then the hook is likely to handle the file not being refreshed. One could contrive an artificial scenario in which the right thing doesn't happen, but in practice I don't think there would be any issues. And still there will be the 'clean' functionality to fall back on. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-lib pull request: CB-11194 Improve cordova load time
Github user jasongin commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/434#discussion_r61915826
--- Diff: cordova-common/src/superspawn.js ---
@@ -22,7 +22,7 @@ var fs = require('fs');
var path = require('path');
var _ = require('underscore');
var Q = require('q');
-var shell = require('shelljs');
+var which = require('shelljs/src/which');
--- End diff --
Also, does this really save any time? Lots of sources require shelljs, so
I'd expect it to be fully loaded somewhere else anyway.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-lib pull request: CB-11194 Improve cordova load time
Github user jasongin commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/434#discussion_r61915398
--- Diff: cordova-lib/src/platforms/PlatformApiPoly.js ---
@@ -474,7 +473,12 @@ function getCreateArgs(destinationDir, projectConfig,
options) {
// CB-6992 it is necessary to normalize characters
// because node and shell scripts handles unicode symbols differently
// We need to normalize the name to NFD form since iOS uses NFD
unicode form
-args.push(platformName == 'ios' ? unorm.nfd(projectConfig.name()) :
projectConfig.name());
+var name = projectConfig.name();
+if (platformName == 'ios') {
+var unorm = require('unorm');
+name = unorm.nfd(projectConfig.name());
--- End diff --
`name = unorm.nfd(name);`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-lib pull request: CB-11194 Improve cordova load time
Github user jasongin commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/434#discussion_r61914375
--- Diff: cordova-common/src/ConfigChanges/ConfigFile.js ---
@@ -17,13 +17,30 @@
var fs = require('fs');
var path = require('path');
-var bplist = require('bplist-parser');
-var et = require('elementtree');
-var glob = require('glob');
-var plist = require('plist');
+var modules = {};
+
+function addProperty(obj, property, modulePath) {
--- End diff --
Perhaps add an optional 4th parameter which is the name of a property on
the required module.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-lib pull request: CB-11194 Improve cordova load time
Github user jasongin commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/434#discussion_r61913509
--- Diff: cordova-common/src/superspawn.js ---
@@ -22,7 +22,7 @@ var fs = require('fs');
var path = require('path');
var _ = require('underscore');
var Q = require('q');
-var shell = require('shelljs');
+var which = require('shelljs/src/which');
--- End diff --
It looks like this relies on shelljs internal layout that may change in a
future version.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-lib pull request: CB-11194 Improve cordova load time
Github user jasongin commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/434#discussion_r61913318
--- Diff: cordova-common/src/ConfigChanges/ConfigFile.js ---
@@ -17,13 +17,30 @@
var fs = require('fs');
var path = require('path');
-var bplist = require('bplist-parser');
-var et = require('elementtree');
-var glob = require('glob');
-var plist = require('plist');
+var modules = {};
+
+function addProperty(obj, property, modulePath) {
--- End diff --
Can this be defined once and then required from other sources?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-lib pull request: CB-11194 Improve cordova load time
Github user jasongin commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/434#discussion_r61913141
--- Diff: cordova-common/cordova-common.js ---
@@ -17,26 +17,40 @@
under the License.
*/
-exports = module.exports = {
-events: require('./src/events'),
-superspawn: require('./src/superspawn'),
-
-ActionStack: require('./src/ActionStack'),
-CordovaError: require('./src/CordovaError/CordovaError'),
-CordovaLogger: require('./src/CordovaLogger'),
-CordovaExternalToolErrorContext:
require('./src/CordovaError/CordovaExternalToolErrorContext'),
-PlatformJson: require('./src/PlatformJson'),
-ConfigParser: require('./src/ConfigParser/ConfigParser.js'),
-
-PluginInfo: require('./src/PluginInfo/PluginInfo.js'),
-PluginInfoProvider: require('./src/PluginInfo/PluginInfoProvider.js'),
-
-PluginManager: require('./src/PluginManager'),
-
-ConfigChanges: require('./src/ConfigChanges/ConfigChanges.js'),
-ConfigKeeper: require('./src/ConfigChanges/ConfigKeeper.js'),
-ConfigFile: require('./src/ConfigChanges/ConfigFile.js'),
-mungeUtil: require('./src/ConfigChanges/munge-util.js'),
-
-xmlHelpers: require('./src/util/xml-helpers')
-};
+function addProperty(obj, property, modulePath) {
--- End diff --
Name this something more specific than addProperty, because it only adds a
specific kind of required-module property.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-lib pull request: CB-11194 Improve cordova load time
Github user jasongin commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/434#discussion_r61912923
--- Diff: cordova-common/cordova-common.js ---
@@ -17,26 +17,40 @@
under the License.
*/
-exports = module.exports = {
-events: require('./src/events'),
-superspawn: require('./src/superspawn'),
-
-ActionStack: require('./src/ActionStack'),
-CordovaError: require('./src/CordovaError/CordovaError'),
-CordovaLogger: require('./src/CordovaLogger'),
-CordovaExternalToolErrorContext:
require('./src/CordovaError/CordovaExternalToolErrorContext'),
-PlatformJson: require('./src/PlatformJson'),
-ConfigParser: require('./src/ConfigParser/ConfigParser.js'),
-
-PluginInfo: require('./src/PluginInfo/PluginInfo.js'),
-PluginInfoProvider: require('./src/PluginInfo/PluginInfoProvider.js'),
-
-PluginManager: require('./src/PluginManager'),
-
-ConfigChanges: require('./src/ConfigChanges/ConfigChanges.js'),
-ConfigKeeper: require('./src/ConfigChanges/ConfigKeeper.js'),
-ConfigFile: require('./src/ConfigChanges/ConfigFile.js'),
-mungeUtil: require('./src/ConfigChanges/munge-util.js'),
-
-xmlHelpers: require('./src/util/xml-helpers')
-};
+function addProperty(obj, property, modulePath) {
+var val = null;
+
+// Add properties as getter to delay load the modules on first
invocation
+Object.defineProperty(obj, property, {
+configurable: true,
+get: function () {
+val = val || require(modulePath);
+ob[property] = val;
--- End diff --
Also, there should be no need to use the 'val' cache if you are overwriting
obj[property].
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-lib pull request: CB-11194 Improve cordova load time
Github user jasongin commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/434#discussion_r61912394
--- Diff: cordova-common/cordova-common.js ---
@@ -17,26 +17,40 @@
under the License.
*/
-exports = module.exports = {
-events: require('./src/events'),
-superspawn: require('./src/superspawn'),
-
-ActionStack: require('./src/ActionStack'),
-CordovaError: require('./src/CordovaError/CordovaError'),
-CordovaLogger: require('./src/CordovaLogger'),
-CordovaExternalToolErrorContext:
require('./src/CordovaError/CordovaExternalToolErrorContext'),
-PlatformJson: require('./src/PlatformJson'),
-ConfigParser: require('./src/ConfigParser/ConfigParser.js'),
-
-PluginInfo: require('./src/PluginInfo/PluginInfo.js'),
-PluginInfoProvider: require('./src/PluginInfo/PluginInfoProvider.js'),
-
-PluginManager: require('./src/PluginManager'),
-
-ConfigChanges: require('./src/ConfigChanges/ConfigChanges.js'),
-ConfigKeeper: require('./src/ConfigChanges/ConfigKeeper.js'),
-ConfigFile: require('./src/ConfigChanges/ConfigFile.js'),
-mungeUtil: require('./src/ConfigChanges/munge-util.js'),
-
-xmlHelpers: require('./src/util/xml-helpers')
-};
+function addProperty(obj, property, modulePath) {
+var val = null;
+
+// Add properties as getter to delay load the modules on first
invocation
+Object.defineProperty(obj, property, {
+configurable: true,
+get: function () {
+val = val || require(modulePath);
+ob[property] = val;
--- End diff --
Typo, should be 'obj', not 'ob' ?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-lib pull request: CB-11194 Improve cordova load time
Github user jasongin commented on a diff in the pull request: https://github.com/apache/cordova-lib/pull/434#discussion_r61912274 --- Diff: cordova-common/cordova-common.js --- @@ -3,7 +3,7 @@ or more contributor license agreements. See the NOTICE file distributed with this work for additional information regarding copyright ownership. The ASF licenses this file -to you under the Apache License, Version 2.0 (the +to you under the Apache License } Version 2.0 (the --- End diff -- fix this --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-lib pull request: CB-11117: Add FileUpdater module to cord...
Github user jasongin commented on the pull request: https://github.com/apache/cordova-lib/pull/429#issuecomment-216585668 Well, after further testing I found creation dates don't work reliably due to some strange behavior on Windows: http://serverfault.com/questions/92757/incorrect-file-creation-date-in-windows-xp-vista So I'm still working on finding the best solution here. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-android pull request: CB-11117: Use FileUpdater to optimiz...
Github user jasongin commented on the pull request: https://github.com/apache/cordova-android/pull/295#issuecomment-216320625 I updated the PR based on feedback. As mentioned in the other comment I've kept the clean functionality, but it is now skipped when not invoked via the CLI. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-lib pull request: CB-11117: Add FileUpdater module to cord...
Github user jasongin commented on the pull request: https://github.com/apache/cordova-lib/pull/429#issuecomment-216319068 I updated the PR to use target creation times instead of modified-times. It works much better: it doesn't require the messy workaround on Windows and doesn't have any problems with imprecise timestamps (as on OS X). --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-android pull request: CB-11117: Use FileUpdater to optimiz...
Github user jasongin commented on a diff in the pull request:
https://github.com/apache/cordova-android/pull/295#discussion_r61776793
--- Diff: bin/templates/cordova/lib/prepare.js ---
@@ -26,26 +26,43 @@ var AndroidManifest = require('./AndroidManifest');
var xmlHelpers = require('cordova-common').xmlHelpers;
var CordovaError = require('cordova-common').CordovaError;
var ConfigParser = require('cordova-common').ConfigParser;
+var FileUpdater = require('cordova-common').FileUpdater;
-module.exports.prepare = function (cordovaProject) {
+module.exports.prepare = function (cordovaProject, options) {
var self = this;
+var platformResourcesDir = path.relative(cordovaProject.root,
path.join(this.locations.root, 'res'));
this._config = updateConfigFilesFrom(cordovaProject.projectConfig,
this._munger, this.locations);
// Update own www dir with project's www assets and plugins' assets
and js-files
-return Q.when(updateWwwFrom(cordovaProject, this.locations))
+return Q.when(updateWww.call(self, cordovaProject))
.then(function () {
// update project according to config.xml changes.
return updateProjectAccordingTo(self._config, self.locations);
})
.then(function () {
-handleIcons(cordovaProject.projectConfig, self.root);
-handleSplashes(cordovaProject.projectConfig, self.root);
+updateIcons.call(self, cordovaProject, platformResourcesDir);
+updateSplashes.call(self, cordovaProject, platformResourcesDir);
})
.then(function () {
-events.emit('verbose', 'updated project successfully');
+events.emit('verbose', 'Prepared project successfully');
+});
+};
+
+module.exports.clean = function (options) {
+// Unfortunately the cordovaProject isn't passed into the clean()
function,
+// but the project root dir and config can be resolved here easily.
--- End diff --
After discussion with Nikhil, we'd really like to keep the clean
functionality. I'm adding a check for invocation via the non-CLI clean script;
I hope that addresses your concern.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-windows pull request: CB-10653 Making the windows activati...
Github user jasongin commented on the pull request: https://github.com/apache/cordova-windows/pull/173#issuecomment-216309577 ð --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-windows pull request: CB-10653 Making the windows activati...
Github user jasongin commented on a diff in the pull request:
https://github.com/apache/cordova-windows/pull/173#discussion_r61665577
--- Diff: template/www/cordova.js ---
@@ -1508,9 +1509,10 @@ module.exports = {
// OR cordova.require('cordova/platform').activationContext
// activationContext:{type: actType, args: args};
var activationHandler = function (e) {
-var args = e.detail.arguments;
-var actType = e.detail.type;
-platform.activationContext = { type: actType, args: args };
+// Making all the details available as activationContext
+platform.activationContext = utils.clone(e.detail);
+platform.activationContext.args = e.detail.arguments;
/* for backwards compatibility */
+platform.activationContext.actType = e.detail.type;
/* for backwards compatibility */
--- End diff --
Same here with actType.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-windows pull request: CB-10653 Making the windows activati...
Github user jasongin commented on a diff in the pull request:
https://github.com/apache/cordova-windows/pull/173#discussion_r61665566
--- Diff: cordova-js-src/platform.js ---
@@ -56,9 +57,10 @@ module.exports = {
// OR cordova.require('cordova/platform').activationContext
// activationContext:{type: actType, args: args};
var activationHandler = function (e) {
-var args = e.detail.arguments;
-var actType = e.detail.type;
-platform.activationContext = { type: actType, args: args };
+// Making all the details available as activationContext
+platform.activationContext = utils.clone(e.detail);
+platform.activationContext.args = e.detail.arguments;
/* for backwards compatibility */
+platform.activationContext.actType = e.detail.type;
/* for backwards compatibility */
--- End diff --
I think you don't need an actType property for back-compat. The property
(not the variable) was previously named just "type".
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-lib pull request: CB-11117: Add FileUpdater module to cord...
Github user jasongin commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/429#discussion_r61665459
--- Diff: cordova-common/src/FileUpdater.js ---
@@ -0,0 +1,499 @@
+/**
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements. See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership. The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License. You may obtain a copy of the License at
+
+http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied. See the License for the
+specific language governing permissions and limitations
+under the License.
+*/
+
+"use strict";
+
+var fs = require("fs");
+var path = require("path");
+var shell = require("shelljs");
+var minimatch = require("minimatch");
+
+var isWindows = (process.platform === "win32");
+var child_process = (isWindows ? require("child_process") : null);
+
+/**
+ * Logging callback used in the FileUpdater methods.
+ * @callback loggingCallback
+ * @param {string} message A message describing a single file update
operation.
+ */
+
+/**
+ * Updates a target file or directory with a source file or directory.
(Directory updates are
+ * not recursive.) Stats for target and source items must be passed in.
This is an internal
+ * helper function used by other methods in this module.
+ *
+ * @param {?string} sourcePath Source file or directory to be used to
update the
+ * destination. If the source is null, then the destination is deleted
if it exists.
+ * @param {?fs.Stats} sourceStats An instance of fs.Stats for the source
path, or null if
+ * the source does not exist.
+ * @param {string} targetPath Required destination file or directory to be
updated. If it does
+ * not exist, it will be created.
+ * @param {?fs.Stats} targetStats An instance of fs.Stats for the target
path, or null if
+ * the target does not exist.
+ * @param {Object} [options] Optional additional parameters for the update.
+ * @param {string} [options.rootDir] Optional root directory (such as a
project) to which target
+ * and source path parameters are relative; may be omitted if the
paths are absolute. The
+ * rootDir is always omitted from any logged paths, to make the logs
easier to read.
+ * @param {boolean} [options.all] If true, all files are copied regardless
of last-modified times.
+ * @param {boolean} [options.newer] If true (and all is not specified),
only files with newer
+ * last-modified times are copied. By default, only files with
different times are copied.
+ * @param {loggingCallback} [log] Optional logging callback that takes a
string message
+ * describing any file operations that are performed.
+ * @param {Object} context Context object for tracking file operations.
+ * @return {boolean} true if any changes were made, or false if the force
flag is not set
+ * and everything was up to date
+ */
+function updatePathWithStats(
+sourcePath, sourceStats, targetPath, targetStats, options, log,
context) {
+var updated = false;
+
+var rootDir = (options && options.rootDir) || "";
+var copyAll = (options && options.all) || false;
+var copyNewer = (options && options.newer) || false;
+
+var targetFullPath = path.join(rootDir || "", targetPath);
+
+if (sourceStats) {
+var sourceFullPath = path.join(rootDir || "", sourcePath);
+
+if (targetStats) {
+// The target exists. But if the directory status doesn't
match the source, delete it.
+if (targetStats.isDirectory() && !sourceStats.isDirectory()) {
+log("rmdir " + targetPath + " (source is a file)");
--- End diff --
Yes. Basically I wanted to enable callers to choose whether they want any
logging and what level and format. I expect these functions could be useful in
more than one scenario, so logging requirements may be different.
Initially I am passing in events.emit.bind('verbose') as the logging
callback to these functions in the updates I'm making to each platform's
prepare code. But I know we may
[GitHub] cordova-lib pull request: CB-11117: Add FileUpdater module to cord...
Github user jasongin commented on the pull request: https://github.com/apache/cordova-lib/pull/429#issuecomment-215911697 @nikhilkh As we discussed, this workaround was the best I could find. The command script approach has similar performance to just copying files via the node fs APIs, and it correctly copies the file times, enabling incremental builds to work correctly. My biggest remaining concern is any errors during the copy will not pinpoint a specific file, but that should be an extremely rare issue. I also tried using a generated PowerShell script to just update the file times after copying in nodejs, but that was too slow, both in PS startup cost and per-file cost. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-lib pull request: CB-11174 Resolve symlinked path before g...
Github user jasongin commented on the pull request: https://github.com/apache/cordova-lib/pull/433#issuecomment-215898416 :+1: --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-lib pull request: CB-11174 Resolve symlinked path before g...
Github user jasongin commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/433#discussion_r61653369
--- Diff: cordova-lib/spec-cordova/platforms/platforms.spec.js ---
@@ -17,19 +17,25 @@
under the License.
*/
+var fs = require('fs');
+var os = require('os');
var path = require('path');
var rewire = require('rewire');
+var shell = require('shelljs');
var util = require('../../src/cordova/util');
var platforms = rewire('../../src/platforms/platforms');
var CORDOVA_ROOT = path.join(__dirname,
'../fixtures/projects/platformApi');
var PLATFORM_WITH_API = path.join(CORDOVA_ROOT, 'platforms/windows');
var PLATFORM_WOUT_API = path.join(CORDOVA_ROOT, 'platforms/android');
+var PLATFORM_SYMLINK = path.join(os.tmpdir(), 'cordova_windows_symlink');
-var MockPlatformApi = require(path.join(PLATFORM_WITH_API, 'cordova',
'Api'));
+var MockPlatformApi = require(fs.realpathSync(path.join(PLATFORM_WITH_API,
'cordova/Api.js')));
var PlatformApiPoly = require('../../src/platforms/PlatformApiPoly');
+shell.ln('-sf', PLATFORM_WITH_API, PLATFORM_SYMLINK);
+
--- End diff --
Have you tested on Windows? (I see the bug is just about OS X.)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-windows pull request: CB-11139 Use PluginManager from comm...
Github user jasongin commented on the pull request: https://github.com/apache/cordova-windows/pull/171#issuecomment-215891369 :+1: --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-ios pull request: CB-11161 Reuse PluginManager from common...
Github user jasongin commented on the pull request: https://github.com/apache/cordova-ios/pull/219#issuecomment-215890484 :+1: --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-lib pull request: CB-11117: Add FileUpdater module to cord...
Github user jasongin commented on the pull request: https://github.com/apache/cordova-lib/pull/429#issuecomment-215780851 I need to add a workaround for the nodejs bug in setting file timestamps on Windows: https://github.com/nodejs/node/issues/2069 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-android pull request: CB-11117: Use FileUpdater to optimiz...
Github user jasongin commented on a diff in the pull request:
https://github.com/apache/cordova-android/pull/295#discussion_r61504885
--- Diff: bin/templates/cordova/lib/prepare.js ---
@@ -84,29 +101,49 @@ function updateConfigFilesFrom(sourceConfig,
configMunger, locations) {
}
/**
+ * Logs all file operations via the verbose event stream, indented.
+ */
+function logFileOp(message) {
+events.emit('verbose', ' ' + message);
+}
+
+/**
* Updates platform 'www' directory by replacing it with contents of
* 'platform_www' and app www. Also copies project's overrides' folder
into
* the platform 'www' folder
*
* @param {Object} cordovaProjectAn object which describes cordova
project.
- * @param {Object} destinations An object that contains destination
- * paths for www files.
*/
-function updateWwwFrom(cordovaProject, destinations) {
-shell.rm('-rf', destinations.www);
-shell.mkdir('-p', destinations.www);
-// Copy source files from project's www directory
-shell.cp('-rf', path.join(cordovaProject.locations.www, '*'),
destinations.www);
-// Override www sources by files in 'platform_www' directory
-shell.cp('-rf', path.join(destinations.platformWww, '*'),
destinations.www);
+function updateWww(cordovaProject) {
+var sourceDirs = [
+path.relative(cordovaProject.root, cordovaProject.locations.www),
+path.relative(cordovaProject.root, this.locations.platformWww)
+];
// If project contains 'merges' for our platform, use them as another
overrides
var merges_path = path.join(cordovaProject.root, 'merges', 'android');
if (fs.existsSync(merges_path)) {
events.emit('verbose', 'Found "merges" for android platform.
Copying over existing "www" files.');
-var overrides = path.join(merges_path, '*');
-shell.cp('-rf', overrides, destinations.www);
+sourceDirs.push(path.join('merges', 'android'));
}
+
+var targetDir = path.relative(cordovaProject.root, this.locations.www);
+events.emit(
+'verbose', "Merging and updating files from [" +
sourceDirs.join(", ") + "] to " + targetDir);
+FileUpdater.mergeAndUpdateDir(
+sourceDirs, targetDir, { rootDir: cordovaProject.root },
logFileOp);
+}
+
+/**
+ * Cleans all files from the platform 'www' directory.
+ */
+function cleanWww(projectRoot) {
+var targetDir = path.relative(projectRoot, this.locations.www);
+events.emit('verbose', "Cleaning " + targetDir);
+
+// No source paths are specified, so mergeAndUpdateDir() will clear
the target directory.
+FileUpdater.mergeAndUpdateDir(
+[], targetDir, { rootDir: projectRoot, all: true }, logFileOp);
--- End diff --
I was mostly going for consistent logging: why log some file operations but
not others? Our customer feedback indicates they would like more visibility
into the cordova build process, and this kind of verbose logging will help with
that.
Anyway this is going away now that I'm removing the clean function.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-android pull request: CB-11117: Use FileUpdater to optimiz...
Github user jasongin commented on a diff in the pull request:
https://github.com/apache/cordova-android/pull/295#discussion_r61503079
--- Diff: bin/templates/cordova/lib/prepare.js ---
@@ -26,26 +26,43 @@ var AndroidManifest = require('./AndroidManifest');
var xmlHelpers = require('cordova-common').xmlHelpers;
var CordovaError = require('cordova-common').CordovaError;
var ConfigParser = require('cordova-common').ConfigParser;
+var FileUpdater = require('cordova-common').FileUpdater;
-module.exports.prepare = function (cordovaProject) {
+module.exports.prepare = function (cordovaProject, options) {
var self = this;
+var platformResourcesDir = path.relative(cordovaProject.root,
path.join(this.locations.root, 'res'));
this._config = updateConfigFilesFrom(cordovaProject.projectConfig,
this._munger, this.locations);
// Update own www dir with project's www assets and plugins' assets
and js-files
-return Q.when(updateWwwFrom(cordovaProject, this.locations))
+return Q.when(updateWww.call(self, cordovaProject))
.then(function () {
// update project according to config.xml changes.
return updateProjectAccordingTo(self._config, self.locations);
})
.then(function () {
-handleIcons(cordovaProject.projectConfig, self.root);
-handleSplashes(cordovaProject.projectConfig, self.root);
+updateIcons.call(self, cordovaProject, platformResourcesDir);
+updateSplashes.call(self, cordovaProject, platformResourcesDir);
})
.then(function () {
-events.emit('verbose', 'updated project successfully');
+events.emit('verbose', 'Prepared project successfully');
+});
+};
+
+module.exports.clean = function (options) {
+// Unfortunately the cordovaProject isn't passed into the clean()
function,
+// but the project root dir and config can be resolved here easily.
--- End diff --
I don't think everyone using CLI workflow treats platforms as build
artifacts. Regardless, I think this kind of clean behavior is not needed now
that incremental prepare does a stronger sync (copying different file times
rather than just newer times). So I'll remove it for now. We can discuss the
merits of something like 'cordova prepare --clean' later.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-android pull request: CB-11138 Reuse PluginManager from co...
Github user jasongin commented on the pull request: https://github.com/apache/cordova-android/pull/301#issuecomment-215561101 :+1: --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-android pull request: CB-11138 Reuse PluginManager from co...
Github user jasongin commented on the pull request: https://github.com/apache/cordova-android/pull/301#issuecomment-215533601 Can I ignore all the changes under node_modules? I don't understand why that is checked in to source control... --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-android pull request: CB-11117: Use FileUpdater to optimiz...
Github user jasongin commented on a diff in the pull request:
https://github.com/apache/cordova-android/pull/295#discussion_r61462710
--- Diff: bin/templates/cordova/lib/prepare.js ---
@@ -26,26 +26,43 @@ var AndroidManifest = require('./AndroidManifest');
var xmlHelpers = require('cordova-common').xmlHelpers;
var CordovaError = require('cordova-common').CordovaError;
var ConfigParser = require('cordova-common').ConfigParser;
+var FileUpdater = require('cordova-common').FileUpdater;
-module.exports.prepare = function (cordovaProject) {
+module.exports.prepare = function (cordovaProject, options) {
var self = this;
+var platformResourcesDir = path.relative(cordovaProject.root,
path.join(this.locations.root, 'res'));
this._config = updateConfigFilesFrom(cordovaProject.projectConfig,
this._munger, this.locations);
// Update own www dir with project's www assets and plugins' assets
and js-files
-return Q.when(updateWwwFrom(cordovaProject, this.locations))
+return Q.when(updateWww.call(self, cordovaProject))
--- End diff --
I had been using `this` in a few more places, before I moved some things
around. I agree it is not really needed now; I'll clean that up everywhere.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-android pull request: CB-11117: Use FileUpdater to optimiz...
Github user jasongin commented on a diff in the pull request:
https://github.com/apache/cordova-android/pull/295#discussion_r61461619
--- Diff: bin/templates/cordova/lib/prepare.js ---
@@ -26,26 +26,43 @@ var AndroidManifest = require('./AndroidManifest');
var xmlHelpers = require('cordova-common').xmlHelpers;
var CordovaError = require('cordova-common').CordovaError;
var ConfigParser = require('cordova-common').ConfigParser;
+var FileUpdater = require('cordova-common').FileUpdater;
-module.exports.prepare = function (cordovaProject) {
+module.exports.prepare = function (cordovaProject, options) {
var self = this;
+var platformResourcesDir = path.relative(cordovaProject.root,
path.join(this.locations.root, 'res'));
this._config = updateConfigFilesFrom(cordovaProject.projectConfig,
this._munger, this.locations);
// Update own www dir with project's www assets and plugins' assets
and js-files
-return Q.when(updateWwwFrom(cordovaProject, this.locations))
+return Q.when(updateWww.call(self, cordovaProject))
.then(function () {
// update project according to config.xml changes.
return updateProjectAccordingTo(self._config, self.locations);
})
.then(function () {
-handleIcons(cordovaProject.projectConfig, self.root);
-handleSplashes(cordovaProject.projectConfig, self.root);
+updateIcons.call(self, cordovaProject, platformResourcesDir);
+updateSplashes.call(self, cordovaProject, platformResourcesDir);
})
.then(function () {
-events.emit('verbose', 'updated project successfully');
+events.emit('verbose', 'Prepared project successfully');
+});
+};
+
+module.exports.clean = function (options) {
+// Unfortunately the cordovaProject isn't passed into the clean()
function,
+// but the project root dir and config can be resolved here easily.
--- End diff --
I guess I don't understand the non-CLI workflow very well. Why would
'cordova clean' be called in a non-CLI workflow, rather than 'gradle clean' or
equivalent? Doesn't "non-CLI workflow" mean the cordova CLI commands like
'cordova clean' aren't being used?
In a normal cordova CLI workflow, I think users would expect that the files
copied by prepare would be cleaned up by 'cordova clean' without any options; I
was surprised to find they were not already.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-android pull request: CB-11117: Use FileUpdater to optimiz...
Github user jasongin commented on the pull request: https://github.com/apache/cordova-android/pull/295#issuecomment-215256784 @vladimir-kotikov @infil00p please review --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-lib pull request: CB-11117: Add FileUpdater module to cord...
Github user jasongin commented on the pull request: https://github.com/apache/cordova-lib/pull/429#issuecomment-215255548 @vladimir-kotikov I think this is ready to go after the latest update. Please let me know if you have any further comments. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-lib pull request: CB-11131 Fix TypeError: message.toUpperC...
Github user jasongin commented on the pull request: https://github.com/apache/cordova-lib/pull/431#issuecomment-214811002 ð --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-lib pull request: CB-11131 Fix TypeError: message.toUpperC...
Github user jasongin commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/431#discussion_r61116509
--- Diff: cordova-common/src/CordovaLogger.js ---
@@ -209,7 +209,7 @@ function formatError(error, isVerbose) {
message = error;
}
-if(message.toUpperCase().indexOf('ERROR:') !== 0) {
+if(message && typeof message.toUpperCase === 'function' &&
message.toUpperCase().indexOf('ERROR:') !== 0) {
--- End diff --
This is fine, but I would have preferred `if (typeof message === 'string'
&& message.toUpperCase()...)`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-plugin-file pull request: CB-11142: Fix the NeedPermission...
Github user jasongin commented on the pull request: https://github.com/apache/cordova-plugin-file/pull/179#issuecomment-214543607 LGTM --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-lib pull request: CB-11117: Add FileUpdater module to cord...
Github user jasongin commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/429#discussion_r60482215
--- Diff: cordova-common/src/FileUpdater.js ---
@@ -0,0 +1,389 @@
+/**
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements. See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership. The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License. You may obtain a copy of the License at
+
+http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied. See the License for the
+specific language governing permissions and limitations
+under the License.
+*/
+
+"use strict";
+
+var fs = require("fs");
+var path = require("path");
+var shell = require("shelljs");
+var minimatch = require("minimatch");
+
+/**
+ * Updates a target file or directory with a source file or directory.
(Directory updates are
+ * not recursive.) Stats for target and source items must be passed in.
This is an internal
+ * helper function used by other methods in this module.
+ *
+ * @param {string|null} rootDir Root directory (such as a project) to
which target and source
+ * path parameters are relative, or null if the paths are absolute.
The rootDir is omitted
+ * from any logged paths, to make the logs easier to read.
+ * @param {string} targetPath Destination file or directory to be updated.
If it does not exist,
+ * it will be created.
+ * @param {fs.Stats|null} targetStats An instance of fs.Stats for the
target path, or null if
+ * the target does not exist.
+ * @param {string|null} sourcePath Source file or directory to be used to
update the
+ * destination. If the source is null, then the destination is deleted
if it exists.
+ * @param {fs.Stats|null} sourceStats An instance of fs.Stats for the
source path, or null if
+ * the source does not exist.
+ * @param {boolean} force If target and source are both files, and the
force flag is not
+ * set, then the file will not be copied unless the source is newer
than the target.
+ * @param {function} [log] Optional logging callback that takes a string
message describing any
+ * file operations that are performed.
+ * @return {boolean} true if any changes were made, or false if the force
flag is not set
+ * and everything was up to date
+ */
+function updatePathWithStats(
+rootDir, targetPath, targetStats, sourcePath, sourceStats, force,
log) {
+log = log || function(message) { };
+var updated = false;
+
+var targetFullPath = path.join(rootDir || "", targetPath);
+
+if (sourceStats) {
+var sourceFullPath = path.join(rootDir || "", sourcePath);
+
+if (targetStats) {
+// The target exists. But if the directory status doesn't
match the source, delete it.
+if (targetStats.isDirectory() && !sourceStats.isDirectory()) {
+log("rmdir " + targetPath + " (source is a file)");
+shell.rm("-rf", targetFullPath);
+targetStats = null;
+updated = true;
+} else if (!targetStats.isDirectory() &&
sourceStats.isDirectory()) {
+log("delete " + targetPath + " (source is a directory)");
+shell.rm("-f", targetFullPath);
+targetStats = null;
+updated = true;
+}
+}
+
+if (!targetStats) {
+if (sourceStats.isDirectory()) {
+// The target directory does not exist, so it should be
created.
+log("mkdir " + targetPath);
+shell.mkdir("-p", targetFullPath);
+updated = true;
+} else if (sourceStats.isFile()) {
+// The target file does not exist, so it should be copied
from the source.
+log("copy " + sourcePath + " " + targetPath +
+(!force ? " (new file)" : ""));
+shell.cp("-f", sourceFullPath, targetFullPath);
+updated
[GitHub] cordova-lib pull request: CB-11117: Add FileUpdater module to cord...
Github user jasongin commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/429#discussion_r60481666
--- Diff: cordova-common/src/FileUpdater.js ---
@@ -0,0 +1,389 @@
+/**
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements. See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership. The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License. You may obtain a copy of the License at
+
+http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied. See the License for the
+specific language governing permissions and limitations
+under the License.
+*/
+
+"use strict";
+
+var fs = require("fs");
+var path = require("path");
+var shell = require("shelljs");
+var minimatch = require("minimatch");
+
+/**
+ * Updates a target file or directory with a source file or directory.
(Directory updates are
+ * not recursive.) Stats for target and source items must be passed in.
This is an internal
+ * helper function used by other methods in this module.
+ *
+ * @param {string|null} rootDir Root directory (such as a project) to
which target and source
--- End diff --
Because it was the first parameter and later parameters are required, it
could not be optional. But it will become optional (actually an optional part
of an optional options object) after I rearrange the parameters.
---
In reply to:
[60481370](https://github.com/apache/cordova-lib/pull/429#discussion_r60481370)
[](ancestors = 60481370)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-lib pull request: CB-11117: Add FileUpdater module to cord...
Github user jasongin commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/429#discussion_r60460847
--- Diff: cordova-common/src/FileUpdater.js ---
@@ -0,0 +1,389 @@
+/**
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements. See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership. The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License. You may obtain a copy of the License at
+
+http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied. See the License for the
+specific language governing permissions and limitations
+under the License.
+*/
+
+"use strict";
+
+var fs = require("fs");
+var path = require("path");
+var shell = require("shelljs");
+var minimatch = require("minimatch");
+
+/**
+ * Updates a target file or directory with a source file or directory.
(Directory updates are
+ * not recursive.) Stats for target and source items must be passed in.
This is an internal
+ * helper function used by other methods in this module.
+ *
+ * @param {string|null} rootDir Root directory (such as a project) to
which target and source
+ * path parameters are relative, or null if the paths are absolute.
The rootDir is omitted
+ * from any logged paths, to make the logs easier to read.
+ * @param {string} targetPath Destination file or directory to be updated.
If it does not exist,
+ * it will be created.
+ * @param {fs.Stats|null} targetStats An instance of fs.Stats for the
target path, or null if
+ * the target does not exist.
+ * @param {string|null} sourcePath Source file or directory to be used to
update the
+ * destination. If the source is null, then the destination is deleted
if it exists.
+ * @param {fs.Stats|null} sourceStats An instance of fs.Stats for the
source path, or null if
+ * the source does not exist.
+ * @param {boolean} force If target and source are both files, and the
force flag is not
+ * set, then the file will not be copied unless the source is newer
than the target.
+ * @param {function} [log] Optional logging callback that takes a string
message describing any
+ * file operations that are performed.
+ * @return {boolean} true if any changes were made, or false if the force
flag is not set
+ * and everything was up to date
+ */
+function updatePathWithStats(
+rootDir, targetPath, targetStats, sourcePath, sourceStats, force,
log) {
+log = log || function(message) { };
+var updated = false;
+
+var targetFullPath = path.join(rootDir || "", targetPath);
+
+if (sourceStats) {
+var sourceFullPath = path.join(rootDir || "", sourcePath);
+
+if (targetStats) {
+// The target exists. But if the directory status doesn't
match the source, delete it.
+if (targetStats.isDirectory() && !sourceStats.isDirectory()) {
+log("rmdir " + targetPath + " (source is a file)");
+shell.rm("-rf", targetFullPath);
+targetStats = null;
+updated = true;
+} else if (!targetStats.isDirectory() &&
sourceStats.isDirectory()) {
+log("delete " + targetPath + " (source is a directory)");
+shell.rm("-f", targetFullPath);
+targetStats = null;
+updated = true;
+}
+}
+
+if (!targetStats) {
+if (sourceStats.isDirectory()) {
+// The target directory does not exist, so it should be
created.
+log("mkdir " + targetPath);
+shell.mkdir("-p", targetFullPath);
+updated = true;
+} else if (sourceStats.isFile()) {
+// The target file does not exist, so it should be copied
from the source.
+log("copy " + sourcePath + " " + targetPath +
+(!force ? " (new file)" : ""));
+shell.cp("-f", sourceFullPath, targetFullPath);
+updated
[GitHub] cordova-lib pull request: CB-11117: Add FileUpdater module to cord...
Github user jasongin commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/429#discussion_r60460033
--- Diff: cordova-common/src/FileUpdater.js ---
@@ -0,0 +1,389 @@
+/**
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements. See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership. The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License. You may obtain a copy of the License at
+
+http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied. See the License for the
+specific language governing permissions and limitations
+under the License.
+*/
+
+"use strict";
+
+var fs = require("fs");
+var path = require("path");
+var shell = require("shelljs");
+var minimatch = require("minimatch");
+
+/**
+ * Updates a target file or directory with a source file or directory.
(Directory updates are
+ * not recursive.) Stats for target and source items must be passed in.
This is an internal
+ * helper function used by other methods in this module.
+ *
+ * @param {string|null} rootDir Root directory (such as a project) to
which target and source
+ * path parameters are relative, or null if the paths are absolute.
The rootDir is omitted
+ * from any logged paths, to make the logs easier to read.
+ * @param {string} targetPath Destination file or directory to be updated.
If it does not exist,
+ * it will be created.
+ * @param {fs.Stats|null} targetStats An instance of fs.Stats for the
target path, or null if
+ * the target does not exist.
+ * @param {string|null} sourcePath Source file or directory to be used to
update the
+ * destination. If the source is null, then the destination is deleted
if it exists.
+ * @param {fs.Stats|null} sourceStats An instance of fs.Stats for the
source path, or null if
+ * the source does not exist.
+ * @param {boolean} force If target and source are both files, and the
force flag is not
+ * set, then the file will not be copied unless the source is newer
than the target.
+ * @param {function} [log] Optional logging callback that takes a string
message describing any
+ * file operations that are performed.
+ * @return {boolean} true if any changes were made, or false if the force
flag is not set
+ * and everything was up to date
+ */
+function updatePathWithStats(
+rootDir, targetPath, targetStats, sourcePath, sourceStats, force,
log) {
+log = log || function(message) { };
+var updated = false;
+
+var targetFullPath = path.join(rootDir || "", targetPath);
+
+if (sourceStats) {
+var sourceFullPath = path.join(rootDir || "", sourcePath);
+
+if (targetStats) {
+// The target exists. But if the directory status doesn't
match the source, delete it.
+if (targetStats.isDirectory() && !sourceStats.isDirectory()) {
+log("rmdir " + targetPath + " (source is a file)");
+shell.rm("-rf", targetFullPath);
+targetStats = null;
+updated = true;
+} else if (!targetStats.isDirectory() &&
sourceStats.isDirectory()) {
+log("delete " + targetPath + " (source is a directory)");
+shell.rm("-f", targetFullPath);
+targetStats = null;
+updated = true;
+}
+}
+
+if (!targetStats) {
+if (sourceStats.isDirectory()) {
+// The target directory does not exist, so it should be
created.
+log("mkdir " + targetPath);
+shell.mkdir("-p", targetFullPath);
+updated = true;
+} else if (sourceStats.isFile()) {
+// The target file does not exist, so it should be copied
from the source.
+log("copy " + sourcePath + " " + targetPath +
+(!force ? " (new file)" : ""));
+shell.cp("-f", sourceFullPath, targetFullPath);
+updated
[GitHub] cordova-lib pull request: CB-11117: Add FileUpdater module to cord...
Github user jasongin commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/429#discussion_r60459967
--- Diff: cordova-common/src/FileUpdater.js ---
@@ -0,0 +1,389 @@
+/**
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements. See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership. The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License. You may obtain a copy of the License at
+
+http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied. See the License for the
+specific language governing permissions and limitations
+under the License.
+*/
+
+"use strict";
+
+var fs = require("fs");
+var path = require("path");
+var shell = require("shelljs");
+var minimatch = require("minimatch");
+
+/**
+ * Updates a target file or directory with a source file or directory.
(Directory updates are
+ * not recursive.) Stats for target and source items must be passed in.
This is an internal
+ * helper function used by other methods in this module.
+ *
+ * @param {string|null} rootDir Root directory (such as a project) to
which target and source
+ * path parameters are relative, or null if the paths are absolute.
The rootDir is omitted
+ * from any logged paths, to make the logs easier to read.
+ * @param {string} targetPath Destination file or directory to be updated.
If it does not exist,
+ * it will be created.
+ * @param {fs.Stats|null} targetStats An instance of fs.Stats for the
target path, or null if
+ * the target does not exist.
+ * @param {string|null} sourcePath Source file or directory to be used to
update the
+ * destination. If the source is null, then the destination is deleted
if it exists.
+ * @param {fs.Stats|null} sourceStats An instance of fs.Stats for the
source path, or null if
+ * the source does not exist.
+ * @param {boolean} force If target and source are both files, and the
force flag is not
+ * set, then the file will not be copied unless the source is newer
than the target.
+ * @param {function} [log] Optional logging callback that takes a string
message describing any
+ * file operations that are performed.
+ * @return {boolean} true if any changes were made, or false if the force
flag is not set
+ * and everything was up to date
+ */
+function updatePathWithStats(
+rootDir, targetPath, targetStats, sourcePath, sourceStats, force,
log) {
+log = log || function(message) { };
+var updated = false;
+
+var targetFullPath = path.join(rootDir || "", targetPath);
+
+if (sourceStats) {
+var sourceFullPath = path.join(rootDir || "", sourcePath);
+
+if (targetStats) {
+// The target exists. But if the directory status doesn't
match the source, delete it.
+if (targetStats.isDirectory() && !sourceStats.isDirectory()) {
+log("rmdir " + targetPath + " (source is a file)");
+shell.rm("-rf", targetFullPath);
+targetStats = null;
+updated = true;
+} else if (!targetStats.isDirectory() &&
sourceStats.isDirectory()) {
+log("delete " + targetPath + " (source is a directory)");
+shell.rm("-f", targetFullPath);
+targetStats = null;
+updated = true;
+}
+}
+
+if (!targetStats) {
+if (sourceStats.isDirectory()) {
+// The target directory does not exist, so it should be
created.
+log("mkdir " + targetPath);
+shell.mkdir("-p", targetFullPath);
+updated = true;
+} else if (sourceStats.isFile()) {
+// The target file does not exist, so it should be copied
from the source.
+log("copy " + sourcePath + " " + targetPath +
+(!force ? " (new file)" : ""));
+shell.cp("-f", sourceFullPath, targetFullPath);
+update
[GitHub] cordova-lib pull request: CB-11117: Add FileUpdater module to cord...
Github user jasongin commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/429#discussion_r60459281
--- Diff: cordova-common/src/FileUpdater.js ---
@@ -0,0 +1,389 @@
+/**
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements. See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership. The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License. You may obtain a copy of the License at
+
+http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied. See the License for the
+specific language governing permissions and limitations
+under the License.
+*/
+
+"use strict";
+
+var fs = require("fs");
+var path = require("path");
+var shell = require("shelljs");
+var minimatch = require("minimatch");
+
+/**
+ * Updates a target file or directory with a source file or directory.
(Directory updates are
+ * not recursive.) Stats for target and source items must be passed in.
This is an internal
+ * helper function used by other methods in this module.
+ *
+ * @param {string|null} rootDir Root directory (such as a project) to
which target and source
+ * path parameters are relative, or null if the paths are absolute.
The rootDir is omitted
+ * from any logged paths, to make the logs easier to read.
+ * @param {string} targetPath Destination file or directory to be updated.
If it does not exist,
+ * it will be created.
+ * @param {fs.Stats|null} targetStats An instance of fs.Stats for the
target path, or null if
+ * the target does not exist.
+ * @param {string|null} sourcePath Source file or directory to be used to
update the
+ * destination. If the source is null, then the destination is deleted
if it exists.
+ * @param {fs.Stats|null} sourceStats An instance of fs.Stats for the
source path, or null if
+ * the source does not exist.
+ * @param {boolean} force If target and source are both files, and the
force flag is not
+ * set, then the file will not be copied unless the source is newer
than the target.
+ * @param {function} [log] Optional logging callback that takes a string
message describing any
+ * file operations that are performed.
+ * @return {boolean} true if any changes were made, or false if the force
flag is not set
+ * and everything was up to date
+ */
+function updatePathWithStats(
+rootDir, targetPath, targetStats, sourcePath, sourceStats, force,
log) {
+log = log || function(message) { };
--- End diff --
Not a bad suggestion, but I'm leaning toward keeping it this way. I can
easily imagine multiple calls to FileUpdater methods using different logging.
(And I'll refactor the code in the other PR so the logging callback code isn't
duplicated.)
---
In reply to:
[60397531](https://github.com/apache/cordova-lib/pull/429#discussion_r60397531)
[](ancestors = 60397531)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-lib pull request: CB-11117: Add FileUpdater module to cord...
Github user jasongin commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/429#discussion_r60458884
--- Diff: cordova-common/src/FileUpdater.js ---
@@ -0,0 +1,389 @@
+/**
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements. See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership. The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License. You may obtain a copy of the License at
+
+http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied. See the License for the
+specific language governing permissions and limitations
+under the License.
+*/
+
+"use strict";
+
+var fs = require("fs");
+var path = require("path");
+var shell = require("shelljs");
+var minimatch = require("minimatch");
+
+/**
+ * Updates a target file or directory with a source file or directory.
(Directory updates are
+ * not recursive.) Stats for target and source items must be passed in.
This is an internal
+ * helper function used by other methods in this module.
+ *
+ * @param {string|null} rootDir Root directory (such as a project) to
which target and source
+ * path parameters are relative, or null if the paths are absolute.
The rootDir is omitted
+ * from any logged paths, to make the logs easier to read.
+ * @param {string} targetPath Destination file or directory to be updated.
If it does not exist,
+ * it will be created.
+ * @param {fs.Stats|null} targetStats An instance of fs.Stats for the
target path, or null if
+ * the target does not exist.
+ * @param {string|null} sourcePath Source file or directory to be used to
update the
+ * destination. If the source is null, then the destination is deleted
if it exists.
+ * @param {fs.Stats|null} sourceStats An instance of fs.Stats for the
source path, or null if
+ * the source does not exist.
+ * @param {boolean} force If target and source are both files, and the
force flag is not
+ * set, then the file will not be copied unless the source is newer
than the target.
+ * @param {function} [log] Optional logging callback that takes a string
message describing any
+ * file operations that are performed.
+ * @return {boolean} true if any changes were made, or false if the force
flag is not set
+ * and everything was up to date
+ */
+function updatePathWithStats(
+rootDir, targetPath, targetStats, sourcePath, sourceStats, force,
log) {
+log = log || function(message) { };
--- End diff --
Yes, there could easily be cases this is called when no logging is desired.
And logging to the console is a bad default for any Cordova code, because any
logging should go through the events pipeline.
---
In reply to:
[60397076](https://github.com/apache/cordova-lib/pull/429#discussion_r60397076)
[](ancestors = 60397076)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-lib pull request: CB-11117: Add FileUpdater module to cord...
Github user jasongin commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/429#discussion_r60452183
--- Diff: cordova-common/src/FileUpdater.js ---
@@ -0,0 +1,389 @@
+/**
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements. See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership. The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License. You may obtain a copy of the License at
+
+http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied. See the License for the
+specific language governing permissions and limitations
+under the License.
+*/
+
+"use strict";
+
+var fs = require("fs");
+var path = require("path");
+var shell = require("shelljs");
+var minimatch = require("minimatch");
+
+/**
+ * Updates a target file or directory with a source file or directory.
(Directory updates are
+ * not recursive.) Stats for target and source items must be passed in.
This is an internal
+ * helper function used by other methods in this module.
+ *
+ * @param {string|null} rootDir Root directory (such as a project) to
which target and source
--- End diff --
According to
[http://usejsdoc.org/tags-type.html,](http://usejsdoc.org/tags-type.html,) the
syntax for a nullable parameter is {?type}, which is distinct from an optional
parameter.
---
In reply to:
[60448617](https://github.com/apache/cordova-lib/pull/429#discussion_r60448617)
[](ancestors = 60448617,60384873)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-lib pull request: CB-11117: Add FileUpdater module to cord...
Github user jasongin commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/429#discussion_r60450430
--- Diff: cordova-common/src/FileUpdater.js ---
@@ -0,0 +1,389 @@
+/**
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements. See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership. The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License. You may obtain a copy of the License at
+
+http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied. See the License for the
+specific language governing permissions and limitations
+under the License.
+*/
+
+"use strict";
+
+var fs = require("fs");
+var path = require("path");
+var shell = require("shelljs");
+var minimatch = require("minimatch");
+
+/**
+ * Updates a target file or directory with a source file or directory.
(Directory updates are
+ * not recursive.) Stats for target and source items must be passed in.
This is an internal
+ * helper function used by other methods in this module.
+ *
+ * @param {string|null} rootDir Root directory (such as a project) to
which target and source
+ * path parameters are relative, or null if the paths are absolute.
The rootDir is omitted
+ * from any logged paths, to make the logs easier to read.
+ * @param {string} targetPath Destination file or directory to be updated.
If it does not exist,
+ * it will be created.
+ * @param {fs.Stats|null} targetStats An instance of fs.Stats for the
target path, or null if
+ * the target does not exist.
+ * @param {string|null} sourcePath Source file or directory to be used to
update the
+ * destination. If the source is null, then the destination is deleted
if it exists.
+ * @param {fs.Stats|null} sourceStats An instance of fs.Stats for the
source path, or null if
+ * the source does not exist.
+ * @param {boolean} force If target and source are both files, and the
force flag is not
+ * set, then the file will not be copied unless the source is newer
than the target.
+ * @param {function} [log] Optional logging callback that takes a string
message describing any
+ * file operations that are performed.
+ * @return {boolean} true if any changes were made, or false if the force
flag is not set
+ * and everything was up to date
+ */
+function updatePathWithStats(
+rootDir, targetPath, targetStats, sourcePath, sourceStats, force,
log) {
+log = log || function(message) { };
+var updated = false;
+
+var targetFullPath = path.join(rootDir || "", targetPath);
+
+if (sourceStats) {
+var sourceFullPath = path.join(rootDir || "", sourcePath);
+
+if (targetStats) {
+// The target exists. But if the directory status doesn't
match the source, delete it.
+if (targetStats.isDirectory() && !sourceStats.isDirectory()) {
+log("rmdir " + targetPath + " (source is a file)");
+shell.rm("-rf", targetFullPath);
+targetStats = null;
+updated = true;
+} else if (!targetStats.isDirectory() &&
sourceStats.isDirectory()) {
+log("delete " + targetPath + " (source is a directory)");
+shell.rm("-f", targetFullPath);
+targetStats = null;
+updated = true;
+}
+}
+
+if (!targetStats) {
+if (sourceStats.isDirectory()) {
+// The target directory does not exist, so it should be
created.
+log("mkdir " + targetPath);
+shell.mkdir("-p", targetFullPath);
+updated = true;
+} else if (sourceStats.isFile()) {
+// The target file does not exist, so it should be copied
from the source.
+log("copy " + sourcePath + " " + targetPath +
+(!force ? " (new file)" : ""));
+shell.cp("-f", sourceFullPath, targetFullPath);
+updated
[GitHub] cordova-lib pull request: CB-11117: Add FileUpdater module to cord...
Github user jasongin commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/429#discussion_r60449617
--- Diff: cordova-common/src/FileUpdater.js ---
@@ -0,0 +1,389 @@
+/**
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements. See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership. The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License. You may obtain a copy of the License at
+
+http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied. See the License for the
+specific language governing permissions and limitations
+under the License.
+*/
+
+"use strict";
+
+var fs = require("fs");
+var path = require("path");
+var shell = require("shelljs");
+var minimatch = require("minimatch");
+
+/**
+ * Updates a target file or directory with a source file or directory.
(Directory updates are
+ * not recursive.) Stats for target and source items must be passed in.
This is an internal
+ * helper function used by other methods in this module.
+ *
+ * @param {string|null} rootDir Root directory (such as a project) to
which target and source
+ * path parameters are relative, or null if the paths are absolute.
The rootDir is omitted
+ * from any logged paths, to make the logs easier to read.
+ * @param {string} targetPath Destination file or directory to be updated.
If it does not exist,
+ * it will be created.
+ * @param {fs.Stats|null} targetStats An instance of fs.Stats for the
target path, or null if
+ * the target does not exist.
+ * @param {string|null} sourcePath Source file or directory to be used to
update the
+ * destination. If the source is null, then the destination is deleted
if it exists.
+ * @param {fs.Stats|null} sourceStats An instance of fs.Stats for the
source path, or null if
+ * the source does not exist.
+ * @param {boolean} force If target and source are both files, and the
force flag is not
+ * set, then the file will not be copied unless the source is newer
than the target.
+ * @param {function} [log] Optional logging callback that takes a string
message describing any
+ * file operations that are performed.
+ * @return {boolean} true if any changes were made, or false if the force
flag is not set
+ * and everything was up to date
+ */
+function updatePathWithStats(
+rootDir, targetPath, targetStats, sourcePath, sourceStats, force,
log) {
+log = log || function(message) { };
+var updated = false;
+
+var targetFullPath = path.join(rootDir || "", targetPath);
+
+if (sourceStats) {
+var sourceFullPath = path.join(rootDir || "", sourcePath);
+
+if (targetStats) {
+// The target exists. But if the directory status doesn't
match the source, delete it.
+if (targetStats.isDirectory() && !sourceStats.isDirectory()) {
+log("rmdir " + targetPath + " (source is a file)");
+shell.rm("-rf", targetFullPath);
+targetStats = null;
+updated = true;
+} else if (!targetStats.isDirectory() &&
sourceStats.isDirectory()) {
+log("delete " + targetPath + " (source is a directory)");
+shell.rm("-f", targetFullPath);
+targetStats = null;
+updated = true;
+}
+}
+
+if (!targetStats) {
+if (sourceStats.isDirectory()) {
+// The target directory does not exist, so it should be
created.
+log("mkdir " + targetPath);
+shell.mkdir("-p", targetFullPath);
+updated = true;
+} else if (sourceStats.isFile()) {
+// The target file does not exist, so it should be copied
from the source.
+log("copy " + sourcePath + " " + targetPath +
+(!force ? " (new file)" : ""));
+shell.cp("-f", sourceFullPath, targetFullPath);
+updated
[GitHub] cordova-lib pull request: CB-11117: Add FileUpdater module to cord...
Github user jasongin commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/429#discussion_r60448617
--- Diff: cordova-common/src/FileUpdater.js ---
@@ -0,0 +1,389 @@
+/**
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements. See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership. The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License. You may obtain a copy of the License at
+
+http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied. See the License for the
+specific language governing permissions and limitations
+under the License.
+*/
+
+"use strict";
+
+var fs = require("fs");
+var path = require("path");
+var shell = require("shelljs");
+var minimatch = require("minimatch");
+
+/**
+ * Updates a target file or directory with a source file or directory.
(Directory updates are
+ * not recursive.) Stats for target and source items must be passed in.
This is an internal
+ * helper function used by other methods in this module.
+ *
+ * @param {string|null} rootDir Root directory (such as a project) to
which target and source
--- End diff --
I was aware of that syntax (and used it for the [log] parameter here). But
my interpretation was that it was for parameters that could be omitted by the
caller, not merely parameters that could be null. The JSDoc documentation isn't
clear about that, but everywhere else I've seen the brackets used was where the
parameter could be omitted.
---
In reply to:
[60384873](https://github.com/apache/cordova-lib/pull/429#discussion_r60384873)
[](ancestors = 60384873)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-android pull request: CB-11117: Use FileUpdater to optimiz...
GitHub user jasongin opened a pull request: https://github.com/apache/cordova-android/pull/295 CB-7: Use FileUpdater to optimize prepare for android platform This uses the FileUpdater module added in https://github.com/apache/cordova-lib/pull/429 to optionally skip copying files that didn't change. Some refactoring was required because previously the target directories would just be wiped before copying; now we need to map out the source and target directories so the FileUpdater has the necessary information to determine the optimal set of file operations. You can merge this pull request into a Git repository by running: $ git pull https://github.com/jasongin/cordova-android master Alternatively you can review and apply these changes as the patch at: https://github.com/apache/cordova-android/pull/295.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #295 commit c8e504de768aa373874fd2f8895b497c5581652b Author: Jason Ginchereau <jason...@microsoft.com> Date: 2016-04-19T20:28:13Z CB-7: Use FileUpdater to optimize prepare for android platform --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-lib pull request: Add FileUpdater module to cordova-common
Github user jasongin commented on the pull request: https://github.com/apache/cordova-lib/pull/429#issuecomment-212121255 I opened CB-7 to track this work. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-lib pull request: Add FileUpdater module to cordova-common
Github user jasongin commented on the pull request: https://github.com/apache/cordova-lib/pull/429#issuecomment-212039256 @nikhilkh @vladimir-kotikov please review --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-lib pull request: Add FileUpdater module to cordova-common
GitHub user jasongin opened a pull request: https://github.com/apache/cordova-lib/pull/429 Add FileUpdater module to cordova-common The new FileUpdater module contains a few functions that enable optimized file copies by comparing timestamps. These functions are primarily intended to be used by each platform's prepare operation to avoid redundantly copying files that haven't changed since the last build, thus greatly speeding up build times after the first build. But the usfulness isn't necessarily limited to the prepare operation; the functions could be used for any Cordova file copy operations that benefit from the same optimization. In this initial change, the FileUpdater isn't used yet anywhere. Look for follow-up changes in other repos which make use of the FileUpdater from platforms' prepare operations, and a CLI change that enables the timestamp-based optimization via a new --incremental flag. You can merge this pull request into a Git repository by running: $ git pull https://github.com/jasongin/cordova-lib master Alternatively you can review and apply these changes as the patch at: https://github.com/apache/cordova-lib/pull/429.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #429 commit 542dd2d57d94829869a1cddbe0ff7b04ed4517df Author: Jason Ginchereau <jason...@microsoft.com> Date: 2016-04-16T02:27:01Z Add FileUpdater module to cordova-common The new FileUpdater module contains a few functions that enable optimized file copies by comparing timestamps. These functions are primarily intended to be used by each platform's prepare operation to avoid redundantly copying files that haven't changed since the last build, thus greatly speeding up build times after the first build. But the usfulness isn't necessarily limited to the prepare operation; the functions could be used for any Cordova file copy operations that benefit from the same optimization. In this initial change, the FileUpdater isn't used yet anywhere. Look for follow-up changes in other repos which make use of the FileUpdater from platforms' prepare operations, and a CLI change that enables the timestamp-based optimization via a new --incremental flag. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-cli pull request: CB-11042: Add cordova run option to skip...
Github user jasongin closed the pull request at: https://github.com/apache/cordova-cli/pull/244 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-ios pull request: CB-11022 Duplicate www files on plugin i...
Github user jasongin commented on the pull request: https://github.com/apache/cordova-ios/pull/218#issuecomment-210559586 :+1: --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-lib pull request: CB-11022 Save modulesMetadata to both ww...
Github user jasongin commented on the pull request: https://github.com/apache/cordova-lib/pull/427#issuecomment-210559039 :+1: --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-windows pull request: CB-11022 Duplicate www files on plug...
Github user jasongin commented on the pull request: https://github.com/apache/cordova-windows/pull/167#issuecomment-210556467 :+1: --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-windows pull request: CB-11022 Duplicate www files on plug...
Github user jasongin commented on a diff in the pull request:
https://github.com/apache/cordova-windows/pull/167#discussion_r59909776
--- Diff: template/cordova/lib/PluginHandler.js ---
@@ -123,24 +123,27 @@ var handlers = {
throw new CordovaError(' tag without required
"target" attribute');
}
-var www = options.usePlatformWww ? project.platformWww :
project.www;
-copyFile(plugin.dir, obj.src, www, obj.target);
+copyFile(plugin.dir, obj.src, project.www, obj.target);
+if (options && options.usePlatformWww) copyFile(plugin.dir,
obj.src, project.platformWww, obj.target);
},
uninstall:function(obj, plugin, project, options) {
var target = obj.target || obj.src;
if (!target) throw new CordovaError(' tag without
required "target" attribute');
-var www = options.usePlatformWww ? project.platformWww :
project.www;
-removeFile(www, target);
-shell.rm('-Rf', path.resolve(www, 'plugins', plugin.id));
+removeFile(project.www, target);
+removeFile(project.www, path.join('plugins', plugin.id));
+if (options && options.usePlatformWww) {
+removeFile(project.platformWww, target);
+removeFile(project.platformWww, path.join('plugins',
plugin.id));
+}
}
},
'js-module': {
install: function (obj, plugin, project, options) {
// Copy the plugin's files into the www directory.
var moduleSource = path.resolve(plugin.dir, obj.src);
-var moduleName = plugin.id + '.' + (obj.name ||
path.parse(obj.src).name);
+var moduleName = plugin.id + '.' + (obj.name ||
path.basename(obj.src, path.extname (obj.src)));
--- End diff --
Is there a reason for this change? Doesn't it give exactly the same result?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-cli pull request: CB-11042: Add cordova run option to skip...
Github user jasongin commented on a diff in the pull request: https://github.com/apache/cordova-cli/pull/244#discussion_r59906168 --- Diff: doc/readme.md --- @@ -464,29 +464,31 @@ cordova build [ [...]] ### Synopsis -Prepares, builds (unless `--nobuild` is specified) and deploys app on specified platform devices/emulators. If a device is connected it will be used, unless an eligible emulator is already running. +Prepares, builds, and deploys app on specified platform devices/emulators. If a device is connected it will be used, unless an eligible emulator is already running. ###Syntax ```bash cordova run [ [...]] -[--list | --nobuild ] +[--list | --debug | --release] +[--noprepare] [--nobuild] [--device|--emulator|--target=] [--buildConfig=] [--browserify] [-- ] ``` -| Option | Description -||-- +| Option | Description +|-|-- | ` [..]` | Platform name(s) to run. If not specified, all platforms are run. -|--nobuild | Skip building -|--debug | Deploy a debug build. This is the default behavior unless `--release` is specified. -|--release | Deploy a release build -|--device| Deploy to a device -|--emulator | Deploy to an emulator -|--target| Deploy to a specific target emulator/device. Use `--list` to display target options -| --list | Lists available targets. Displays both device and emulator deployment targets unless specified +| --list | Lists available targets. Displays both device and emulator deployment targets unless specified +| --debug | Deploy a debug build. This is the default behavior unless `--release` is specified. +| --release | Deploy a release build +| --noprepare | Skip preparing --- End diff -- I updated the PR with a note about requiring 6.2. --- In reply to: [59599203](https://github.com/apache/cordova-cli/pull/244#discussion_r59599203) [](ancestors = 59599203) --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-plugin-camera pull request: CB-10873 Avoid crash due to us...
Github user jasongin commented on the pull request: https://github.com/apache/cordova-plugin-camera/pull/205#issuecomment-209669073 LGTM --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-lib pull request: CB-11022 Improve performance of `cordova...
Github user jasongin commented on the pull request: https://github.com/apache/cordova-lib/pull/423#issuecomment-209620711 Yes, LGTM --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-cli pull request: CB-11042: Add cordova run option to skip...
Github user jasongin commented on the pull request: https://github.com/apache/cordova-cli/pull/244#issuecomment-209571650 @omefire Isn't that covered by the update to docs/readme.md in this change? Or is it duplicated somewhere else? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-plugin-camera pull request: CB-10873 Avoid crash due to us...
Github user jasongin commented on a diff in the pull request:
https://github.com/apache/cordova-plugin-camera/pull/205#discussion_r59583794
--- Diff: src/ios/CDVCamera.m ---
@@ -358,6 +358,7 @@ - (NSData*)processImage:(UIImage*)image
info:(NSDictionary*)info options:(CDVPic
// use image unedited as requested , don't resize
data = UIImageJPEGRepresentation(image, 1.0);
} else {
+data = UIImageJPEGRepresentation(image, [options.quality
floatValue] / 100.0f);
if (options.usesGeolocation) {
--- End diff --
When using the "unedited" image above, the geolocation option is ignored --
is that correct behavior? Or should the if (options.usesGeolocation) be moved
to outside/after the else block?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-cli pull request: CB-11042: Add cordova run option to skip...
GitHub user jasongin opened a pull request: https://github.com/apache/cordova-cli/pull/244 CB-11042: Add cordova run option to skip prepare Parse and document a --noprepare option for the cordova run command. See https://github.com/apache/cordova-lib/pull/426 for the corresponding cordova-lib change that skips the prepare step if the noprepare option is specified. You can merge this pull request into a Git repository by running: $ git pull https://github.com/jasongin/cordova-cli CB-11042 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/cordova-cli/pull/244.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #244 commit 053bbdf841997276112d1675913d7871792f3469 Author: Jason Ginchereau <jason...@microsoft.com> Date: 2016-04-12T20:59:09Z CB-11042: Add cordova run option to skip prepare --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-lib pull request: CB-11042: Add cordova run option to skip...
GitHub user jasongin opened a pull request: https://github.com/apache/cordova-lib/pull/426 CB-11042: Add cordova run option to skip prepare I'm also submitting a PR for the cordova-cli repo to parse and document the --noprepare option. You can merge this pull request into a Git repository by running: $ git pull https://github.com/jasongin/cordova-lib CB-11042 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/cordova-lib/pull/426.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #426 commit 8b60d5f6e718d0e21ae5c39f5564e90dffda2bb0 Author: Jason Ginchereau <jason...@microsoft.com> Date: 2016-04-12T20:57:06Z CB-11042: Add cordova run option to skip prepare --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-lib pull request: CB-11022 Improve performance of `cordova...
Github user jasongin commented on the pull request: https://github.com/apache/cordova-lib/pull/423#issuecomment-208603032 OK, I didn't realize config file transformations were also done already in plugin add. And I think I have a better understanding of the code now. My understanding is we still need to do prepare for any platforms which don't do the following things: 1. Implement the PlatformApi 2. Copy over the plugins' www files in addPlugin() 3. Return true from addPlugin() So do we also need changes to iOS and Windows platforms to do 2 and 3, to match what you did for Android in the linked PR? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-lib pull request: CB-11022 Improve performance of `cordova...
Github user jasongin commented on the pull request: https://github.com/apache/cordova-lib/pull/423#issuecomment-207527874 This code is pretty hard to follow. I don't mean your changes, but plugin installation overall is more complex than I expected. How do these changes affect plugins' config file transformations? Those are normally processed during prepare, right? How would we ever fully eliminate prepare after installing plugins. Don't we need to keep at least the config file transformations? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-docs pull request: CB-11020, CB-11040 Documenting event fi...
Github user jasongin commented on the pull request: https://github.com/apache/cordova-docs/pull/573#issuecomment-207138716 LGTM --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-docs pull request: CB-11020, CB-11040 Documenting event fi...
Github user jasongin commented on a diff in the pull request: https://github.com/apache/cordova-docs/pull/573#discussion_r58962639 --- Diff: www/docs/en/dev/config_ref/index.md --- @@ -348,6 +349,7 @@ platform. See [Customize icons topic](images.html) for more information. + --- End diff -- Maybe 10.0.10586.0 would be a better example? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-plugin-file pull request: adding sample section to readme
Github user jasongin commented on the pull request: https://github.com/apache/cordova-plugin-file/pull/176#issuecomment-207123605 Rob, what platform are you looking at? The W3C FileSystems spec for web browsers is deprecated; what that effectively means is no other browsers will implement the spec. Chrome was the only browser to ever implement that spec, and I believe it still works in current builds of Chrome However, the Cordova file plugin implements that spec as a polyfill for Cordova platforms other than the "Browser" platform (because the Browser platform isn't able to call native plugin APIs). So it makes the deprecation of the spec mostly irrelevant for Cordova app developers. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-plugin-file pull request: adding sample section to readme
Github user jasongin commented on the pull request: https://github.com/apache/cordova-plugin-file/pull/176#issuecomment-207076949 Looks good, thanks! --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
