[GitHub] cordova-lib pull request: CB-8595 Merge platforms.js from cordova ...
Github user mmocny commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/183#discussion_r26214059
--- Diff: cordova-lib/src/platforms/platforms.js ---
@@ -0,0 +1,102 @@
+/**
+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.
+*/
+
+var platforms = require('./platformsConfig.json');
+
+// Remove this block soon. The parser property is no longer used in
+// cordova-lib but some downstream tools still use it.
+var addModuleProperty = require('../cordova/util').addModuleProperty;
+Object.keys(platforms).forEach(function(key) {
+var obj = platforms[key];
+if (obj.parser_file) {
+addModuleProperty(module, 'parser', obj.parser_file, false, obj);
+}
+});
+
+
+// Avoid loading the same platform projects more than once (identified by
path)
+var cachedProjects = {};
+
+var PARSER_PUBLIC_METHODS = [
+'config_xml',
+'cordovajs_path',
+'update_from_config',
+'update_overrides',
+'update_project',
+'update_www',
+'www_dir',
+];
+
+var HANDLER_PUBLIC_METHODS = [
+'package_name',
+'parseProjectFile',
+'purgeProjectFileCache',
+];
+
+
+// A single class that exposes functionality from platform specific files
from
+// both places cordova/metadata and plugman/platforms. Hopefully, to be
soon
+// replaced by real unified platform specific classes.
+function PlatformProjectAdapter(platform, platformRootDir) {
+var self = this;
+self.root = platformRootDir;
+self.platform = platform;
+var ParserConstructor = require(platforms[platform].parser_file);
+self.parser = new ParserConstructor(platformRootDir);
+self.handler = require(platforms[platform].handler_file);
+
+// Expos all public methods from the parser and handler, properly
bound.
+PARSER_PUBLIC_METHODS.forEach(function(method) {
+if (self.parser[method]) {
--- End diff --
Is it an error to not be there? Should we throw if this fails?
---
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-8595 Merge platforms.js from cordova ...
Github user mmocny commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/183#discussion_r26224327
--- Diff: cordova-lib/src/platforms/platforms.js ---
@@ -0,0 +1,102 @@
+/**
+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.
+*/
+
+var platforms = require('./platformsConfig.json');
+
+// Remove this block soon. The parser property is no longer used in
+// cordova-lib but some downstream tools still use it.
+var addModuleProperty = require('../cordova/util').addModuleProperty;
+Object.keys(platforms).forEach(function(key) {
+var obj = platforms[key];
+if (obj.parser_file) {
+addModuleProperty(module, 'parser', obj.parser_file, false, obj);
+}
+});
+
+
+// Avoid loading the same platform projects more than once (identified by
path)
+var cachedProjects = {};
+
+var PARSER_PUBLIC_METHODS = [
+'config_xml',
+'cordovajs_path',
+'update_from_config',
+'update_overrides',
+'update_project',
+'update_www',
+'www_dir',
+];
+
+var HANDLER_PUBLIC_METHODS = [
+'package_name',
+'parseProjectFile',
+'purgeProjectFileCache',
+];
+
+
+// A single class that exposes functionality from platform specific files
from
+// both places cordova/metadata and plugman/platforms. Hopefully, to be
soon
+// replaced by real unified platform specific classes.
+function PlatformProjectAdapter(platform, platformRootDir) {
+var self = this;
+self.root = platformRootDir;
+self.platform = platform;
+var ParserConstructor = require(platforms[platform].parser_file);
+self.parser = new ParserConstructor(platformRootDir);
+self.handler = require(platforms[platform].handler_file);
+
+// Expos all public methods from the parser and handler, properly
bound.
+PARSER_PUBLIC_METHODS.forEach(function(method) {
+if (self.parser[method]) {
+self[method] = self.parser[method].bind(self.parser);
+}
+});
+
+HANDLER_PUBLIC_METHODS.forEach(function(method) {
+if (self.handler[method]) {
+self[method] = self.handler[method].bind(self.handler);
+}
+});
+
+self.getInstaller = function(type) {
+return self.handler[type].install;
+};
+
+self.getUninstaller = function(type) {
+return self.handler[type].uninstall;
+};
+}
+
+// getPlatformProject() should be the only method of instantiating the
+// PlatformProject classes for now.
+function getPlatformProject(platform, platformRootDir) {
+var cached = cachedProjects[platformRootDir];
+if (cached && cached.platform == platform) {
--- End diff --
Okay, Fair enough.
On Wed, Mar 11, 2015 at 10:57 AM Mark Koudritsky
wrote:
> In cordova-lib/src/platforms/platforms.js
> <https://github.com/apache/cordova-lib/pull/183#discussion_r26218885>:
>
> > +});
> > +
> > +self.getInstaller = function(type) {
> > +return self.handler[type].install;
> > +};
> > +
> > +self.getUninstaller = function(type) {
> > +return self.handler[type].uninstall;
> > +};
> > +}
> > +
> > +// getPlatformProject() should be the only method of instantiating the
> > +// PlatformProject classes for now.
> > +function getPlatformProject(platform, platformRootDir) {
> > +var cached = cachedProjects[platformRootDir];
> > +if (cached && cached.platform == platform) {
>
> The tests sometimes reuse the same dir for several platforms. Added the
> platform check afte
[GitHub] cordova-lib pull request: CB-8595 Merge platforms.js from cordova ...
Github user mmocny commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/183#discussion_r26215219
--- Diff: cordova-lib/src/plugman/uninstall.js ---
@@ -293,58 +293,25 @@ function runUninstallPlatform(actions, platform,
project_dir, plugin_dir, plugin
function handleUninstall(actions, platform, pluginInfo, project_dir,
www_dir, plugins_dir, is_top_level, options) {
var plugin_id = pluginInfo.id;
var plugin_dir = pluginInfo.dir;
-var platform_modules = require('./platforms');
-var handler = platform_modules[platform];
-www_dir = www_dir || handler.www_dir(project_dir);
+var handler = platform_modules.getPlatformProject(platform,
project_dir);
+www_dir = www_dir || handler.www_dir();
events.emit('log', 'Uninstalling ' + plugin_id + ' from ' + platform);
+var pluginItems = pluginInfo.getFilesAndFrameworks(platform);
var assets = pluginInfo.getAssets(platform);
--- End diff --
Why are assets different?
---
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-8595 Merge platforms.js from cordova ...
Github user mmocny commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/183#discussion_r26214642
--- Diff: cordova-lib/src/platforms/platforms.js ---
@@ -0,0 +1,102 @@
+/**
+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.
+*/
+
+var platforms = require('./platformsConfig.json');
+
+// Remove this block soon. The parser property is no longer used in
+// cordova-lib but some downstream tools still use it.
+var addModuleProperty = require('../cordova/util').addModuleProperty;
+Object.keys(platforms).forEach(function(key) {
+var obj = platforms[key];
+if (obj.parser_file) {
+addModuleProperty(module, 'parser', obj.parser_file, false, obj);
+}
+});
+
+
+// Avoid loading the same platform projects more than once (identified by
path)
+var cachedProjects = {};
+
+var PARSER_PUBLIC_METHODS = [
+'config_xml',
+'cordovajs_path',
+'update_from_config',
+'update_overrides',
+'update_project',
+'update_www',
+'www_dir',
+];
+
+var HANDLER_PUBLIC_METHODS = [
+'package_name',
+'parseProjectFile',
+'purgeProjectFileCache',
+];
+
+
+// A single class that exposes functionality from platform specific files
from
+// both places cordova/metadata and plugman/platforms. Hopefully, to be
soon
+// replaced by real unified platform specific classes.
+function PlatformProjectAdapter(platform, platformRootDir) {
+var self = this;
+self.root = platformRootDir;
+self.platform = platform;
+var ParserConstructor = require(platforms[platform].parser_file);
+self.parser = new ParserConstructor(platformRootDir);
+self.handler = require(platforms[platform].handler_file);
+
+// Expos all public methods from the parser and handler, properly
bound.
+PARSER_PUBLIC_METHODS.forEach(function(method) {
+if (self.parser[method]) {
+self[method] = self.parser[method].bind(self.parser);
+}
+});
+
+HANDLER_PUBLIC_METHODS.forEach(function(method) {
+if (self.handler[method]) {
+self[method] = self.handler[method].bind(self.handler);
+}
+});
+
+self.getInstaller = function(type) {
+return self.handler[type].install;
+};
+
+self.getUninstaller = function(type) {
+return self.handler[type].uninstall;
+};
+}
+
+// getPlatformProject() should be the only method of instantiating the
+// PlatformProject classes for now.
+function getPlatformProject(platform, platformRootDir) {
+var cached = cachedProjects[platformRootDir];
+if (cached && cached.platform == platform) {
--- End diff --
If we have a `cached` PlatformProject already, shouldn't we just assert
that `cached.platform == platform` inside the if? If the platform is wrong,
theres a bug right?
---
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-8595 Merge platforms.js from cordova ...
Github user mmocny commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/183#discussion_r26214063
--- Diff: cordova-lib/src/platforms/platforms.js ---
@@ -0,0 +1,102 @@
+/**
+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.
+*/
+
+var platforms = require('./platformsConfig.json');
+
+// Remove this block soon. The parser property is no longer used in
+// cordova-lib but some downstream tools still use it.
+var addModuleProperty = require('../cordova/util').addModuleProperty;
+Object.keys(platforms).forEach(function(key) {
+var obj = platforms[key];
+if (obj.parser_file) {
+addModuleProperty(module, 'parser', obj.parser_file, false, obj);
+}
+});
+
+
+// Avoid loading the same platform projects more than once (identified by
path)
+var cachedProjects = {};
+
+var PARSER_PUBLIC_METHODS = [
+'config_xml',
+'cordovajs_path',
+'update_from_config',
+'update_overrides',
+'update_project',
+'update_www',
+'www_dir',
+];
+
+var HANDLER_PUBLIC_METHODS = [
+'package_name',
+'parseProjectFile',
+'purgeProjectFileCache',
+];
+
+
+// A single class that exposes functionality from platform specific files
from
+// both places cordova/metadata and plugman/platforms. Hopefully, to be
soon
+// replaced by real unified platform specific classes.
+function PlatformProjectAdapter(platform, platformRootDir) {
+var self = this;
+self.root = platformRootDir;
+self.platform = platform;
+var ParserConstructor = require(platforms[platform].parser_file);
+self.parser = new ParserConstructor(platformRootDir);
+self.handler = require(platforms[platform].handler_file);
+
+// Expos all public methods from the parser and handler, properly
bound.
+PARSER_PUBLIC_METHODS.forEach(function(method) {
+if (self.parser[method]) {
+self[method] = self.parser[method].bind(self.parser);
+}
+});
+
+HANDLER_PUBLIC_METHODS.forEach(function(method) {
+if (self.handler[method]) {
--- End diff --
ditto.
---
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-8595 Merge platforms.js from cordova ...
Github user mmocny commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/183#discussion_r26213790
--- Diff: cordova-lib/src/PluginInfo.js ---
@@ -288,6 +293,19 @@ function PluginInfo(dirname) {
return ret;
});
};
+
+self.getFilesAndFrameworks = getFilesAndFrameworks;
+function getFilesAndFrameworks(platform) {
+var items = [];
+// Please avoid changing the order of the calls below, files will
be
+// installed in this order.
+items = items.concat(self.getSourceFiles(platform));
+items = items.concat(self.getHeaderFiles(platform));
+items = items.concat(self.getResourceFiles(platform));
+items = items.concat(self.getFrameworks(platform));
+items = items.concat(self.getLibFiles(platform));
--- End diff --
Tip: `concat` actually accepts any number of arguments and will concat all
in one go.
Can also do: `[].concat.apply([], [ self.getSourceFiles(..),
self.getHeaderFiles(..), ...])`
Which I think looks nicest.
---
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-8551 npm registry integration for fet...
Github user mmocny commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/175#discussion_r25722381
--- Diff: cordova-lib/src/plugman/registry/registry.js ---
@@ -322,3 +324,102 @@ function makeRequest (method, where, what, cb_) {
return req;
}
+
+/**
+ * @method fetchNPM
+ * @param {Array} with one element - the plugin id or "id@version"
+ * @return {Promise.} Promised path to fetched package.
+ */
+function fetchNPM(plugin, client) {
+return initSettings(true)
+.then(function (settings) {
+return Q.nfcall(npm.load)
+// configure npm here instead of passing parameters to npm.load
due to CB-7670
+.then(function () {
+for (var prop in settings){
+npm.config.set(prop, settings[prop]);
+}
+});
+})
+.then(function() {
+events.emit('log', 'Fetching plugin "' + plugin + '" via npm');
+return Q.ninvoke(npm.commands, 'cache', ['add', plugin]);
+})
+.then(function(info) {
+var cl = (client === 'plugman' ? 'plugman' : 'cordova-cli');
+bumpCounter(info, cl);
+var pluginDir = path.resolve(npm.cache, info.name, info.version,
'package');
+// Unpack the plugin that was added to the cache (CB-8154)
+var package_tgz = path.resolve(npm.cache, info.name, info.version,
'package.tgz');
+return unpack.unpackTgz(package_tgz, pluginDir);
+})
+.fail(function(error) {
+events.emit('log', 'Fetching from npm registry failed');
+return Q.reject(error);
+});
+}
+
+
+/**
+ * @method fetchPlugReg
+ * @param {Array} with one element - the plugin id or "id@version"
+ * @return {Promise.} Promised path to fetched package.
+ */
+function fetchPlugReg(plugin, client) {
+return initSettings()
+.then(function (settings) {
+return Q.nfcall(npm.load)
+// configure npm here instead of passing parameters to npm.load
due to CB-7670
+.then(function () {
+for (var prop in settings){
+npm.config.set(prop, settings[prop]);
+}
+});
+})
+.then(function() {
+events.emit('log', 'Fetching plugin "' + plugin + '" via plugin
registry');
+return Q.ninvoke(npm.commands, 'cache', ['add', plugin]);
+})
+.then(function(info) {
+var cl = (client === 'plugman' ? 'plugman' : 'cordova-cli');
+bumpCounter(info, cl);
+var pluginDir = path.resolve(npm.cache, info.name, info.version,
'package');
+// Unpack the plugin that was added to the cache (CB-8154)
+var package_tgz = path.resolve(npm.cache, info.name, info.version,
'package.tgz');
+return unpack.unpackTgz(package_tgz, pluginDir);
+})
+.fail(function(error) {
+events.emit('log', 'Fetching from cordova plugin registry failed');
+return Q.reject(error);
+});
+}
+
+/**
+ * @method checkPluginID
+ * @param {Array} with one element - the plugin id or "id@version"
+ * @return {Promise.} Promised path to fetched package.
+ */
+function checkPluginID(plugin) {
+//if plugin id is not reverse domain name style, skip CPR and fetch
from npm
+
+//Create regex to for digits, words and dashes and three dots in
plugin ids which excludes @VERSION.
+var re = /([\w-]*\.[\w-]*\.[\w-]*\.[\w-]*[^@])/;
+var pluginID = plugin.match(re);
+//If pluginID equals null, plugin is not reverse domain name style
+if(pluginID === null) {
+events.emit('verbose', 'Skipping CPR');
+//Q.reject will send us straight to the fail method which is where
fetchNPM gets called.
+return Q.reject();
+} else {
+//Reverse domain name style plugin ID
+//Check if a mapping exists for the pluginID
+//if it does, warn the users to use package-name
+var packageName = pluginMapper[pluginID[0]];
+if(packageName) {
+events.emit('log', 'WARNING: ' + plugin + ' has been renamed
to ' +
+packageName + ' and moved to npm. Please use `cordova
plugin add ' +
+packageName + '` next time.');
--- End diff --
Okay, thanks for clarification.
[GitHub] cordova-lib pull request: CB-8551 npm registry integration for fet...
Github user mmocny commented on the pull request: https://github.com/apache/cordova-lib/pull/175#issuecomment-76999272 Overall, impressed with the lack of significant impact. --- 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-8551 npm registry integration for fet...
Github user mmocny commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/175#discussion_r25710017
--- Diff: cordova-lib/src/plugman/registry/registry.js ---
@@ -322,3 +324,102 @@ function makeRequest (method, where, what, cb_) {
return req;
}
+
+/**
+ * @method fetchNPM
+ * @param {Array} with one element - the plugin id or "id@version"
+ * @return {Promise.} Promised path to fetched package.
+ */
+function fetchNPM(plugin, client) {
+return initSettings(true)
+.then(function (settings) {
+return Q.nfcall(npm.load)
+// configure npm here instead of passing parameters to npm.load
due to CB-7670
+.then(function () {
+for (var prop in settings){
+npm.config.set(prop, settings[prop]);
+}
+});
+})
+.then(function() {
+events.emit('log', 'Fetching plugin "' + plugin + '" via npm');
+return Q.ninvoke(npm.commands, 'cache', ['add', plugin]);
+})
+.then(function(info) {
+var cl = (client === 'plugman' ? 'plugman' : 'cordova-cli');
+bumpCounter(info, cl);
+var pluginDir = path.resolve(npm.cache, info.name, info.version,
'package');
+// Unpack the plugin that was added to the cache (CB-8154)
+var package_tgz = path.resolve(npm.cache, info.name, info.version,
'package.tgz');
+return unpack.unpackTgz(package_tgz, pluginDir);
+})
+.fail(function(error) {
+events.emit('log', 'Fetching from npm registry failed');
+return Q.reject(error);
+});
+}
+
+
+/**
+ * @method fetchPlugReg
+ * @param {Array} with one element - the plugin id or "id@version"
+ * @return {Promise.} Promised path to fetched package.
+ */
+function fetchPlugReg(plugin, client) {
+return initSettings()
+.then(function (settings) {
+return Q.nfcall(npm.load)
+// configure npm here instead of passing parameters to npm.load
due to CB-7670
+.then(function () {
+for (var prop in settings){
+npm.config.set(prop, settings[prop]);
+}
+});
+})
+.then(function() {
+events.emit('log', 'Fetching plugin "' + plugin + '" via plugin
registry');
+return Q.ninvoke(npm.commands, 'cache', ['add', plugin]);
+})
+.then(function(info) {
+var cl = (client === 'plugman' ? 'plugman' : 'cordova-cli');
+bumpCounter(info, cl);
+var pluginDir = path.resolve(npm.cache, info.name, info.version,
'package');
+// Unpack the plugin that was added to the cache (CB-8154)
+var package_tgz = path.resolve(npm.cache, info.name, info.version,
'package.tgz');
+return unpack.unpackTgz(package_tgz, pluginDir);
+})
+.fail(function(error) {
+events.emit('log', 'Fetching from cordova plugin registry failed');
+return Q.reject(error);
+});
+}
+
+/**
+ * @method checkPluginID
+ * @param {Array} with one element - the plugin id or "id@version"
+ * @return {Promise.} Promised path to fetched package.
+ */
+function checkPluginID(plugin) {
+//if plugin id is not reverse domain name style, skip CPR and fetch
from npm
+
+//Create regex to for digits, words and dashes and three dots in
plugin ids which excludes @VERSION.
+var re = /([\w-]*\.[\w-]*\.[\w-]*\.[\w-]*[^@])/;
+var pluginID = plugin.match(re);
+//If pluginID equals null, plugin is not reverse domain name style
+if(pluginID === null) {
+events.emit('verbose', 'Skipping CPR');
+//Q.reject will send us straight to the fail method which is where
fetchNPM gets called.
+return Q.reject();
+} else {
+//Reverse domain name style plugin ID
+//Check if a mapping exists for the pluginID
+//if it does, warn the users to use package-name
+var packageName = pluginMapper[pluginID[0]];
+if(packageName) {
+events.emit('log', 'WARNING: ' + plugin + ' has been renamed
to ' +
+packageName + ' and moved to npm. Please use `cordova
plugin add ' +
+packageName + '` next time.');
--- End diff --
Suggestion: `'next time
[GitHub] cordova-lib pull request: CB-8551 npm registry integration for fet...
Github user mmocny commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/175#discussion_r25709825
--- Diff: cordova-lib/src/plugman/registry/registry.js ---
@@ -322,3 +324,102 @@ function makeRequest (method, where, what, cb_) {
return req;
}
+
+/**
+ * @method fetchNPM
+ * @param {Array} with one element - the plugin id or "id@version"
+ * @return {Promise.} Promised path to fetched package.
+ */
+function fetchNPM(plugin, client) {
+return initSettings(true)
+.then(function (settings) {
+return Q.nfcall(npm.load)
+// configure npm here instead of passing parameters to npm.load
due to CB-7670
+.then(function () {
+for (var prop in settings){
+npm.config.set(prop, settings[prop]);
+}
+});
+})
+.then(function() {
+events.emit('log', 'Fetching plugin "' + plugin + '" via npm');
+return Q.ninvoke(npm.commands, 'cache', ['add', plugin]);
+})
+.then(function(info) {
+var cl = (client === 'plugman' ? 'plugman' : 'cordova-cli');
+bumpCounter(info, cl);
+var pluginDir = path.resolve(npm.cache, info.name, info.version,
'package');
+// Unpack the plugin that was added to the cache (CB-8154)
+var package_tgz = path.resolve(npm.cache, info.name, info.version,
'package.tgz');
+return unpack.unpackTgz(package_tgz, pluginDir);
+})
+.fail(function(error) {
+events.emit('log', 'Fetching from npm registry failed');
+return Q.reject(error);
+});
+}
+
+
+/**
+ * @method fetchPlugReg
+ * @param {Array} with one element - the plugin id or "id@version"
+ * @return {Promise.} Promised path to fetched package.
+ */
+function fetchPlugReg(plugin, client) {
+return initSettings()
+.then(function (settings) {
+return Q.nfcall(npm.load)
+// configure npm here instead of passing parameters to npm.load
due to CB-7670
+.then(function () {
+for (var prop in settings){
+npm.config.set(prop, settings[prop]);
+}
+});
+})
+.then(function() {
+events.emit('log', 'Fetching plugin "' + plugin + '" via plugin
registry');
+return Q.ninvoke(npm.commands, 'cache', ['add', plugin]);
+})
+.then(function(info) {
+var cl = (client === 'plugman' ? 'plugman' : 'cordova-cli');
+bumpCounter(info, cl);
+var pluginDir = path.resolve(npm.cache, info.name, info.version,
'package');
+// Unpack the plugin that was added to the cache (CB-8154)
+var package_tgz = path.resolve(npm.cache, info.name, info.version,
'package.tgz');
+return unpack.unpackTgz(package_tgz, pluginDir);
+})
+.fail(function(error) {
+events.emit('log', 'Fetching from cordova plugin registry failed');
+return Q.reject(error);
+});
+}
+
+/**
+ * @method checkPluginID
+ * @param {Array} with one element - the plugin id or "id@version"
+ * @return {Promise.} Promised path to fetched package.
+ */
+function checkPluginID(plugin) {
+//if plugin id is not reverse domain name style, skip CPR and fetch
from npm
+
+//Create regex to for digits, words and dashes and three dots in
plugin ids which excludes @VERSION.
+var re = /([\w-]*\.[\w-]*\.[\w-]*\.[\w-]*[^@])/;
+var pluginID = plugin.match(re);
+//If pluginID equals null, plugin is not reverse domain name style
+if(pluginID === null) {
+events.emit('verbose', 'Skipping CPR');
+//Q.reject will send us straight to the fail method which is where
fetchNPM gets called.
--- End diff --
Bravo, thanks for adding straight-to-npm.
---
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-8551 npm registry integration for fet...
Github user mmocny commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/175#discussion_r25709472
--- Diff: cordova-lib/src/plugman/registry/registry.js ---
@@ -218,10 +205,18 @@ module.exports = {
/**
* @method initSettings
+ * @param {Boolean} using npm registry
* @return {Promise.} Promised settings.
*/
-function initSettings() {
+function initSettings(npm) {
--- End diff --
nit: `npm` -> `useNpmRegistry`. Otherwise it looks like a reference to the
node 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-8551 npm registry integration for fet...
Github user mmocny commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/175#discussion_r25709236
--- Diff: cordova-lib/src/cordova/plugin.js ---
@@ -238,7 +239,21 @@ module.exports = function plugin(command, targets,
opts) {
return opts.plugins.reduce(function(soFar, target) {
// Check if we have the plugin.
if (plugins.indexOf(target) < 0) {
-return Q.reject(new CordovaError('Plugin "' +
target + '" is not present in the project. See `'+cordova_util.binname+' plugin
list`.'));
+// Convert target from package-name to package-id
if necessary
+var keys = Object.keys(pluginMapper);
+//Traverse through pluginMapper values to see if
it equals our target.
+//Cordova-plugin-device would get changes to
org.apache.cordova.device
+for (var i = 0; i < keys.length; i++) {
--- End diff --
Nevermind, I see its doing reverse mapping.
---
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-8551 npm registry integration for fet...
Github user mmocny commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/175#discussion_r25709035
--- Diff: cordova-lib/src/cordova/plugin.js ---
@@ -238,7 +239,21 @@ module.exports = function plugin(command, targets,
opts) {
return opts.plugins.reduce(function(soFar, target) {
// Check if we have the plugin.
if (plugins.indexOf(target) < 0) {
-return Q.reject(new CordovaError('Plugin "' +
target + '" is not present in the project. See `'+cordova_util.binname+' plugin
list`.'));
+// Convert target from package-name to package-id
if necessary
+var keys = Object.keys(pluginMapper);
+//Traverse through pluginMapper values to see if
it equals our target.
+//Cordova-plugin-device would get changes to
org.apache.cordova.device
+for (var i = 0; i < keys.length; i++) {
--- End diff --
I don't know why we are iterating anyway? Its already old_id->new_id.
Can't we just `.hasOwnProperty()` ?
---
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-8551 npm registry integration for fet...
Github user mmocny commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/175#discussion_r25708820
--- Diff: cordova-lib/package.json ---
@@ -19,6 +19,7 @@
"dependencies": {
"bplist-parser": "0.0.6",
"cordova-js": "3.8.0",
+"cordova-registry-mapper": "0.0.2",
--- End diff --
Looks like: https://github.com/stevengill/cordova-registry-mapper
---
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 commit comment: fcf37d03d12301d26cb4bcbb0107b085325f40...
Github user mmocny commented on commit fcf37d03d12301d26cb4bcbb0107b085325f40d6: https://github.com/apache/cordova-lib/commit/fcf37d03d12301d26cb4bcbb0107b085325f40d6#commitcomment-9664080 In cordova-lib/src/plugman/createpackagejson.js: In cordova-lib/src/plugman/createpackagejson.js on line 46: Aside: There are a lot of "if (err) throw err;" statements in node callback code. I've been using Q for a long time and have grown quite fond of Q.nfcall(...).done() where it does this behaviour automatically for me. Just pondering out loud as Promises become available to js by default (as is the case in io.js 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-lib commit comment: fcf37d03d12301d26cb4bcbb0107b085325f40...
Github user mmocny commented on commit fcf37d03d12301d26cb4bcbb0107b085325f40d6: https://github.com/apache/cordova-lib/commit/fcf37d03d12301d26cb4bcbb0107b085325f40d6#commitcomment-9664041 In cordova-lib/src/plugman/plugman.js: In cordova-lib/src/plugman/plugman.js on line 226: Nit: Pass in `cli_opts.argv.remain[0]` here, and change `createpackagejson` to accept an actual path. If you actually do want multiple args, and want `createpackagejson` to do the arg parsing (I don't think so), then change this variable name here at least to `args`. --- 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 commit comment: fcf37d03d12301d26cb4bcbb0107b085325f40...
Github user mmocny commented on commit fcf37d03d12301d26cb4bcbb0107b085325f40d6: https://github.com/apache/cordova-lib/commit/fcf37d03d12301d26cb4bcbb0107b085325f40d6#commitcomment-9663971 In cordova-lib/src/plugman/init-defaults.js: In cordova-lib/src/plugman/init-defaults.js on line 1: This file is supposed to be a [PromZard](https://github.com/isaacs/promzard) right? Perhaps a comment to this extent, it took a bit of reading to figure this out. --- 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 commit comment: fcf37d03d12301d26cb4bcbb0107b085325f40...
Github user mmocny commented on commit fcf37d03d12301d26cb4bcbb0107b085325f40d6: https://github.com/apache/cordova-lib/commit/fcf37d03d12301d26cb4bcbb0107b085325f40d6#commitcomment-9663882 In cordova-lib/src/plugman/init-defaults.js: In cordova-lib/src/plugman/init-defaults.js on line 13: Nit: newline after brace and before return? Otherwise its unclear that this is a factory. --- 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 commit comment: fcf37d03d12301d26cb4bcbb0107b085325f40...
Github user mmocny commented on commit fcf37d03d12301d26cb4bcbb0107b085325f40d6: https://github.com/apache/cordova-lib/commit/fcf37d03d12301d26cb4bcbb0107b085325f40d6#commitcomment-9663850 In cordova-lib/src/plugman/createpackagejson.js: In cordova-lib/src/plugman/createpackagejson.js on line 48: Cool! Didn't know about require.resolve() --- 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 commit comment: fcf37d03d12301d26cb4bcbb0107b085325f40...
Github user mmocny commented on commit fcf37d03d12301d26cb4bcbb0107b085325f40d6: https://github.com/apache/cordova-lib/commit/fcf37d03d12301d26cb4bcbb0107b085325f40d6#commitcomment-9663808 In cordova-lib/src/PluginInfo.js: In cordova-lib/src/PluginInfo.js on line 317: Nit: For next time, someArray.map() would have been more succinct and appropriate than in-place modification. --- 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-browser pull request: CB-8206: Browser platform: Add suppo...
Github user mmocny commented on the pull request: https://github.com/apache/cordova-browser/pull/6#issuecomment-71211172 (Josh I'll let you have the last word on landing, you have been running 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-browser pull request: CB-8206: Browser platform: Add suppo...
Github user mmocny commented on the pull request: https://github.com/apache/cordova-browser/pull/6#issuecomment-71210993 ..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-browser pull request: CB-8206: Browser platform: Add suppo...
Github user mmocny commented on the pull request: https://github.com/apache/cordova-browser/pull/6#issuecomment-71210975 I reviewed this in a previous iteration, and the latest changes look fine. Since this feature does not exist yet anyway, and I'm not too concerned about inline updates to browser platform, I think this is more than ready to ship. --- 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-test-framework pull request: CB-7459 Add header to ...
Github user mmocny commented on a diff in the pull request:
https://github.com/apache/cordova-plugin-test-framework/pull/6#discussion_r22968802
--- Diff: www/main.js ---
@@ -215,6 +237,12 @@ function createEnablerList() {
enablerList.appendChild(checkButtonBar);
+ header.appendChild(headerPrefix);
+ header.appendChild(enabledCount);
+ header.appendChild(totalCount);
+ header.appendChild(headerSuffix);
--- End diff --
This seems needlessly complex. Why not just a single text element, and we
set the whole "Running X of Y" text each time? We already append the '/' to
the first span and we aren't using any fancy data binding.
---
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-test-framework pull request: CB-7459 Add header to ...
Github user mmocny commented on a diff in the pull request:
https://github.com/apache/cordova-plugin-test-framework/pull/6#discussion_r22968690
--- Diff: www/main.js ---
@@ -225,6 +253,43 @@ function createEnablerList() {
/**/
+function updateTotalTestCount(totalCount) {
+ var label = document.getElementById('test-total-count');
+
+ label.setAttribute('count', totalCount);
+ label.innerText = totalCount;
+}
+
+/**/
+
+function updateEnabledTestCount(enabledCount) {
+
+ var enabledLabel = document.getElementById('test-enabled-count');
+
+ if (!enabledCount) {
+// Determine how many tests are currently enabled
+var cdvtests =
cordova.require('org.apache.cordova.test-framework.cdvtests');
+var enabled = 0;
+iterateAutoTests(cdvtests, function(api, testModule) {
+ if (testModule.getEnabled()) {
+enabled++;
+ }
+});
+enabledCount = enabled;
+ }
+
+ // Compare enabled count vs the total to show if all tests are to be run
+ var totalCount =
document.getElementById('test-total-count').getAttribute('count');
--- End diff --
Why use an attribute on the tag and not just a javascript variable?
---
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-browser pull request: CB-8206: Browser platform: Add suppo...
Github user mmocny commented on a diff in the pull request: https://github.com/apache/cordova-browser/pull/6#discussion_r22960109 --- Diff: node_modules/.bin/shjs --- @@ -1 +1,15 @@ -../shelljs/bin/shjs --- End diff -- For offline installs you still need to pre-fetch, where you can do the npm install step as well. And I'm not at this point discussing what does into an RC, just what's stored in our git master. --- 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-browser pull request: CB-8206: Browser platform: Add suppo...
Github user mmocny commented on a diff in the pull request: https://github.com/apache/cordova-browser/pull/6#discussion_r22957388 --- Diff: node_modules/.bin/shjs --- @@ -1 +1,15 @@ -../shelljs/bin/shjs --- End diff -- Why are node_modules even checked in? Shouldn't they just be added to .gitignore? I double checked, and the only other cordova repo that checks these in is firefoxos, which I think is also an oversight. --- 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-browser pull request: CB-7978 Fixed launching chrome in Li...
Github user mmocny commented on a diff in the pull request:
https://github.com/apache/cordova-browser/pull/4#discussion_r20839578
--- Diff: bin/templates/project/cordova/run ---
@@ -20,10 +20,33 @@
*/
var shell = require('shelljs'),
-spawn = require('child_process').spawn,
-project = 'file://' + shell.pwd() +
'/platforms/browser/www/index.html';
+ fs = require('fs');
+var configFile = shell.pwd() + '/config.xml';
+var configXML = fs.readFileSync(configFile, 'utf8');
+var sourceFile = /
[GitHub] cordova-browser pull request: CB-7978 Fixed launching chrome in Li...
Github user mmocny commented on a diff in the pull request:
https://github.com/apache/cordova-browser/pull/4#discussion_r20823674
--- Diff: bin/templates/project/cordova/run ---
@@ -20,10 +20,33 @@
*/
var shell = require('shelljs'),
-spawn = require('child_process').spawn,
-project = 'file://' + shell.pwd() +
'/platforms/browser/www/index.html';
+ fs = require('fs');
+var configFile = shell.pwd() + '/config.xml';
+var configXML = fs.readFileSync(configFile, 'utf8');
+var sourceFile = /
[GitHub] cordova-browser pull request: CB-7978 Fixed launching chrome in Li...
Github user mmocny commented on a diff in the pull request:
https://github.com/apache/cordova-browser/pull/4#discussion_r20823394
--- Diff: bin/templates/project/cordova/run ---
@@ -20,10 +20,33 @@
*/
var shell = require('shelljs'),
-spawn = require('child_process').spawn,
-project = 'file://' + shell.pwd() +
'/platforms/browser/www/index.html';
+ fs = require('fs');
+var configFile = shell.pwd() + '/config.xml';
+var configXML = fs.readFileSync(configFile, 'utf8');
+var sourceFile = /
[GitHub] cordova-browser pull request: CB-7978 Fixed launching chrome in Li...
Github user mmocny commented on a diff in the pull request:
https://github.com/apache/cordova-browser/pull/4#discussion_r20823313
--- Diff: bin/templates/project/cordova/run ---
@@ -20,10 +20,33 @@
*/
var shell = require('shelljs'),
-spawn = require('child_process').spawn,
-project = 'file://' + shell.pwd() +
'/platforms/browser/www/index.html';
+ fs = require('fs');
+var configFile = shell.pwd() + '/config.xml';
--- End diff --
Consider using `path.join` in the future, but in this case, perhaps use
`path.resolve('config.xml')` instead.
---
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-browser pull request: CB-7978 Fixed launching chrome in Li...
Github user mmocny commented on a diff in the pull request:
https://github.com/apache/cordova-browser/pull/4#discussion_r20823525
--- Diff: bin/templates/project/cordova/run ---
@@ -20,10 +20,33 @@
*/
var shell = require('shelljs'),
-spawn = require('child_process').spawn,
-project = 'file://' + shell.pwd() +
'/platforms/browser/www/index.html';
+ fs = require('fs');
+var configFile = shell.pwd() + '/config.xml';
+var configXML = fs.readFileSync(configFile, 'utf8');
+var sourceFile = /
[GitHub] cordova-browser pull request: CB-7978 Fixed launching chrome in Li...
Github user mmocny commented on a diff in the pull request:
https://github.com/apache/cordova-browser/pull/4#discussion_r20823145
--- Diff: bin/templates/project/cordova/run ---
@@ -20,10 +20,33 @@
*/
var shell = require('shelljs'),
-spawn = require('child_process').spawn,
-project = 'file://' + shell.pwd() +
'/platforms/browser/www/index.html';
+ fs = require('fs');
+var configFile = shell.pwd() + '/config.xml';
+var configXML = fs.readFileSync(configFile, 'utf8');
+var sourceFile = /
[GitHub] cordova-lib pull request: Breaking src into submodules
Github user mmocny commented on the pull request: https://github.com/apache/cordova-lib/pull/62#issuecomment-61817080 Should we close this? Are we still hoping to make these changes? Did they land with a different set of patches? --- 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-coho pull request: CB-7774 capture updates from Hangout an...
Github user mmocny commented on a diff in the pull request: https://github.com/apache/cordova-coho/pull/53#discussion_r18836698 --- Diff: docs/versioning-and-release-strategy.md --- @@ -19,17 +19,100 @@ # --> -# Versioning and Release Strategy +# Versioning Strategy -## Versioning Strategies - 1. `SemVer` ([Semantic Version](http://www.semver.org)) - * Used by platforms, plugman, CLI, core plugins - * Is important when describing dependencies in a sane way (e.g. within plugin.xml files) +## Version Format -CLI exists in both lists because its version has the format: `CadVer-SemVer` - * E.g.: `3.0.0-0.5.1` +`SemVer` ([Semantic Version](http://www.semver.org)) will be used as the +version format for all components, including platforms, plugman, CLI, core +plugins. Doing so is important when describing dependencies in a sane way +(e.g. within plugin.xml files). Although the CLI previously used a +`CadVer-SemVer` format, it now uses a simple SemVer format. The `CadVer` format +is no longer used in any Cordova components. The plugins no longer have an `r` +prefix. -## Release Strategies +The semantics of `SemVer` should be followed, bumping the appropriate digit +based on the impact of the new content. + +## Branching and Tagging + +All components also follow the same branching and tagging strategy, including +plugins and tools. A `major.minor.X` release branch (i.e., "3.7.x") should be +created, and any fixes should be appended to that release branch. New content +should be on the master branch, and a new release branch created at release +time. When a release is performed, a release tag is added to the appropriate +branch (i.e., "3.8.0" tag is put on the "3.8.x" branch). + +## Version Behavior + +Plugin versions will all be separate and independent. So there may be a "1.2.0" +of the Device plugin, and a "3.4.5" of the Camera plugin at the same time. +The bumping of the version numbers of each plugin should be appropriate to the +new content added to that plugin. The `cordova plugin add` command will add +the most recent version of that plugin by default, though alternately the user +may manually specify an explicit version of that plugin to be installed (i.e., +`cordova plugin add org.apache.cordova.device@1.2.0`). Plugin docs should be +stored in each plugin repo, so that the docs are versioned with their source +code + +Platform versions will all be separate and independent. So there may be a +"3.7.0" of the iOS platform and a "4.0.0" of the Android platform at the same +time. The bumping of version numbers of each platform should be appropriate +to the new content being added to that platform. The `cordova platform add` +command will add a platform version specific to the CLI by default, though +alternatively the user may manually specify an explicit version of that +platform to be installed (i.e., `cordova platform add android@4.0.1`). +The CLI will hold the list of default versions for each platform +(i.e., platform version pinning). Platform docs should be stored in each +platform repo, so that the docs are versioned with their source code. + +Platforms will have an <engine> tag or equivalent, to specify when a +platform needs a newer version of the CLI. + +`cordova-js` versions should continue to be single-sourced, meaning that when +`cordova-js` is used by multiple components such as `cordova-lib` or +`cordova-android`, the `cordova-js` version number should not be manually +modified upon insertion to the consuming component, but instead should retain +its build-time value. This means that there may be different versions of +`cordova-js` in use across each Cordova component. + +`cordova-lib`, `plugman`, and CLI versions will all be separate. So there +may be a "0.25.3" version of `plugman` and a "1.3.2" version of `cordova-lib` +and a "3.8.0" version of the CLI at the same time. The bumping of version +numbers of each of the tool components should be appropriate to the new +content being added to that individual component. The exception to this +is that when a new platform is released, and the platform pin in the CLI +is correspondingly updated, the CLI receives a bump to its third digit, no +matter the size of the version bump to those platform(s). + +When a user updates the version of the CLI they have installed, this CLI +update has no effect on the platform and plugin versions that are already +installed in their project, but they may receive a warning or notice if +the installed platform versions are older than the ve
[GitHub] cordova-coho pull request: CB-7774 capture updates from Hangout an...
Github user mmocny commented on a diff in the pull request: https://github.com/apache/cordova-coho/pull/53#discussion_r18836492 --- Diff: docs/versioning-and-release-strategy.md --- @@ -19,17 +19,100 @@ # --> -# Versioning and Release Strategy +# Versioning Strategy -## Versioning Strategies - 1. `SemVer` ([Semantic Version](http://www.semver.org)) - * Used by platforms, plugman, CLI, core plugins - * Is important when describing dependencies in a sane way (e.g. within plugin.xml files) +## Version Format -CLI exists in both lists because its version has the format: `CadVer-SemVer` - * E.g.: `3.0.0-0.5.1` +`SemVer` ([Semantic Version](http://www.semver.org)) will be used as the +version format for all components, including platforms, plugman, CLI, core +plugins. Doing so is important when describing dependencies in a sane way +(e.g. within plugin.xml files). Although the CLI previously used a +`CadVer-SemVer` format, it now uses a simple SemVer format. The `CadVer` format +is no longer used in any Cordova components. The plugins no longer have an `r` +prefix. -## Release Strategies +The semantics of `SemVer` should be followed, bumping the appropriate digit +based on the impact of the new content. + +## Branching and Tagging + +All components also follow the same branching and tagging strategy, including +plugins and tools. A `major.minor.X` release branch (i.e., "3.7.x") should be +created, and any fixes should be appended to that release branch. New content +should be on the master branch, and a new release branch created at release +time. When a release is performed, a release tag is added to the appropriate +branch (i.e., "3.8.0" tag is put on the "3.8.x" branch). + +## Version Behavior + +Plugin versions will all be separate and independent. So there may be a "1.2.0" +of the Device plugin, and a "3.4.5" of the Camera plugin at the same time. +The bumping of the version numbers of each plugin should be appropriate to the +new content added to that plugin. The `cordova plugin add` command will add +the most recent version of that plugin by default, though alternately the user +may manually specify an explicit version of that plugin to be installed (i.e., +`cordova plugin add org.apache.cordova.device@1.2.0`). Plugin docs should be +stored in each plugin repo, so that the docs are versioned with their source +code + +Platform versions will all be separate and independent. So there may be a +"3.7.0" of the iOS platform and a "4.0.0" of the Android platform at the same +time. The bumping of version numbers of each platform should be appropriate +to the new content being added to that platform. The `cordova platform add` +command will add a platform version specific to the CLI by default, though +alternatively the user may manually specify an explicit version of that +platform to be installed (i.e., `cordova platform add android@4.0.1`). +The CLI will hold the list of default versions for each platform +(i.e., platform version pinning). Platform docs should be stored in each +platform repo, so that the docs are versioned with their source code. + +Platforms will have an <engine> tag or equivalent, to specify when a +platform needs a newer version of the CLI. + +`cordova-js` versions should continue to be single-sourced, meaning that when +`cordova-js` is used by multiple components such as `cordova-lib` or +`cordova-android`, the `cordova-js` version number should not be manually +modified upon insertion to the consuming component, but instead should retain +its build-time value. This means that there may be different versions of +`cordova-js` in use across each Cordova component. + +`cordova-lib`, `plugman`, and CLI versions will all be separate. So there +may be a "0.25.3" version of `plugman` and a "1.3.2" version of `cordova-lib` +and a "3.8.0" version of the CLI at the same time. The bumping of version +numbers of each of the tool components should be appropriate to the new +content being added to that individual component. The exception to this +is that when a new platform is released, and the platform pin in the CLI +is correspondingly updated, the CLI receives a bump to its third digit, no +matter the size of the version bump to those platform(s). --- End diff -- With the caveat, that we still semver CLI appropriately. The "bar" here is: will this new CLI add features or break compatibility with existing plugins/platforms/projects. If not, its just a point release. However, updates to the pinned version may ofte
[GitHub] cordova-coho pull request: CB-7774 capture updates from Hangout an...
Github user mmocny commented on a diff in the pull request: https://github.com/apache/cordova-coho/pull/53#discussion_r18835133 --- Diff: docs/versioning-and-release-strategy.md --- @@ -19,17 +19,100 @@ # --> -# Versioning and Release Strategy +# Versioning Strategy -## Versioning Strategies - 1. `SemVer` ([Semantic Version](http://www.semver.org)) - * Used by platforms, plugman, CLI, core plugins - * Is important when describing dependencies in a sane way (e.g. within plugin.xml files) +## Version Format -CLI exists in both lists because its version has the format: `CadVer-SemVer` - * E.g.: `3.0.0-0.5.1` +`SemVer` ([Semantic Version](http://www.semver.org)) will be used as the +version format for all components, including platforms, plugman, CLI, core +plugins. Doing so is important when describing dependencies in a sane way +(e.g. within plugin.xml files). Although the CLI previously used a +`CadVer-SemVer` format, it now uses a simple SemVer format. The `CadVer` format +is no longer used in any Cordova components. The plugins no longer have an `r` +prefix. -## Release Strategies +The semantics of `SemVer` should be followed, bumping the appropriate digit +based on the impact of the new content. + +## Branching and Tagging + +All components also follow the same branching and tagging strategy, including +plugins and tools. A `major.minor.X` release branch (i.e., "3.7.x") should be +created, and any fixes should be appended to that release branch. New content +should be on the master branch, and a new release branch created at release +time. When a release is performed, a release tag is added to the appropriate +branch (i.e., "3.8.0" tag is put on the "3.8.x" branch). + +## Version Behavior + +Plugin versions will all be separate and independent. So there may be a "1.2.0" +of the Device plugin, and a "3.4.5" of the Camera plugin at the same time. +The bumping of the version numbers of each plugin should be appropriate to the +new content added to that plugin. The `cordova plugin add` command will add +the most recent version of that plugin by default, though alternately the user --- End diff -- This is correct as of today, but we would prefer to amend to say "most recent *compatible* version". I know you are capturing today's behaviour here, but just calling this out. --- 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-7707] Added basic multipart plug...
Github user mmocny commented on a diff in the pull request:
https://github.com/apache/cordova-android/pull/125#discussion_r18400602
--- Diff: framework/src/org/apache/cordova/PluginResult.java ---
@@ -80,6 +83,12 @@ public PluginResult(Status status, byte[] data, boolean
binaryString) {
this.encodedMessage = Base64.encodeToString(data, Base64.NO_WRAP);
}
+public PluginResult(Status status, List
multipartMessages) {
--- End diff --
You mean, of the nested PluginResults? Also status will be ignored.
---
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-browser pull request: No longer need to kill Chrome to run
Github user mmocny commented on the pull request: https://github.com/apache/cordova-browser/pull/1#issuecomment-56838479 ..only tested on Mac. --- 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. ---
[GitHub] cordova-browser pull request: No longer need to kill Chrome to run
GitHub user mmocny opened a pull request: https://github.com/apache/cordova-browser/pull/1 No longer need to kill Chrome to run You can merge this pull request into a Git repository by running: $ git pull https://github.com/mmocny/cordova-browser update_chrome_run Alternatively you can review and apply these changes as the patch at: https://github.com/apache/cordova-browser/pull/1.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 #1 commit 17d5851f2a5f5a3697b342c5db20a24470ff4071 Author: Michal Mocny Date: 2014-09-25T15:40:47Z No longer need to kill Chrome --- 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. ---
[GitHub] cordova-plugin-contacts pull request: Add missing test, skip some ...
Github user mmocny commented on the pull request: https://github.com/apache/cordova-plugin-contacts/pull/41#issuecomment-52351394 Looks good, but I didn't test on windows. Additionally, we should write define/it helpers that accept a list of platforms to include/exclude so that we don't have to handle this inline. --- 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. ---
[GitHub] cordova-plugin-geolocation pull request: CB-7146 ported geolocatio...
Github user mmocny commented on the pull request: https://github.com/apache/cordova-plugin-geolocation/pull/19#issuecomment-52341636 Wait, this was merged, it just didn't close itself, like https://github.com/apache/cordova-plugin-device-motion/pull/16 --- 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. ---
[GitHub] cordova-plugin-geolocation pull request: CB-7146 ported geolocatio...
Github user mmocny commented on the pull request: https://github.com/apache/cordova-plugin-geolocation/pull/19#issuecomment-52340827 Was this PR not merged for a reason? Perhaps because it was missing a subtask from https://issues.apache.org/jira/browse/CB-7086 ? I've modified it to be a subtask and am testing locally now. --- 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. ---
[GitHub] cordova-lib pull request: CB-6767 Allow `cordova` to be replaceabl...
Github user mmocny commented on the pull request: https://github.com/apache/cordova-lib/pull/20#issuecomment-44489272 I haven't tested yet, but.. I like it! @kamrik to also take a look, please. --- 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. ---
[GitHub] cordova-cli pull request: CB-6469 - Restore plugins from config.xm...
Github user mmocny commented on the pull request: https://github.com/apache/cordova-cli/pull/176#issuecomment-43118417 May you please wrap this with a check for `--experimental` flag? I think this is a useful feature, but I'm not convinced its in a form that we want to keep forever. --- 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. ---
[GitHub] cordova-cli pull request: CB-5833: Copy/link to custom merges and ...
Github user mmocny commented on the pull request: https://github.com/apache/cordova-cli/pull/169#issuecomment-41454574 Looks great, lets ship it. --- 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. ---
[GitHub] cordova-cli pull request: android-parser: Add launch-mode preferen...
Github user mmocny commented on the pull request: https://github.com/apache/cordova-cli/pull/164#issuecomment-41329099 Thanks! Pulled in (will take a few minutes for this PR to auto-close as apache mirrors repo). --- 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. ---
[GitHub] cordova-cli pull request: android-parser: Add launch-mode preferen...
Github user mmocny commented on the pull request: https://github.com/apache/cordova-cli/pull/164#issuecomment-41327034 Shall I merge as is, or do you want to fix the ConfigParser? --- 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. ---
[GitHub] cordova-cli pull request: android-parser: Add launch-mode preferen...
Github user mmocny commented on the pull request: https://github.com/apache/cordova-cli/pull/164#issuecomment-41326953 Thats simply an implicit API description that calls to `getPreference` must pass in lowercase names ;) (Okay, so its probably just a bug). --- 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. ---
[GitHub] cordova-coho pull request: Update processing-pull-requests.md
GitHub user mmocny opened a pull request: https://github.com/apache/cordova-coho/pull/19 Update processing-pull-requests.md Mention that coho list-pulls also provides commands to run You can merge this pull request into a Git repository by running: $ git pull https://github.com/mmocny/cordova-coho patch-1 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/cordova-coho/pull/19.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 #19 commit 6dd576d3a5965386b2fafb9778dcf1ae3ab5769e Author: Michal Mocny Date: 2014-04-24T20:12:49Z Update processing-pull-requests.md Mention that coho list-pulls also provides commands to run --- 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. ---
[GitHub] cordova-cli pull request: android-parser: Add launch-mode preferen...
Github user mmocny commented on the pull request: https://github.com/apache/cordova-cli/pull/164#issuecomment-41321690 Ah, I forgot, we will also need to update the docs (I can look at that if you prefer not to). --- 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. ---
[GitHub] cordova-cli pull request: android-parser: Add launch-mode preferen...
Github user mmocny commented on the pull request: https://github.com/apache/cordova-cli/pull/164#issuecomment-41321647 The casing is actually insensitive, but the other preferences don't have dashes, i.e. "WebViewBounce" --- 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. ---
[GitHub] cordova-cli pull request: android-parser: Add launch-mode preferen...
Github user mmocny commented on the pull request: https://github.com/apache/cordova-cli/pull/164#issuecomment-41320706 Awesome. One last tiny request: may we change the name of the preference to "AndroidLaunchMode"? --- 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. ---
[GitHub] cordova-cli pull request: android-parser: Add launch-mode preferen...
Github user mmocny commented on the pull request: https://github.com/apache/cordova-cli/pull/164#issuecomment-41319318 Looks great. Will need to remember to update android's defaults.xml. --- 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. ---
[GitHub] cordova-cli pull request: android-parser: Add launch-mode preferen...
Github user mmocny commented on a diff in the pull request:
https://github.com/apache/cordova-cli/pull/164#discussion_r11925400
--- Diff: src/metadata/android_parser.js ---
@@ -97,6 +108,29 @@ module.exports.prototype = {
}
}
+// Set launchMode in AndroidManifest
+var launchModePref = this.findLaunchModePreference(config);
+if (launchModePref) {
+var act = manifest.getroot().find('./application/activity');
+switch (launchModePref) {
+case 'standard':
+act.attrib["android:launchMode"] = 'standard';
+break;
+case 'singleTop':
+act.attrib["android:launchMode"] = 'singleTop';
+break;
+case 'singleTask':
+act.attrib["android:launchMode"] = 'singleTask';
+break;
+case 'singleInstance':
+act.attrib["android:launchMode"] = 'singleInstance';
+break;
+case 'default':
+delete act.attrib["android:launchMode"];
--- End diff --
I think the way it is is fine: if you explicitly set an invalid value for
the preference in your application config, you get the default *android*
application launchMode. If you don't set any preference, you get the default
*cordova* application launchMode. That makes sense to me!
---
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.
---
[GitHub] cordova-cli pull request: android-parser: Add launch-mode preferen...
Github user mmocny commented on a diff in the pull request:
https://github.com/apache/cordova-cli/pull/164#discussion_r11924705
--- Diff: src/metadata/android_parser.js ---
@@ -97,6 +108,29 @@ module.exports.prototype = {
}
}
+// Set launchMode in AndroidManifest
+var launchModePref = this.findLaunchModePreference(config);
+if (launchModePref) {
+var act = manifest.getroot().find('./application/activity');
+switch (launchModePref) {
+case 'standard':
+act.attrib["android:launchMode"] = 'standard';
+break;
+case 'singleTop':
+act.attrib["android:launchMode"] = 'singleTop';
+break;
+case 'singleTask':
+act.attrib["android:launchMode"] = 'singleTask';
+break;
+case 'singleInstance':
+act.attrib["android:launchMode"] = 'singleInstance';
+break;
+case 'default':
+delete act.attrib["android:launchMode"];
--- End diff --
As of this change:
https://git-wip-us.apache.org/repos/asf?p=cordova-android.git;h=298cd9e there
is a new default value for android projects for launchMode (singleTop).
If we make this change, we are reverting to a default launchMode of
standard, which I do not think is a great default for cordova apps. We can
solve the problem by changing the parser, or by setting the default launchMode
in defaults.xml. I'm happy with the second option!
---
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.
---
[GitHub] cordova-cli pull request: android-parser: Add launch-mode preferen...
Github user mmocny commented on a diff in the pull request:
https://github.com/apache/cordova-cli/pull/164#discussion_r11911345
--- Diff: src/metadata/android_parser.js ---
@@ -97,6 +108,29 @@ module.exports.prototype = {
}
}
+// Set launchMode in AndroidManifest
+var launchModePref = this.findLaunchModePreference(config);
+if (launchModePref) {
+var act = manifest.getroot().find('./application/activity');
+switch (launchModePref) {
--- End diff --
Why this switch statement? Couldn't you just `if (launchModePref)
act.attrib["android:launchMode"] = launchModePref;` ?
---
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.
---
[GitHub] cordova-cli pull request: android-parser: Add launch-mode preferen...
Github user mmocny commented on a diff in the pull request:
https://github.com/apache/cordova-cli/pull/164#discussion_r11911288
--- Diff: src/metadata/android_parser.js ---
@@ -97,6 +108,29 @@ module.exports.prototype = {
}
}
+// Set launchMode in AndroidManifest
+var launchModePref = this.findLaunchModePreference(config);
+if (launchModePref) {
+var act = manifest.getroot().find('./application/activity');
+switch (launchModePref) {
+case 'standard':
+act.attrib["android:launchMode"] = 'standard';
+break;
+case 'singleTop':
+act.attrib["android:launchMode"] = 'singleTop';
+break;
+case 'singleTask':
+act.attrib["android:launchMode"] = 'singleTask';
+break;
+case 'singleInstance':
+act.attrib["android:launchMode"] = 'singleInstance';
+break;
+case 'default':
--- End diff --
default isn't a valid mode (according to findLaunchModePreference)
---
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.
---
[GitHub] cordova-cli pull request: android-parser: Add launch-mode preferen...
Github user mmocny commented on a diff in the pull request:
https://github.com/apache/cordova-cli/pull/164#discussion_r11911266
--- Diff: src/metadata/android_parser.js ---
@@ -97,6 +108,29 @@ module.exports.prototype = {
}
}
+// Set launchMode in AndroidManifest
+var launchModePref = this.findLaunchModePreference(config);
+if (launchModePref) {
+var act = manifest.getroot().find('./application/activity');
+switch (launchModePref) {
+case 'standard':
+act.attrib["android:launchMode"] = 'standard';
+break;
+case 'singleTop':
+act.attrib["android:launchMode"] = 'singleTop';
+break;
+case 'singleTask':
+act.attrib["android:launchMode"] = 'singleTask';
+break;
+case 'singleInstance':
+act.attrib["android:launchMode"] = 'singleInstance';
+break;
+case 'default':
+delete act.attrib["android:launchMode"];
--- End diff --
This would delete the default from our application template, right? I
think we would like to set a sensible default (thats the bug we fixed in the
first place).
However, an alternative way would be to leave this patch as-is and instead
update defaults.xml to include a default preference for this. Thoughts?
---
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.
---
[GitHub] cordova-cli pull request: CB-5833: Copy/link to custom merges and ...
Github user mmocny commented on a diff in the pull request:
https://github.com/apache/cordova-cli/pull/169#discussion_r11900183
--- Diff: src/create.js ---
@@ -146,11 +170,20 @@ module.exports = function create (dir, id, name, cfg)
{
} else {
shell.mkdir(www_dir);
shell.cp('-rf', path.join(www_lib, '*'), www_dir);
+if (custom_merges) {
+shell.cp('-rf', path.join(custom_merges, '*'),
path.join(dir, 'merges'));
--- End diff --
Why do we pre-create www_dir and not merges?
---
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.
---
[GitHub] cordova-cli pull request: CB-5833: Copy/link to custom merges and ...
Github user mmocny commented on a diff in the pull request:
https://github.com/apache/cordova-cli/pull/169#discussion_r1194
--- Diff: src/create.js ---
@@ -124,9 +129,22 @@ module.exports = function create (dir, id, name, cfg) {
}
// Keep going into child "www" folder if exists in stock app
package.
while (fs.existsSync(path.join(www_lib, 'www'))) {
+www_parent_dir = www_lib;
www_lib = path.join(www_lib, 'www');
}
+// Find if we also have custom merges and config.xml as siblings
of custom www.
+if (www_parent_dir && config_json.lib && config_json.lib.www) {
+custom_config_xml = path.join(www_parent_dir, 'config.xml');
+if ( !fs.existsSync(custom_config_xml) ) {
+custom_config_xml = '';
--- End diff --
I prefer null to empty string for these failure cases. Empty string tests
false in if statements, but is a valid input for others.
---
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.
---
