[GitHub] cordova-android pull request: CB-9149: Make gradle alias subprojec...

2015-06-11 Thread agrieve
Github user agrieve commented on a diff in the pull request:

https://github.com/apache/cordova-android/pull/182#discussion_r32287653
  
--- Diff: bin/templates/cordova/lib/build.js ---
@@ -245,18 +258,29 @@ var builders = {
 }
 }
 
+var name=extractRealProjectNameFromManifest(ROOT);
 var subProjectsAsGradlePaths = subProjects.map(function(p) { 
return ':' + p.replace(/[/\\]/g, ':'); });
+//Remove the proj.id/name- prefix from projects
+var settingsGradlePaths =  subProjects.map(function(p){
+var realDir=p.replace(/[/\\]/g, ':');
+var libName=realDir.replace(name+'-','');
+var str='include :'+libName+'\n';
+if(realDir.indexOf(name+'-')!==-1)
+str+='project(:'+libName+').projectDir = new 
File('+p+')\n';
+return str;
+});
+
 // Write the settings.gradle file.
 fs.writeFileSync(path.join(projectPath, 'settings.gradle'),
 '// GENERATED FILE - DO NOT EDIT\n' +
-'include :\n' +
-'include ' + subProjectsAsGradlePaths.join('\ninclude 
') + '\n');
+'include :\n' + settingsGradlePaths.join(''));
 // Update dependencies within build.gradle.
 var buildGradle = fs.readFileSync(path.join(projectPath, 
'build.gradle'), 'utf8');
 var depsList = '';
 subProjectsAsGradlePaths.forEach(function(p) {
--- End diff --

nit: delete subProjectsAsGradlePaths, and just loop on subProjects 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-9149: Make gradle alias subprojec...

2015-06-11 Thread agrieve
Github user agrieve commented on a diff in the pull request:

https://github.com/apache/cordova-android/pull/182#discussion_r32287669
  
--- Diff: bin/templates/cordova/lib/build.js ---
@@ -245,18 +258,29 @@ var builders = {
 }
 }
 
+var name=extractRealProjectNameFromManifest(ROOT);
 var subProjectsAsGradlePaths = subProjects.map(function(p) { 
return ':' + p.replace(/[/\\]/g, ':'); });
+//Remove the proj.id/name- prefix from projects
--- End diff --

nit: add a link to the JIRA here since it's not obvious why we're doing 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.
---

-
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-9149: Make gradle alias subprojec...

2015-06-11 Thread agrieve
Github user agrieve commented on a diff in the pull request:

https://github.com/apache/cordova-android/pull/182#discussion_r3228
  
--- Diff: bin/templates/cordova/lib/build.js ---
@@ -102,6 +102,19 @@ function hasCustomRules() {
 return fs.existsSync(path.join(ROOT, 'custom_rules.xml'));
 }
 
+function extractRealProjectNameFromManifest(projectPath) {
+var manifestPath = path.join(projectPath, 'AndroidManifest.xml');
+var manifestData = fs.readFileSync(manifestPath, 'utf8');
+var m = /manifest[\s\S]*?package*=\s*(.*?)/i.exec(manifestData);
--- End diff --

missed a `\s` after `package`


---
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-9149: Make gradle alias subprojec...

2015-06-11 Thread agrieve
Github user agrieve commented on the pull request:

https://github.com/apache/cordova-android/pull/182#issuecomment-111340031
  
a few nits, but LGTM! Thanks for doing 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-plugin-splashscreen pull request: CB-8396 - Read property ...

2015-05-28 Thread agrieve
Github user agrieve commented on a diff in the pull request:


https://github.com/apache/cordova-plugin-splashscreen/pull/48#discussion_r31235221
  
--- Diff: src/android/SplashScreen.java ---
@@ -91,7 +91,9 @@ protected void pluginInitialize() {
 
 firstShow = false;
 loadSpinner();
-showSplashScreen(true);
+
+boolean hideAfterDelay = 
preferences.getBoolean(AutoHideSplashScreen, true);
--- End diff --

The AutoHide preference is meant to hide without the use of a timer (e.g. 
hide on first paint after onload). Would be happy to accept a PR for this, but 
I don't think we should have a PR that just turns it off. For this, set a very 
large timeout.


---
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-9033 : fix WatchKit support

2015-05-20 Thread agrieve
Github user agrieve commented on the pull request:

https://github.com/apache/cordova-lib/pull/219#issuecomment-103903180
  
Looks like this pr breaks tests. Could you please address? (run npm test)


---
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: Add command to merge PR

2015-05-20 Thread agrieve
Github user agrieve commented on a diff in the pull request:

https://github.com/apache/cordova-coho/pull/78#discussion_r30717099
  
--- Diff: src/executil.js ---
@@ -52,7 +52,7 @@ function execHelper(cmdAndArgs, silent, allowError) {
 var result = superspawn.spawn(cmdAndArgs[0], cmdAndArgs.slice(1), 
{stdio: (silent  (silent !== 2)) ? 'default' : 'inherit'});
 return result.then(null, function(e) {
 if (allowError) {
-return null;
+throw e;
--- End diff --

This is much nicer :+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-coho pull request: Add command to merge PR

2015-05-20 Thread agrieve
Github user agrieve commented on the pull request:

https://github.com/apache/cordova-coho/pull/78#issuecomment-103934974
  
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-coho pull request: Add command to merge PR

2015-05-19 Thread agrieve
Github user agrieve commented on a diff in the pull request:

https://github.com/apache/cordova-coho/pull/78#discussion_r30661261
  
--- Diff: src/merge-pr.js ---
@@ -0,0 +1,62 @@
+
+var flagutil = require('./flagutil');
+var optimist = require('optimist');
+var shelljs = require('shelljs');
+var executil = require('./executil');
+var gitutil = require('./gitutil');
+var superspawn = require('./superspawn');
+
+module.exports = function *(argv) {
+var opt = flagutil.registerHelpFlag(optimist);
+opt.options('pr', {
+desc: 'PR # that needs to be merged',
+demand: true
+}).options('remote', {
--- End diff --

would be much slicker if `remote` was figured out automatically. I *think* 
this info is already in list-pulls.js, so could maybe get it from there?


---
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: Add command to merge PR

2015-05-19 Thread agrieve
Github user agrieve commented on the pull request:

https://github.com/apache/cordova-coho/pull/78#issuecomment-103703256
  
Awesome addition!


---
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: Add command to merge PR

2015-05-19 Thread agrieve
Github user agrieve commented on a diff in the pull request:

https://github.com/apache/cordova-coho/pull/78#discussion_r30661140
  
--- Diff: src/merge-pr.js ---
@@ -0,0 +1,62 @@
+
--- End diff --

missing header


---
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: Add command to merge PR

2015-05-19 Thread agrieve
Github user agrieve commented on a diff in the pull request:

https://github.com/apache/cordova-coho/pull/78#discussion_r30661704
  
--- Diff: src/merge-pr.js ---
@@ -0,0 +1,62 @@
+
+var flagutil = require('./flagutil');
+var optimist = require('optimist');
+var shelljs = require('shelljs');
+var executil = require('./executil');
+var gitutil = require('./gitutil');
+var superspawn = require('./superspawn');
+
+module.exports = function *(argv) {
+var opt = flagutil.registerHelpFlag(optimist);
+opt.options('pr', {
+desc: 'PR # that needs to be merged',
+demand: true
+}).options('remote', {
+   desc: 'Named remote for the github apache mirror',
+   demand: true  
+}).options('pretend', {
+desc: 'Don\'t actually run commands, just print what would be 
run.',
+type:'boolean'
+});
+argv = opt
+.usage('Merges the pull request to master\n' +
+'\n' +
+'Usage: $0 merge-pr --pr 111 --remote mirror')
+.argv;
+   if (argv.h) {
+optimist.showHelp();
+process.exit(1);
+   }
+   yield gitutil.stashAndPop(, function*() {
+   yield executil.execOrPretend(executil.ARGS('git checkout master'), 
argv.pretend);
+
+   yield executil.execOrPretend(executil.ARGS('git pull origin 
master'), argv.pretend);
+   
+   yield executil.execOrPretend(['git', 'fetch', '-fu', argv.remote,
+'refs/pull/' + argv.pr + '/head:pr/' + argv.pr], argv.pretend);
+
+   try {
+yield executil.execHelper(executil.ARGS('git merge --ff-only 
pr/' + argv.pr),
+/*silent*/ true, /*allowError*/ true, argv.pretend);
+   } catch (e) {   
+   if (e.message.indexOf('fatal: Not possible to fast-forward, 
aborting.')  0) {
+   // Let's try to rebase
+   yield executil.execOrPretend(executil.ARGS('git checkout 
pr/' + argv.pr), argv.pretend);
+   
+   yield executil.execOrPretend(executil.ARGS('git pull 
--rebase origin master'), argv.pretend);
+   
+   yield executil.execOrPretend(executil.ARGS('git checkout 
master'), argv.pretend);
+   
+   yield executil.execOrPretend(executil.ARGS('git merge 
--ff-only pr/' + argv.pr), argv.pretend);
+   
+   var commitMessage = yield 
executil.execOrPretend(executil.ARGS('git log --format=%B -n 1 HEAD'), 
+argv.pretend);
+   
+   yield executil.execOrPretend(['git', 'commit', '--amend', 
'-m',  
--- End diff --

For the case of multiple commits, it would also be a good idea to end with 
a git rebase -i so commits  commit messages can be squashed.


---
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-3360 Set custom User-Agent

2015-05-06 Thread agrieve
Github user agrieve commented on the pull request:

https://github.com/apache/cordova-android/pull/162#issuecomment-99489261
  
(and 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-android pull request: CB-3360 Set custom User-Agent

2015-05-06 Thread agrieve
Github user agrieve commented on the pull request:

https://github.com/apache/cordova-android/pull/162#issuecomment-99489147
  
Merged. would be great if you could do another PR to update the docs:

https://github.com/apache/cordova-docs/blob/master/docs/en/edge/guide/platforms/android/config.md


---
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: Remove hardcoded reference to registry.n...

2015-05-05 Thread agrieve
Github user agrieve commented on the pull request:

https://github.com/apache/cordova-lib/pull/211#issuecomment-99094530
  
Thanks Matt!
JIRA for this: https://issues.apache.org/jira/browse/CB-8956


---
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-8757 - Resolve symlinks in order to a...

2015-05-05 Thread agrieve
Github user agrieve commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/212#discussion_r29671237
  
--- Diff: cordova-lib/src/plugman/plugman.js ---
@@ -89,9 +90,12 @@ plugman.commands =  {
 plugman.owner(cli_opts.argv.remain);
 },
 'install'  : function(cli_opts) {
+resolvePathsInArgs(cli_opts);
--- End diff --

CLI code doesn't pass through here either I don't think. Within 
`install.js` is where you should do it I think.


---
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-8757 - Resolve symlinks in order to a...

2015-05-05 Thread agrieve
Github user agrieve commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/212#discussion_r29683101
  
--- Diff: cordova-lib/src/plugman/plugman.js ---
@@ -228,3 +228,4 @@ plugman.commands =  {
 };
 
 module.exports = plugman;
+
--- End diff --

looks good. just revert this unchanged file and fix the one failing test.


---
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-plugman pull request: CB-8757 - Resolve any symlinks in or...

2015-05-04 Thread agrieve
Github user agrieve commented on a diff in the pull request:

https://github.com/apache/cordova-plugman/pull/83#discussion_r29583217
  
--- Diff: main.js ---
@@ -60,6 +61,9 @@ var cli_opts = nopt(known_opts, shortHands);
 
 var cmd = cli_opts.argv.remain.shift();
 
+cli_opts.project = convertToRealPathSafe(cli_opts.project);
--- End diff --

Making them absolute here won't help when using cordova-lib via `cordova` 
or via `cordova-lib` directly. Could you look and see if you can resolve them 
outside of main.js?


---
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-8902: use immersive mode when ava...

2015-04-24 Thread agrieve
Github user agrieve commented on a diff in the pull request:

https://github.com/apache/cordova-android/pull/175#discussion_r29046326
  
--- Diff: framework/src/org/apache/cordova/CordovaActivity.java ---
@@ -130,6 +147,18 @@ public void onCreate(Bundle savedInstanceState) {
 cordovaInterface.restoreInstanceState(savedInstanceState);
 }
 }
+
+private void setSystemUiVisibilityMode(Window window) {
+final int uiOptions =
+View.SYSTEM_UI_FLAG_LAYOUT_STABLE
--- End diff --

Are these LAYOUT flags really necessary? (if so, add a comment explaining 
why)


---
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-8902: use immersive mode when ava...

2015-04-24 Thread agrieve
Github user agrieve commented on a diff in the pull request:

https://github.com/apache/cordova-android/pull/175#discussion_r29046696
  
--- Diff: framework/src/org/apache/cordova/CordovaActivity.java ---
@@ -130,6 +147,18 @@ public void onCreate(Bundle savedInstanceState) {
 cordovaInterface.restoreInstanceState(savedInstanceState);
 }
 }
+
+private void setSystemUiVisibilityMode(Window window) {
--- End diff --

If you're going to pass in the window, then you should make this method 
static.
If you're going to call it set...Mode, then it should accept the mode as 
a parameter.
Really though, hoping you don't need the listener so that you don't need a 
helper at all.


---
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-8902: use immersive mode when ava...

2015-04-24 Thread agrieve
Github user agrieve commented on a diff in the pull request:

https://github.com/apache/cordova-android/pull/175#discussion_r29046647
  
--- Diff: framework/src/org/apache/cordova/CordovaActivity.java ---
@@ -115,8 +116,24 @@ public void onCreate(Bundle savedInstanceState) {
 
getWindow().setFlags(WindowManager.LayoutParams.FLAG_FULLSCREEN,
--- End diff --

SetFullscreen is deprecated, but we shouldn't make it behave differently 
from Fullscreen. How about changing this to:

if (SetFullscreen) {
  Log();
  preferences.set(Fullscreen, true);
}
if (Fullscreen) {
  ...
}


---
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-splashscreen pull request: Add fullscreen options

2015-04-24 Thread agrieve
Github user agrieve commented on a diff in the pull request:


https://github.com/apache/cordova-plugin-splashscreen/pull/45#discussion_r29045852
  
--- Diff: README.md ---
@@ -54,12 +54,18 @@ In your `config.xml`, you need to add the following 
preferences:
 preference name=SplashScreen value=foo /
 preference name=SplashScreenDelay value=1 /
 preference name=SplashMaintainAspectRatio value=true|false /
+   preference name=SplashHideNavigationBar value=true|false /
--- End diff --

Do these do the same thing as Fullscreen and ShowTitle prefs?
If so, can we make the default values for these be the values of 
Fullscreen and ShowTitle?



---
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-8902: use immersive mode when ava...

2015-04-24 Thread agrieve
Github user agrieve commented on a diff in the pull request:

https://github.com/apache/cordova-android/pull/175#discussion_r29046516
  
--- Diff: framework/src/org/apache/cordova/CordovaActivity.java ---
@@ -115,8 +116,24 @@ public void onCreate(Bundle savedInstanceState) {
 
getWindow().setFlags(WindowManager.LayoutParams.FLAG_FULLSCREEN,
 WindowManager.LayoutParams.FLAG_FULLSCREEN);
 } else if (preferences.getBoolean(Fullscreen, false)) {
-
getWindow().setFlags(WindowManager.LayoutParams.FLAG_FULLSCREEN,
+
+if (Build.VERSION.SDK_INT = Build.VERSION_CODES.KITKAT)
+{
+setSystemUiVisibilityMode(getWindow());
+
getWindow().getDecorView().setOnSystemUiVisibilityChangeListener(new 
View.OnSystemUiVisibilityChangeListener() {
--- End diff --

I don't think this listener should be necessary. According to:
https://developer.android.com/training/system-ui/immersive.html

setting IMMERSIVE_STICKY should take care of re-entering fullscreen.


---
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-splashscreen pull request: Add fullscreen options

2015-04-24 Thread agrieve
Github user agrieve commented on a diff in the pull request:


https://github.com/apache/cordova-plugin-splashscreen/pull/45#discussion_r29045956
  
--- Diff: src/android/SplashScreen.java ---
@@ -254,8 +262,12 @@ public void run() {
 }
 splashDialog.setContentView(splashImageView);
 splashDialog.setCancelable(false);
-splashDialog.show();
-
+if (isHideNavigationBar() || isHideStatusBar()) {
+   
splashDialog.getWindow().getDecorView().setSystemUiVisibility(
+   (isHideNavigationBar() ? 
View.SYSTEM_UI_FLAG_HIDE_NAVIGATION : 0) | (isHideNavigationBar() ? 
View.SYSTEM_UI_FLAG_FULLSCREEN : 0));
--- End diff --

The indenting on pretty much every line you changed looks off. Likely the 
problem is that your editor is inserting tabs instead of spaces.


---
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: solves CB-8804 https://github....

2015-04-22 Thread agrieve
Github user agrieve commented on a diff in the pull request:

https://github.com/apache/cordova-plugin-camera/pull/83#discussion_r28881787
  
--- Diff: src/android/CameraLauncher.java ---
@@ -163,7 +167,15 @@ else if ((srcType == PHOTOLIBRARY) || (srcType == 
SAVEDPHOTOALBUM)) {
 PluginResult r = new 
PluginResult(PluginResult.Status.NO_RESULT);
 r.setKeepCallback(true);
 callbackContext.sendPluginResult(r);
-
+
+return true;
+} else if (action.equals(checkForSavedResult)) {
+this.imageUri = Uri.fromFile(createCaptureFile(JPEG));
--- End diff --

should you do this only when savedRequestCode  0?


---
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: solves CB-8804 https://github....

2015-04-22 Thread agrieve
Github user agrieve commented on a diff in the pull request:

https://github.com/apache/cordova-plugin-camera/pull/83#discussion_r28882307
  
--- Diff: src/android/CameraLauncher.java ---
@@ -632,73 +645,79 @@ else if (destType == FILE_URI || destType == 
NATIVE_URI) {
  */
 public void onActivityResult(int requestCode, int resultCode, Intent 
intent) {
 
-// Get src and dest types from request code for a Camera Activity
-int srcType = (requestCode / 16) - 1;
-int destType = (requestCode % 16) - 1;
-
-// If Camera Crop
-if (requestCode = CROP_CAMERA) {
-if (resultCode == Activity.RESULT_OK) {
+if (this.imageUri == null  this.getPictureFromGallery == false) {
--- End diff --

Would it be more appropriate to check if `callbackContext == null`? An app 
can be evicted even when getting a picture from the gallery.


---
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: solves CB-8804 https://github....

2015-04-22 Thread agrieve
Github user agrieve commented on a diff in the pull request:

https://github.com/apache/cordova-plugin-camera/pull/83#discussion_r28882407
  
--- Diff: www/Camera.js ---
@@ -72,4 +72,8 @@ cameraExport.cleanup = function(successCallback, 
errorCallback) {
 exec(successCallback, errorCallback, Camera, cleanup, []);
 };
 
+cameraExport.checkForSavedResult = function(successCallback, 
errorCallback) {
--- End diff --

Please update the plugin README.md with docs on this new API


---
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: solves CB-8804 https://github....

2015-04-21 Thread agrieve
Github user agrieve commented on the pull request:


https://github.com/apache/cordova-plugin-camera/pull/83#issuecomment-94831626
  
just went to have a look but the diff has changes that you didn't make in 
it. Can you try rebasing?


---
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: fix package name validation

2015-04-15 Thread agrieve
Github user agrieve commented on the pull request:

https://github.com/apache/cordova-android/pull/174#issuecomment-93614422
  
@boynet - your fiddle doesn't use the regex that's checked in.
http://jsfiddle.net/hakxc2uv/

shows that the regexp as-is works fine for your case (I just copied and 
pasted it from the Files changed section of this 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-android pull request: fix CB-8684 - support onStart/onStop...

2015-04-08 Thread agrieve
Github user agrieve commented on a diff in the pull request:

https://github.com/apache/cordova-android/pull/173#discussion_r28009343
  
--- Diff: framework/src/org/apache/cordova/CordovaActivity.java ---
@@ -95,6 +95,9 @@ Licensed to the Apache Software Foundation (ASF) under one
 protected ArrayListPluginEntry pluginEntries;
 protected CordovaInterfaceImpl cordovaInterface;
 
+//flag for case when appView is null during onStart
+private boolean shouldHandleStartOnAppViewInit = false;
--- End diff --

The webview (and thus plugins) are generally created from onCreate(), which 
means they will be initialized before the first call to onStart. If you're unit 
tests passes without this flag (and without the one in CordovaWebViewImpl), 
then I think you should remove them. It's not obvious (to me anyways) that this 
behaviour would even be the desired behaviour. If there's a reason it needs to 
be this way, then let me know! But if so, then we should upated onResume as 
well so that we're consistent.


---
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-splashscreen pull request: CB-3679 Break Android sp...

2015-04-07 Thread agrieve
Github user agrieve closed the pull request at:

https://github.com/apache/cordova-plugin-splashscreen/pull/32


---
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-splashscreen pull request: CB-3679 Break Android sp...

2015-04-07 Thread agrieve
Github user agrieve commented on the pull request:


https://github.com/apache/cordova-plugin-splashscreen/pull/32#issuecomment-90591245
  
This was merged. Will be applicable for cordova-android@4.0.0


---
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: 3.7.x

2015-04-07 Thread agrieve
Github user agrieve commented on the pull request:

https://github.com/apache/cordova-android/pull/172#issuecomment-90602838
  
Was this PR opened by mistake? Missing a description.


---
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-8484 Add documentation for signing a...

2015-04-07 Thread agrieve
Github user agrieve commented on a diff in the pull request:

https://github.com/apache/cordova-docs/pull/277#discussion_r27888531
  
--- Diff: docs/en/edge/guide/platforms/android/tools.md ---
@@ -85,6 +85,43 @@ AVD is available as a target, you're prompted to select 
one. By
 default the `run` command detects a connected device, or a currently
 running emulator if no device is found.
 
+## Signing the App
+
+You can review Android app signing requirements here: 
http://developer.android.com/tools/publishing/app-signing.html
+
+To sign an app, you need the following parameters:
+  * Keystore (`--keystore`): Path to a binary file which can hold a set of 
keys.
+  * Keystore password (`--storePassword`): Password to the keystore
+  * Alias (`--alias`): The id specifying the private key used for singing.
+  * Password (`--password`): Passsword for the private key specified.
--- End diff --

Pasword :)


---
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-8484 Add documentation for signing a...

2015-04-07 Thread agrieve
Github user agrieve commented on a diff in the pull request:

https://github.com/apache/cordova-docs/pull/277#discussion_r27888508
  
--- Diff: docs/en/edge/guide/platforms/android/tools.md ---
@@ -85,6 +85,43 @@ AVD is available as a target, you're prompted to select 
one. By
 default the `run` command detects a connected device, or a currently
 running emulator if no device is found.
 
+## Signing the App
+
+You can review Android app signing requirements here: 
http://developer.android.com/tools/publishing/app-signing.html
+
+To sign an app, you need the following parameters:
+  * Keystore (`--keystore`): Path to a binary file which can hold a set of 
keys.
+  * Keystore password (`--storePassword`): Password to the keystore
+  * Alias (`--alias`): The id specifying the private key used for singing.
+  * Password (`--password`): Passsword for the private key specified.
+  * Type of the keystore (`--keystoreType`): pkcs12, jks (Default:pkcs12)
--- End diff --

believe the default is jks. When not specified, it'll switch to pkcs12 
automatically if the keystore path ends with .p12 or .pfx, so you normally 
wouldn't need to set this explicitly.


---
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-8500 Documentation update for signin...

2015-04-07 Thread agrieve
Github user agrieve commented on the pull request:

https://github.com/apache/cordova-docs/pull/267#issuecomment-90716706
  
Do we still want this change given that both gradle and ant-style are 
accepted? I'm thinking no since I find gradle-style makes more sense. If so, 
though, need to fix key.storeType - key.store.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-android pull request: CB-8484: Adds support creating signe...

2015-04-01 Thread agrieve
Github user agrieve commented on the pull request:

https://github.com/apache/cordova-android/pull/164#issuecomment-88668426
  
Merged!


---
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-8484: Adds support creating signe...

2015-04-01 Thread agrieve
Github user agrieve commented on a diff in the pull request:

https://github.com/apache/cordova-android/pull/164#discussion_r27591881
  
--- Diff: bin/templates/cordova/lib/build.js ---
@@ -422,11 +500,18 @@ function parseOpts(options, resolvedTarget) {
 module.exports.runClean = function(options) {
 var opts = parseOpts(options);
 var builder = builders[opts.buildMethod];
-return builder.prepEnv()
+return builder.prepEnv(opts)
 .then(function() {
-return builder.clean(opts.extraArgs);
+return builder.clean(opts);
 }).then(function() {
 shell.rm('-rf', path.join(ROOT, 'out'));
+
+['debug', 'release'].forEach(function(config) {
+var propertiesFilePath = path.join(ROOT, config + 
SIGNING_PROPERTIES);
+if(isAutoGenerated(propertiesFilePath)){
+shell.rm('-f', propertiesFilePath);
--- End diff --

I don't think clean is really used (the command isn't even wired up to CLI 
afaik, although it really should be). If passwords are the concern, then 
probably we should be deleting the files after building.


---
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-3360 Set custom User-Agent

2015-04-01 Thread agrieve
Github user agrieve commented on a diff in the pull request:

https://github.com/apache/cordova-android/pull/162#discussion_r27606969
  
--- Diff: framework/src/org/apache/cordova/engine/SystemWebViewEngine.java 
---
@@ -199,7 +199,19 @@ private void initWebViewSettings() {
 
 // Fix for CB-1405
 // Google issue 4641
-settings.getUserAgentString();
+String defaultUserAgent = settings.getUserAgentString();
+
+// Fix for CB-3360
+String overrideUserAgent = 
getCordovaWebView().getPreferences().getString(OverrideUserAgent, null);
--- End diff --

I'd like to avoid circular dependency if possible on CordovaWebView.

Rather than depending on CordovaWebView here, could you save the instance 
of CordovaPreferences passed to the constructor and use that instead? Would 
also need to add CordovaPreferences as a 2nd arg to the other constructor as 
well.


---
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: solves CB-8768 issue where onActivit...

2015-03-31 Thread agrieve
Github user agrieve commented on the pull request:

https://github.com/apache/cordova-android/pull/171#issuecomment-88189256
  
Awesome! Merged and made one minor change: Removed onCordovaInit from 
CordovaInterface (but left it in CordovaInterfaceImpl). If it were to be called 
from CordovaWebViewImpl, then it would belong in the interface, but not 
otherwise.


---
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-8484: Adds support creating signe...

2015-03-31 Thread agrieve
Github user agrieve commented on a diff in the pull request:

https://github.com/apache/cordova-android/pull/164#discussion_r27536261
  
--- Diff: bin/templates/cordova/lib/build.js ---
@@ -402,6 +440,16 @@ function parseOpts(options, resolvedTarget) {
 case 'gradleArg':
 ret.extraArgs.push(flagValue);
 break;
+case 'keystore':
+case 'alias':
+case 'keytorePassword':
+case 'password':
+case 'keystoreType':
+packageArgs[flagName] = flagValue;
+break;
+case 'buildConfig':
+buildConfig = flagValue.replace(//g, '');
--- End diff --

Quotes for these reasons are generally interpreted by the shell (cmd.exe), 
and when we see it here, they should be gone already (and flagValue will have a 
space in 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.
---

-
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-8484: Adds support creating signe...

2015-03-31 Thread agrieve
Github user agrieve commented on a diff in the pull request:

https://github.com/apache/cordova-android/pull/164#discussion_r27536475
  
--- Diff: bin/templates/cordova/lib/build.js ---
@@ -422,11 +500,18 @@ function parseOpts(options, resolvedTarget) {
 module.exports.runClean = function(options) {
 var opts = parseOpts(options);
 var builder = builders[opts.buildMethod];
-return builder.prepEnv()
+return builder.prepEnv(opts)
 .then(function() {
-return builder.clean(opts.extraArgs);
+return builder.clean(opts);
 }).then(function() {
 shell.rm('-rf', path.join(ROOT, 'out'));
+
+['debug', 'release'].forEach(function(config) {
+var propertiesFilePath = path.join(ROOT, config + 
SIGNING_PROPERTIES);
+if(isAutoGenerated(propertiesFilePath)){
+shell.rm('-f', propertiesFilePath);
--- End diff --

prepEnv (which is called in this case) takes care of deleting these if 
there is no supplied signing args. I don't think there's a need to delete them 
unconditionally since we don't bother deleting other build-time-supplied-files 
(like build.xml, build.gradle)


---
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-plugins pull request: CB-6289 Keyboard plugin on Android

2015-03-30 Thread agrieve
Github user agrieve commented on a diff in the pull request:

https://github.com/apache/cordova-plugins/pull/15#discussion_r27393159
  
--- Diff: keyboard/src/android/Keyboard.java ---
@@ -0,0 +1,124 @@
+/*
+   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.
+*/
+package org.apache.cordova.labs.keyboard; 
+
+import org.apache.cordova.CallbackContext;
+import org.apache.cordova.CordovaInterface;
+import org.apache.cordova.CordovaPlugin;
+import org.apache.cordova.CordovaWebView;
+import org.apache.cordova.LOG;
+import org.json.JSONArray;
+import org.json.JSONException;
+
+import android.app.Activity;
+import android.content.Context;
+import android.graphics.Rect;
+import android.util.DisplayMetrics;
+import android.view.View;
+import android.view.ViewTreeObserver.OnGlobalLayoutListener;
+import android.view.inputmethod.InputMethodManager;
+
+public class Keyboard extends CordovaPlugin {
+/**
+* Delta height of the visible area, to be treated as keyboard opening.
+*/
+private final static int MinHeghtDelta = 100;
+private static final String TAG = Keyboard;
+
+public void initialize(CordovaInterface cordova, CordovaWebView 
webView) {
+super.initialize(cordova, webView);
+
+Activity activity = cordova.getActivity();
+DisplayMetrics metrics = new DisplayMetrics();
+
activity.getWindowManager().getDefaultDisplay().getMetrics(metrics);
+final float density = metrics.density;
+
+final CordovaWebView appView = webView;
+
+final View rootView = 
activity.getWindow().getDecorView().findViewById(android.R.id.content).getRootView();
+OnGlobalLayoutListener list = new OnGlobalLayoutListener() {
+int previousHeightDifference = 0;
+
+@Override
+public void onGlobalLayout() {
+LOG.d(TAG, Entering global layout notification);
+
+   Rect visibleRect = new Rect();
+//r will be populated with the coordinates of your view 
that area still visible.
+rootView.getWindowVisibleDisplayFrame(visibleRect);
+
+int visibleHeight = visibleRect.bottom - visibleRect.top;
+int viewHeight = rootView.getRootView().getHeight();
+int heightDifference = (int)((viewHeight - visibleHeight) 
/ density);
+if (heightDifference  MinHeghtDelta 
+ heightDifference != previousHeightDifference) {
+// If the height of the view is bigger then 
+// visible area by delta, then assume that keyboard
+// is shown on the screen.
+appView.sendJavascript(Keyboard.isVisible = true; if 
(Keyboard.onshow) Keyboard.onshow(););
+}
+else if (heightDifference != previousHeightDifference 
+  (previousHeightDifference - heightDifference) 
 MinHeghtDelta){
+// If the difference between visible and view area 
dropped by the delta
+// then assume that this means that keyboard is hidden.
+appView.sendJavascript(Keyboard.isVisible = false; if 
(Keyboard.onhide) Keyboard.onhide(););
--- End diff --

Rather than calling an `onhide` function, I'd suggest you wire this up to 
fire a window event via `cordova.fireWindowEvent`


---
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

[GitHub] cordova-android pull request: solves CB-8768 issue where onActivit...

2015-03-30 Thread agrieve
Github user agrieve commented on a diff in the pull request:

https://github.com/apache/cordova-android/pull/170#discussion_r27391907
  
--- Diff: framework/src/org/apache/cordova/CordovaActivity.java ---
@@ -314,10 +318,52 @@ public void startActivityForResult(Intent intent, int 
requestCode, Bundle option
  * @param intentAn Intent, which can return result data to 
the caller (various data can be attached to Intent extras).
  */
 @Override
-protected void onActivityResult(int requestCode, int resultCode, 
Intent intent) {
+protected void onActivityResult(final int requestCode, final int 
resultCode, final Intent intent) {
 LOG.d(TAG, Incoming Result. Request code =  + requestCode);
 super.onActivityResult(requestCode, resultCode, intent);
-cordovaInterface.onActivityResult(requestCode, resultCode, intent);
+// check if plugins are ready to receive the result
+if (this.pluginsReady) {
+cordovaInterface.onActivityResult(requestCode, resultCode, 
intent);
+this.findCallbackTries = 0;
+} else {
+/**
+ * If the Android OS kills this activity when a plugin 
launches an a new activity 
+ * the onActivityResult event fires before the onResume event 
+ * so we have to wait for the plugins to be loaded again 
before we can hand the result to the correct plugin.
+ */
+final ScheduledThreadPoolExecutor exec = new 
ScheduledThreadPoolExecutor(1);
--- End diff --

Using a separate thread to accomplish what you want here is a bit 
overcomplicated and as is has the unwanted side-effect that the callback will 
be called on a non-UI thread. We also don't want to wait for a resume event to 
fire this.

I'd suggest instead:
- Call `CordovaInterfaceImpl.onActivityResult()`, as before (probably just 
revert all changes to CordovaActivity).
- Have `onActivityResult` just store the result if no initialization has 
happened yet.
- Change `CordovaInterfaceImpl.setPluginManager(pluginManager)` to 
`onCordovaInit(pluginManager)`
- Have `onCordovaInit` dispatch any pending result to the pluginManager 
(plugins services should be registered by 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.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org



[GitHub] cordova-plugins pull request: CB-8337: Fix keyboardShrinksView for...

2015-03-30 Thread agrieve
Github user agrieve commented on the pull request:

https://github.com/apache/cordova-plugins/pull/18#issuecomment-87696102
  
Merged this in, but last I recall discussing the plugin on the dev 
mailinglist, I don't think anyone was interested in pursuing it. Might try 
raising the subject again to see, but otherwise I think it'd be awesome if you 
wanted to fork it and maintain a defacto version of 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.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org



[GitHub] cordova-plugins pull request: CB-6289 Keyboard plugin on Android

2015-03-30 Thread agrieve
Github user agrieve commented on a diff in the pull request:

https://github.com/apache/cordova-plugins/pull/15#discussion_r27393052
  
--- Diff: keyboard/src/android/Keyboard.java ---
@@ -0,0 +1,124 @@
+/*
+   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.
+*/
+package org.apache.cordova.labs.keyboard; 
+
+import org.apache.cordova.CallbackContext;
+import org.apache.cordova.CordovaInterface;
+import org.apache.cordova.CordovaPlugin;
+import org.apache.cordova.CordovaWebView;
+import org.apache.cordova.LOG;
+import org.json.JSONArray;
+import org.json.JSONException;
+
+import android.app.Activity;
+import android.content.Context;
+import android.graphics.Rect;
+import android.util.DisplayMetrics;
+import android.view.View;
+import android.view.ViewTreeObserver.OnGlobalLayoutListener;
+import android.view.inputmethod.InputMethodManager;
+
+public class Keyboard extends CordovaPlugin {
+/**
+* Delta height of the visible area, to be treated as keyboard opening.
+*/
+private final static int MinHeghtDelta = 100;
+private static final String TAG = Keyboard;
+
+public void initialize(CordovaInterface cordova, CordovaWebView 
webView) {
+super.initialize(cordova, webView);
+
+Activity activity = cordova.getActivity();
+DisplayMetrics metrics = new DisplayMetrics();
+
activity.getWindowManager().getDefaultDisplay().getMetrics(metrics);
+final float density = metrics.density;
+
+final CordovaWebView appView = webView;
+
+final View rootView = 
activity.getWindow().getDecorView().findViewById(android.R.id.content).getRootView();
+OnGlobalLayoutListener list = new OnGlobalLayoutListener() {
+int previousHeightDifference = 0;
+
+@Override
+public void onGlobalLayout() {
+LOG.d(TAG, Entering global layout notification);
+
+   Rect visibleRect = new Rect();
+//r will be populated with the coordinates of your view 
that area still visible.
+rootView.getWindowVisibleDisplayFrame(visibleRect);
+
+int visibleHeight = visibleRect.bottom - visibleRect.top;
+int viewHeight = rootView.getRootView().getHeight();
+int heightDifference = (int)((viewHeight - visibleHeight) 
/ density);
+if (heightDifference  MinHeghtDelta 
+ heightDifference != previousHeightDifference) {
+// If the height of the view is bigger then 
+// visible area by delta, then assume that keyboard
+// is shown on the screen.
+appView.sendJavascript(Keyboard.isVisible = true; if 
(Keyboard.onshow) Keyboard.onshow(););
--- End diff --

sendJavascript is deprecated. You should use PluginResults here with 
`setKeepCallback(true)` (allows you to send native-JS at any time after the 
initial call from JS)


---
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-splashscreen pull request: CB-8753 Maintain splash ...

2015-03-30 Thread agrieve
Github user agrieve commented on a diff in the pull request:


https://github.com/apache/cordova-plugin-splashscreen/pull/43#discussion_r27413370
  
--- Diff: src/android/SplashScreen.java ---
@@ -194,9 +263,25 @@ public void run() {
 // TODO: Use the background color of the webView's parent 
instead of using the
 // preference.
 
root.setBackgroundColor(preferences.getInteger(backgroundColor, Color.BLACK));
+
 root.setLayoutParams(new 
LinearLayout.LayoutParams(ViewGroup.LayoutParams.MATCH_PARENT,
 ViewGroup.LayoutParams.MATCH_PARENT, 0.0F));
-root.setBackgroundResource(drawableId);
+
+// Use an ImageView to render the image because of its 
flexible scaling options.
+splashImageView = new ImageView(context);
+splashImageView.setImageResource(drawableId);
--- End diff --

If you call setBackgroundColor on the ImageView, can you omit having a 
parent LinearLayout?


---
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-splashscreen pull request: CB-8753 Maintain splash ...

2015-03-30 Thread agrieve
Github user agrieve commented on a diff in the pull request:


https://github.com/apache/cordova-plugin-splashscreen/pull/43#discussion_r27413145
  
--- Diff: src/android/SplashScreen.java ---
@@ -151,13 +193,40 @@ public Object onMessage(String id, Object data) {
 }
 return null;
 }
+
+@Override
+public void onConfigurationChanged(Configuration newConfig) {
+super.onConfigurationChanged(newConfig);
+
+// Reload splash drawable when orientation changes if so 
configured.
+if (newConfig.orientation != orientation) {
+orientation = newConfig.orientation;
+reloadDrawable();
+}
+}
 
-private void removeSplashScreen() {
+private void reloadDrawable() {
+if (isReloadOnOrientationChange()  splashImageView != null) {
+final int drawableId = 
preferences.getInteger(SplashDrawableId, 0);
+if (drawableId != 0) {
+cordova.getActivity().runOnUiThread(new Runnable() {
--- End diff --

Does this not always run on the UI thread?


---
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-splashscreen pull request: CB-8753 Maintain splash ...

2015-03-30 Thread agrieve
Github user agrieve commented on a diff in the pull request:


https://github.com/apache/cordova-plugin-splashscreen/pull/43#discussion_r27413236
  
--- Diff: src/android/SplashScreen.java ---
@@ -151,13 +193,40 @@ public Object onMessage(String id, Object data) {
 }
 return null;
 }
+
+@Override
+public void onConfigurationChanged(Configuration newConfig) {
+super.onConfigurationChanged(newConfig);
+
+// Reload splash drawable when orientation changes if so 
configured.
+if (newConfig.orientation != orientation) {
+orientation = newConfig.orientation;
+reloadDrawable();
+}
+}
 
-private void removeSplashScreen() {
+private void reloadDrawable() {
+if (isReloadOnOrientationChange()  splashImageView != null) {
--- End diff --

Not sure if you answered in the other PR, but why not just always reload on 
configuration changes? It'll just no-op if there's no new image.


---
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-splashscreen pull request: CB-8753 Maintain splash ...

2015-03-30 Thread agrieve
Github user agrieve commented on the pull request:


https://github.com/apache/cordova-plugin-splashscreen/pull/43#issuecomment-87767088
  
Good stuff! A few minor comments then we're good to go I think!


---
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-splashscreen pull request: CB-8753 Maintain splash ...

2015-03-30 Thread agrieve
Github user agrieve commented on a diff in the pull request:


https://github.com/apache/cordova-plugin-splashscreen/pull/43#discussion_r27412851
  
--- Diff: src/android/SplashScreen.java ---
@@ -37,6 +39,19 @@ Licensed to the Apache Software Foundation (ASF) under 
one
 import org.json.JSONArray;
 import org.json.JSONException;
 
+/**
+ * Splash Screen plugin. Uses the following preferences:
+ * ul
+ * liSplashScreen: Splash screen resource name./li
+ * liSplashScreenDelay: How long splash screen should be shown in 
milliseconds./li
+ * liSplashMaintainAspectRatio: Maintain aspect ratio of the drawable, 
like CSS
--- End diff --

Please add to the README.md as well.


---
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-plugins pull request: CB-6289 Keyboard plugin on Android

2015-03-30 Thread agrieve
Github user agrieve commented on a diff in the pull request:

https://github.com/apache/cordova-plugins/pull/15#discussion_r27444603
  
--- Diff: keyboard/src/android/Keyboard.java ---
@@ -0,0 +1,124 @@
+/*
+   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.
+*/
+package org.apache.cordova.labs.keyboard; 
+
+import org.apache.cordova.CallbackContext;
+import org.apache.cordova.CordovaInterface;
+import org.apache.cordova.CordovaPlugin;
+import org.apache.cordova.CordovaWebView;
+import org.apache.cordova.LOG;
+import org.json.JSONArray;
+import org.json.JSONException;
+
+import android.app.Activity;
+import android.content.Context;
+import android.graphics.Rect;
+import android.util.DisplayMetrics;
+import android.view.View;
+import android.view.ViewTreeObserver.OnGlobalLayoutListener;
+import android.view.inputmethod.InputMethodManager;
+
+public class Keyboard extends CordovaPlugin {
+/**
+* Delta height of the visible area, to be treated as keyboard opening.
+*/
+private final static int MinHeghtDelta = 100;
+private static final String TAG = Keyboard;
+
+public void initialize(CordovaInterface cordova, CordovaWebView 
webView) {
+super.initialize(cordova, webView);
+
+Activity activity = cordova.getActivity();
+DisplayMetrics metrics = new DisplayMetrics();
+
activity.getWindowManager().getDefaultDisplay().getMetrics(metrics);
+final float density = metrics.density;
+
+final CordovaWebView appView = webView;
+
+final View rootView = 
activity.getWindow().getDecorView().findViewById(android.R.id.content).getRootView();
+OnGlobalLayoutListener list = new OnGlobalLayoutListener() {
+int previousHeightDifference = 0;
+
+@Override
+public void onGlobalLayout() {
+LOG.d(TAG, Entering global layout notification);
+
+   Rect visibleRect = new Rect();
+//r will be populated with the coordinates of your view 
that area still visible.
+rootView.getWindowVisibleDisplayFrame(visibleRect);
+
+int visibleHeight = visibleRect.bottom - visibleRect.top;
+int viewHeight = rootView.getRootView().getHeight();
+int heightDifference = (int)((viewHeight - visibleHeight) 
/ density);
+if (heightDifference  MinHeghtDelta 
+ heightDifference != previousHeightDifference) {
+// If the height of the view is bigger then 
+// visible area by delta, then assume that keyboard
+// is shown on the screen.
+appView.sendJavascript(Keyboard.isVisible = true; if 
(Keyboard.onshow) Keyboard.onshow(););
--- End diff --

The main technical reason to not use sendJavascript is because on Android 
it's implemented using `eval()`, and depending on the user's 
Content-Security-Policy, `eval` may not work.

`setKeepCallback(false)` is the default, which means you can call the 
`exec`'s success or failure callback only once.  `setKeepCallback(true)` allows 
you to call success/failure multiple times (so long as each time you also call 
`setKeepCallback(true)`). To go even further:
- By waiting for an initial `exec()` to come in, you won't ever send a 
message to JS before it's ready to receive one.
- You don't need to export an extra symbol just to be called by the native 
code
- On Android, PluginResults are more efficient than sendJavascript


---
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

[GitHub] cordova-plugin-splashscreen pull request: CB-8753 Maintain splash ...

2015-03-30 Thread agrieve
Github user agrieve commented on a diff in the pull request:


https://github.com/apache/cordova-plugin-splashscreen/pull/43#discussion_r27446516
  
--- Diff: src/android/SplashScreen.java ---
@@ -194,9 +263,25 @@ public void run() {
 // TODO: Use the background color of the webView's parent 
instead of using the
 // preference.
 
root.setBackgroundColor(preferences.getInteger(backgroundColor, Color.BLACK));
+
 root.setLayoutParams(new 
LinearLayout.LayoutParams(ViewGroup.LayoutParams.MATCH_PARENT,
 ViewGroup.LayoutParams.MATCH_PARENT, 0.0F));
-root.setBackgroundResource(drawableId);
+
+// Use an ImageView to render the image because of its 
flexible scaling options.
+splashImageView = new ImageView(context);
+splashImageView.setImageResource(drawableId);
--- End diff --

Ahh, okay, that's what I thought your preference was adding, but of course, 
having it always cover the entire screen makes more sense! Great stuff!


---
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-splashscreen pull request: CB-8753 Maintain splash ...

2015-03-30 Thread agrieve
Github user agrieve commented on a diff in the pull request:


https://github.com/apache/cordova-plugin-splashscreen/pull/43#discussion_r27444238
  
--- Diff: src/android/SplashScreen.java ---
@@ -151,13 +193,40 @@ public Object onMessage(String id, Object data) {
 }
 return null;
 }
+
+@Override
+public void onConfigurationChanged(Configuration newConfig) {
+super.onConfigurationChanged(newConfig);
+
+// Reload splash drawable when orientation changes if so 
configured.
+if (newConfig.orientation != orientation) {
+orientation = newConfig.orientation;
+reloadDrawable();
+}
+}
 
-private void removeSplashScreen() {
+private void reloadDrawable() {
+if (isReloadOnOrientationChange()  splashImageView != null) {
--- End diff --

I think a performance hit would be acceptable here given the ease of 
configuration. How often are you going to rotate your device while the splash 
screen is up? Probably close to never, or at most once.


---
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-plugins pull request: CB-6289 Keyboard plugin on Android

2015-03-30 Thread agrieve
Github user agrieve commented on a diff in the pull request:

https://github.com/apache/cordova-plugins/pull/15#discussion_r2735
  
--- Diff: keyboard/src/android/Keyboard.java ---
@@ -0,0 +1,124 @@
+/*
+   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.
+*/
+package org.apache.cordova.labs.keyboard; 
+
+import org.apache.cordova.CallbackContext;
+import org.apache.cordova.CordovaInterface;
+import org.apache.cordova.CordovaPlugin;
+import org.apache.cordova.CordovaWebView;
+import org.apache.cordova.LOG;
+import org.json.JSONArray;
+import org.json.JSONException;
+
+import android.app.Activity;
+import android.content.Context;
+import android.graphics.Rect;
+import android.util.DisplayMetrics;
+import android.view.View;
+import android.view.ViewTreeObserver.OnGlobalLayoutListener;
+import android.view.inputmethod.InputMethodManager;
+
+public class Keyboard extends CordovaPlugin {
+/**
+* Delta height of the visible area, to be treated as keyboard opening.
+*/
+private final static int MinHeghtDelta = 100;
+private static final String TAG = Keyboard;
+
+public void initialize(CordovaInterface cordova, CordovaWebView 
webView) {
+super.initialize(cordova, webView);
+
+Activity activity = cordova.getActivity();
+DisplayMetrics metrics = new DisplayMetrics();
+
activity.getWindowManager().getDefaultDisplay().getMetrics(metrics);
+final float density = metrics.density;
+
+final CordovaWebView appView = webView;
+
+final View rootView = 
activity.getWindow().getDecorView().findViewById(android.R.id.content).getRootView();
+OnGlobalLayoutListener list = new OnGlobalLayoutListener() {
+int previousHeightDifference = 0;
+
+@Override
+public void onGlobalLayout() {
+LOG.d(TAG, Entering global layout notification);
+
+   Rect visibleRect = new Rect();
+//r will be populated with the coordinates of your view 
that area still visible.
+rootView.getWindowVisibleDisplayFrame(visibleRect);
+
+int visibleHeight = visibleRect.bottom - visibleRect.top;
+int viewHeight = rootView.getRootView().getHeight();
+int heightDifference = (int)((viewHeight - visibleHeight) 
/ density);
+if (heightDifference  MinHeghtDelta 
+ heightDifference != previousHeightDifference) {
+// If the height of the view is bigger then 
+// visible area by delta, then assume that keyboard
+// is shown on the screen.
+appView.sendJavascript(Keyboard.isVisible = true; if 
(Keyboard.onshow) Keyboard.onshow(););
+}
+else if (heightDifference != previousHeightDifference 
+  (previousHeightDifference - heightDifference) 
 MinHeghtDelta){
+// If the difference between visible and view area 
dropped by the delta
+// then assume that this means that keyboard is hidden.
+appView.sendJavascript(Keyboard.isVisible = false; if 
(Keyboard.onhide) Keyboard.onhide(););
--- End diff --

ugh, yeah, should be fixed up on iOS as well too I suppose. There's no 
reason to use a single callback rather than an event. Events are much more 
flexible / normal.


---
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-android pull request: CB-8484: Adds support creating signe...

2015-03-30 Thread agrieve
Github user agrieve commented on a diff in the pull request:

https://github.com/apache/cordova-android/pull/164#discussion_r27446054
  
--- Diff: bin/templates/cordova/lib/build.js ---
@@ -409,7 +457,31 @@ function parseOpts(options, resolvedTarget) {
 console.warn('Build option \'' + options[i] + '\' not 
recognized (ignoring).');
 }
 }
+if (packageArgs.keystore  packageArgs.alias) {
+var keystore = path.relative(ROOT, 
path.resolve(packageArgs.keystore));
+ret.packageInfo = new PackageInfo(keystore, packageArgs.alias, 
packageArgs.keystorePassword,
+packageArgs.password, packageArgs.keystoreType);
+} else if (fs.existsSync(buildConfig)) {
+console.log('Reading build config file: '+ buildConfig);
+var config = JSON.parse(fs.readFileSync(buildConfig, 'utf8'));
+if (config.android  config.android[ret.buildType]) {
+var androidInfo = config.android[ret.buildType];
+if (androidInfo.keystore  androidInfo.alias) {
+var configDir = path.dirname(buildConfig);
+var keystorePath = path.relative(ROOT, 
path.join(configDir, androidInfo.keystore));
+ret.packageInfo = new PackageInfo(keystorePath, 
androidInfo.alias,
+androidInfo.keystorePassword, androidInfo.password, 
androidInfo.storeType);
--- End diff --

A bit inconsistent to have keystorePassword and storeType (should be 
storePassword, or keystoreType). I think the [gradle 
terms](http://stackoverflow.com/questions/18328730/how-to-create-a-release-signed-apk-file-using-gradle)
 for these make sense, so might as well use them?


---
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-8484: Adds support creating signe...

2015-03-30 Thread agrieve
Github user agrieve commented on a diff in the pull request:

https://github.com/apache/cordova-android/pull/164#discussion_r27446058
  
--- Diff: bin/templates/cordova/lib/build.js ---
@@ -409,7 +457,31 @@ function parseOpts(options, resolvedTarget) {
 console.warn('Build option \'' + options[i] + '\' not 
recognized (ignoring).');
 }
 }
+if (packageArgs.keystore  packageArgs.alias) {
+var keystore = path.relative(ROOT, 
path.resolve(packageArgs.keystore));
+ret.packageInfo = new PackageInfo(keystore, packageArgs.alias, 
packageArgs.keystorePassword,
+packageArgs.password, packageArgs.keystoreType);
+} else if (fs.existsSync(buildConfig)) {
--- End diff --

Should just check: `if (buildConfig)`, and show an error if it's set and 
the file doesn't exist.


---
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-8484: Adds support creating signe...

2015-03-30 Thread agrieve
Github user agrieve commented on a diff in the pull request:

https://github.com/apache/cordova-android/pull/164#discussion_r27446020
  
--- Diff: bin/templates/cordova/lib/build.js ---
@@ -530,8 +609,54 @@ module.exports.findBestApkForArchitecture = 
function(buildResults, arch) {
 throw new Error('Could not find apk architecture: ' + arch + ' 
build-type: ' + buildResults.buildType);
 };
 
+function PackageInfo(keystore, alias, keystorePassword, password, 
storeType) {
+keystore = keystore.replace(/\\/g, '');
+this.keystore = {
+'name': 'key.store',
+'value': keystore
+};
+this.alias = {
+'name': 'key.alias',
+'value': alias
+};
+if (keystorePassword) {
+this.keystorePassword = {
+'name': 'key.store.password',
+'value': keystorePassword
+};
+}
+if (password) {
+this.password = {
+'name': 'key.alias.password',
+'value': password
+};
+}
+if (storeType) {
+this.storeType = {
+'name': 'key.store.type',
+'value': storeType
+};
+}
+}
+
+PackageInfo.prototype = {
+toProperties: function() {
+var self = this;
+var result = '';
+Object.keys(self).forEach(function(key) {
+if (self[key]) {
--- End diff --

Probably should not require the value to be non-empty. Perfectly valid to 
have `key.alias.password=` in the file (although not recommended :P)


---
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-splashscreen pull request: CB-8753 Maintain splash ...

2015-03-30 Thread agrieve
Github user agrieve commented on a diff in the pull request:


https://github.com/apache/cordova-plugin-splashscreen/pull/43#discussion_r27444149
  
--- Diff: src/android/SplashScreen.java ---
@@ -194,9 +263,25 @@ public void run() {
 // TODO: Use the background color of the webView's parent 
instead of using the
 // preference.
 
root.setBackgroundColor(preferences.getInteger(backgroundColor, Color.BLACK));
+
 root.setLayoutParams(new 
LinearLayout.LayoutParams(ViewGroup.LayoutParams.MATCH_PARENT,
 ViewGroup.LayoutParams.MATCH_PARENT, 0.0F));
-root.setBackgroundResource(drawableId);
+
+// Use an ImageView to render the image because of its 
flexible scaling options.
+splashImageView = new ImageView(context);
+splashImageView.setImageResource(drawableId);
--- End diff --

Hmm, I figured the background would show for a square image when 
SplashMaintainAspectRatio is set. I think you've done due diligence to it 
though, so if it's not required, then don't even bother setting 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.
---

-
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-8484: Adds support creating signe...

2015-03-30 Thread agrieve
Github user agrieve commented on a diff in the pull request:

https://github.com/apache/cordova-android/pull/164#discussion_r27446338
  
--- Diff: bin/templates/cordova/lib/build.js ---
@@ -409,7 +457,31 @@ function parseOpts(options, resolvedTarget) {
 console.warn('Build option \'' + options[i] + '\' not 
recognized (ignoring).');
 }
 }
+if (packageArgs.keystore  packageArgs.alias) {
--- End diff --

Wonding if this should actually not be exclusive with the else if clause. 
E.g. allow specifying buildConfig, and then also allow setting the password via 
--password


---
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-8484: Adds support creating signe...

2015-03-30 Thread agrieve
Github user agrieve commented on a diff in the pull request:

https://github.com/apache/cordova-android/pull/164#discussion_r27446385
  
--- Diff: bin/templates/cordova/lib/build.js ---
@@ -422,11 +477,15 @@ function parseOpts(options, resolvedTarget) {
 module.exports.runClean = function(options) {
 var opts = parseOpts(options);
 var builder = builders[opts.buildMethod];
-return builder.prepEnv()
+return builder.prepEnv(opts)
 .then(function() {
-return builder.clean(opts.extraArgs);
+return builder.clean(opts);
 }).then(function() {
 shell.rm('-rf', path.join(ROOT, 'out'));
+}).then(function() {
+['debug', 'release'].forEach(function(config) {
+removeIfExists(path.join(ROOT, config + SIGNING_PROPERTIES));
--- End diff --

I like 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.
---

-
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-8484: Adds support creating signe...

2015-03-30 Thread agrieve
Github user agrieve commented on a diff in the pull request:

https://github.com/apache/cordova-android/pull/164#discussion_r27445351
  
--- Diff: bin/templates/cordova/lib/build.js ---
@@ -372,6 +409,7 @@ function parseOpts(options, resolvedTarget) {
 if (multiValueArgs[flagName]  !flagValue) {
 flagValue = options[i + 1];
 ++i;
+console.warn('A value is expected for the flag: ' + 
flagName);
--- End diff --

I think this warning will show when not using an = for the flag value 
(generally for flags with values, you're allowed to do: --foo=bar *or* --foo bar


---
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-8484: Adds support creating signe...

2015-03-30 Thread agrieve
Github user agrieve commented on a diff in the pull request:

https://github.com/apache/cordova-android/pull/164#discussion_r27445404
  
--- Diff: bin/templates/cordova/lib/build.js ---
@@ -402,6 +440,16 @@ function parseOpts(options, resolvedTarget) {
 case 'gradleArg':
 ret.extraArgs.push(flagValue);
 break;
+case 'keystore':
+case 'alias':
+case 'keytorePassword':
+case 'password':
+case 'keystoreType':
+packageArgs[flagName] = flagValue;
+break;
+case 'buildConfig':
+buildConfig = flagValue.replace(//g, '');
--- End diff --

Shouldn't need the `.replace()` here... Did you find it necessary for some 
reason?


---
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-8754 Auto-restoring a plugin fails wh...

2015-03-27 Thread agrieve
Github user agrieve commented on the pull request:

https://github.com/apache/cordova-lib/pull/194#issuecomment-86956650
  
Nice clean-up! Let me know when comments are addressed and I'll merge it in.


---
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-8754 Auto-restoring a plugin fails wh...

2015-03-27 Thread agrieve
Github user agrieve commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/194#discussion_r27298069
  
--- Diff: cordova-lib/src/util/npm-helper.js ---
@@ -0,0 +1,76 @@
+/**
+ 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.
+ */
+
+// Helper methods to help keep npm operations separated.
+
+var npm = require('npm'),
+Q = require('q'),
+cachedSettings = null,
+cachedSettingsValues = null;
+
+/**
+ * @description Calls npm.load, then initializes npm.config with the 
specified settings. Then executes a chain of
+ * promises that rely on those npm settings, then restores npm settings 
back to their previous value. Use this rather
+ * than passing settings to npm.load, since that only works the first time 
you try to load npm.
+ * @param {Object} settings
+ * @param {Function} promiseChain
+ */
+function loadWithSettingsThenRestore(settings, promiseChain) {
+return 
loadWithSettings(settings).then(promiseChain).then(restoreSettings);
--- End diff --

nit: should restoreSettings be within a .finally() rather than a .then()?


---
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-8754 Auto-restoring a plugin fails wh...

2015-03-27 Thread agrieve
Github user agrieve commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/194#discussion_r27298165
  
--- Diff: cordova-lib/src/plugman/registry/registry.js ---
@@ -223,11 +211,13 @@ module.exports = {
  * @param {Boolean} determines if we are using the npm registry
  * @return {Promise.Object} Promised settings.
  */
-function initSettings() {
+function initSettings(returnEmptySettings) {
 var settings = module.exports.settings;
 
 // check if settings already set
-if(settings !== null) return Q(settings);
+if(settings !== null) {
--- End diff --

This returns settings even when returnEmptySettings is true. should 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.
---

-
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-8753 Maintain splash screen aspec...

2015-03-27 Thread agrieve
Github user agrieve commented on the pull request:

https://github.com/apache/cordova-android/pull/168#issuecomment-87076470
  
Install the plugin with the --link flag, and then you should be able to 
edit it in the projects and the changes will be reflected in the repo (it 
creates symlinks)


---
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-8753 Maintain splash screen aspec...

2015-03-26 Thread agrieve
Github user agrieve commented on the pull request:

https://github.com/apache/cordova-android/pull/168#issuecomment-86521912
  
Great questions.

The statics are meant to prevent multiple splash screens from showing if 
there is more than one CordovaWebView within a project.

`CordovaPreferences.copyIntoIntentExtras(Activity)` - it's legacy.

keep getting preferences from CordovaPreferences - because embedders can 
change these preferences during app start-up, and we want those changes to take 
effect.

New Prefs android only - yes. that's fine. Splash screen already has mostly 
platform-specific prefs :(

Overall comments:
- Randomly, an `onConfigurationChange` hook was just recently added to 
master, so you should just use that instead.
- We aren't planning on doing another release of 3.x branch, so you should 
make these changes to cordova-plugin-splashscreen, which is what will work with 
cordova-android@4.0.x (and with master branch of cordova-android)
- I don't see why anyone would want reloadOnOrientationChange=false, so 
maybe don't make that a preference?
- Might be better to get rid of the LinearLayout, and instead switch to an 
ImageView in both cases


---
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-8484: Adds support creating signe...

2015-03-26 Thread agrieve
Github user agrieve commented on a diff in the pull request:

https://github.com/apache/cordova-android/pull/164#discussion_r27270342
  
--- Diff: bin/templates/cordova/lib/build.js ---
@@ -422,11 +477,15 @@ function parseOpts(options, resolvedTarget) {
 module.exports.runClean = function(options) {
 var opts = parseOpts(options);
 var builder = builders[opts.buildMethod];
-return builder.prepEnv()
+return builder.prepEnv(opts)
 .then(function() {
-return builder.clean(opts.extraArgs);
+return builder.clean(opts);
 }).then(function() {
 shell.rm('-rf', path.join(ROOT, 'out'));
+}).then(function() {
+['debug', 'release'].forEach(function(config) {
+removeIfExists(path.join(ROOT, config + SIGNING_PROPERTIES));
--- End diff --

hmm, maybe for this reason you should pass the parameters on the command 
line as gradle properties so that there is no file to clean up. Note that it's 
unlikely the ANT workflow is important anymore, as gradle is the new default 
and seems to work much better.


---
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-splashscreen pull request: Fixed installation instr...

2015-03-24 Thread agrieve
Github user agrieve commented on the pull request:


https://github.com/apache/cordova-plugin-splashscreen/pull/40#issuecomment-85662478
  
The next version of the plugin will be installable by this name, it's just 
not working yet.


---
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-network-information pull request: Allowing to exten...

2015-03-24 Thread agrieve
Github user agrieve commented on the pull request:


https://github.com/apache/cordova-plugin-network-information/pull/26#issuecomment-85620698
  
I'd prefer not to open up the internals of the plugin, as that would 
increase the maintenance burden by broadening its API. It's not very big, so 
how about just copy / pasting it into your own plugin?


---
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-8484: Adds support creating signe...

2015-03-18 Thread agrieve
Github user agrieve commented on a diff in the pull request:

https://github.com/apache/cordova-android/pull/164#discussion_r26704092
  
--- Diff: bin/templates/cordova/lib/build.js ---
@@ -409,7 +442,29 @@ function parseOpts(options, resolvedTarget) {
 console.warn('Build option \'' + options[i] + '\' not 
recognized (ignoring).');
 }
 }
+if (packageArgs.keystore  packageArgs.alias) {
+var keystore = path.relative(ROOT, 
path.resolve(packageArgs.keystore));
+ret.packageInfo = new PackageInfo(keystore, packageArgs.alias, 
packageArgs.keystorePassword,
+packageArgs.password, packageArgs.keystoreType);
+} else if (fs.existsSync(path.join(CORDOVAROOT, BUILD_CONFIG_FILE))) {
--- End diff --

We've made it this far without platforms having CLI-specific code, so I 
think we should not check at this file location. Instead, we could have 
cordova prepare create -signing.properties files, or have cordova run pass 
along all the signing args as CLI arguments. Could even pass it along as a 
single argument that points to the JSON file, but I don't think we should look 
for it here directly.


---
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-8484: Adds support creating signe...

2015-03-18 Thread agrieve
Github user agrieve commented on a diff in the pull request:

https://github.com/apache/cordova-android/pull/164#discussion_r26704343
  
--- Diff: bin/templates/cordova/lib/build.js ---
@@ -422,11 +477,15 @@ function parseOpts(options, resolvedTarget) {
 module.exports.runClean = function(options) {
 var opts = parseOpts(options);
 var builder = builders[opts.buildMethod];
-return builder.prepEnv()
+return builder.prepEnv(opts)
 .then(function() {
-return builder.clean(opts.extraArgs);
+return builder.clean(opts);
 }).then(function() {
 shell.rm('-rf', path.join(ROOT, 'out'));
+}).then(function() {
+['debug', 'release'].forEach(function(config) {
+removeIfExists(path.join(ROOT, config + SIGNING_PROPERTIES));
--- End diff --

For non-CLI workflow, the user will likely create these files manually and 
be upset if clean deletes them. Probably best to just never delete them.


---
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-8484: Adds support creating signe...

2015-03-18 Thread agrieve
Github user agrieve commented on a diff in the pull request:

https://github.com/apache/cordova-android/pull/164#discussion_r26704402
  
--- Diff: bin/templates/cordova/lib/build.js ---
@@ -530,8 +589,60 @@ module.exports.findBestApkForArchitecture = 
function(buildResults, arch) {
 throw new Error('Could not find apk architecture: ' + arch + ' 
build-type: ' + buildResults.buildType);
 };
 
+function removeIfExists(file) {
--- End diff --

nit: `shell.rm('-f', file)` will do 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-android pull request: CB-8484: Adds support creating signe...

2015-03-18 Thread agrieve
Github user agrieve commented on a diff in the pull request:

https://github.com/apache/cordova-android/pull/164#discussion_r26704510
  
--- Diff: bin/templates/cordova/lib/build.js ---
@@ -542,5 +653,12 @@ module.exports.help = function() {
 console.log('\'--versionCode=#\': Override versionCode for this 
build. Useful for uploading multiple APKs. Requires --gradle.');
 console.log('\'--minSdkVersion=#\': Override minSdkVersion for 
this build. Useful for uploading multiple APKs. Requires --gradle.');
 console.log('\'--gradleArg=gradle command line arg\': Extra args 
to pass to the gradle command. Use one flag per arg. Ex. 
--gradleArg=-PcdvBuildMultipleApks=true');
+console.log('');
+console.log('Signed APK flags:');
--- End diff --

Would be good to mention that these are overrides for what's already in 
`debug-signing.properties` / `release-signing.properties`


---
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-8554 Updated source to pass F...

2015-03-17 Thread agrieve
Github user agrieve commented on the pull request:

https://github.com/apache/cordova-plugin-file/pull/102#issuecomment-82477043
  
Changes look good I think, but I've sadly landed some code before looking 
at this that causes it to not merge cleanly. Would you be able to rebase?


---
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: fix(ios): always use setters to ...

2015-03-17 Thread agrieve
Github user agrieve commented on the pull request:

https://github.com/apache/cordova-plugin-file/pull/104#issuecomment-82476143
  
Why not just enable ARC on the file? Cordova  its plugins now assume ARC 
to be enabled, so problems will just keep popping up without it I think.


---
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-whitelist pull request: Test PR for github / JIRA i...

2015-03-13 Thread agrieve
GitHub user agrieve opened a pull request:

https://github.com/apache/cordova-plugin-whitelist/pull/2

Test PR for github / JIRA integration



You can merge this pull request into a Git repository by running:

$ git pull https://github.com/agrieve/cordova-plugin-whitelist patch-1

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/cordova-plugin-whitelist/pull/2.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 #2


commit 546604953c6056d60bb1420f1fb105ada7b97e5c
Author: agrieve agri...@chromium.org
Date:   2015-03-13T17:11:40Z

Test PR for github / JIRA integration




---
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-whitelist pull request: Test PR for github / JIRA i...

2015-03-13 Thread agrieve
Github user agrieve commented on the pull request:


https://github.com/apache/cordova-plugin-whitelist/pull/2#issuecomment-79145875
  
Test CB-4789


---
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-whitelist pull request: Test PR for github / JIRA i...

2015-03-13 Thread agrieve
Github user agrieve closed the pull request at:

https://github.com/apache/cordova-plugin-whitelist/pull/2


---
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: Fix issue where android cordov...

2015-03-04 Thread agrieve
Github user agrieve commented on the pull request:


https://github.com/apache/cordova-plugin-camera/pull/65#issuecomment-77190993
  
Change looks awesome! It's a bit much to pull in all at once though. Could 
you rebase this into three distinct commits (error msgs, dropbox fix, gallery 
param)?

Wondering if you'd also like to send a PR to improve the URL-filePath 
logic in cordova-android's [CordovaResourceApi 
class](https://github.com/apache/cordova-android/blob/master/framework/src/org/apache/cordova/CordovaResourceApi.java#L143)?





---
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...

2015-03-04 Thread agrieve
Github user agrieve commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/175#discussion_r25779722
  
--- Diff: cordova-lib/src/plugman/registry/registry.js ---
@@ -236,15 +231,22 @@ function initSettings() {
 module.exports.settings =
 rc('plugman', {
 cache: plugmanCacheDir,
-registry: 'http://registry.cordova.io',
+registry: registryURL,
 logstream: fs.createWriteStream(path.resolve(plugmanConfigDir, 
'plugman.log')),
 userconfig: path.resolve(plugmanConfigDir, 'config'),
 'cache-min': oneDay
 });
+
+// if npm is true, use npm registry. 
+// ~/.plugman/config overides the above config if it exists. 
--- End diff --

ahh, gotcha.


---
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...

2015-03-03 Thread agrieve
Github user agrieve commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/175#discussion_r25725351
  
--- Diff: cordova-lib/src/plugman/registry/registry.js ---
@@ -236,15 +231,22 @@ function initSettings() {
 module.exports.settings =
 rc('plugman', {
 cache: plugmanCacheDir,
-registry: 'http://registry.cordova.io',
+registry: registryURL,
 logstream: fs.createWriteStream(path.resolve(plugmanConfigDir, 
'plugman.log')),
 userconfig: path.resolve(plugmanConfigDir, 'config'),
 'cache-min': oneDay
 });
+
+// if npm is true, use npm registry. 
+// ~/.plugman/config overides the above config if it exists. 
--- End diff --

I think it's the same that Tim points out. Normally users don't have a 
registry override in their plugman/config, but when they do, we should use it. 
Probably the logic should actually be:

if (custom registry) {
  use it only
} else {
  try CPR and fallback to NPM
}

It's pretty fringe though, so maybe just add a TODO to support custom 
registries again?


---
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...

2015-03-03 Thread agrieve
Github user agrieve commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/175#discussion_r25690407
  
--- Diff: cordova-lib/src/plugman/fetch.js ---
@@ -147,6 +146,9 @@ function fetchPlugin(plugin_src, plugins_dir, options) {
 checkID(options.expected_id, result.pinfo);
 metadata.save_fetch_metadata(plugins_dir, result.pinfo.id, { 
source: result.fetchJsonSource });
 return result.dest;
+})
+.fail(function(error) {
--- End diff --

This is a no-op, is it not?


---
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...

2015-03-03 Thread agrieve
Github user agrieve commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/175#discussion_r25690632
  
--- 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.string} 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.string} 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
--- End diff --

I don't think we need a comment to tell us what the function name is :)


---
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...

2015-03-03 Thread agrieve
Github user agrieve commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/175#discussion_r25690367
  
--- 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 --

Might as well export the reverse mapping from pluginMapper so that you 
don't need to do this linear search. Exposing the map as a sub-key of the 
module is a good idea anyways, since doing it top-level restricts you from ever 
adding another symbol to 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.
---

-
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...

2015-03-03 Thread agrieve
Github user agrieve commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/175#discussion_r25690540
  
--- Diff: cordova-lib/src/plugman/registry/registry.js ---
@@ -236,15 +231,22 @@ function initSettings() {
 module.exports.settings =
 rc('plugman', {
 cache: plugmanCacheDir,
-registry: 'http://registry.cordova.io',
+registry: registryURL,
 logstream: fs.createWriteStream(path.resolve(plugmanConfigDir, 
'plugman.log')),
 userconfig: path.resolve(plugmanConfigDir, 'config'),
 'cache-min': oneDay
 });
+
+// if npm is true, use npm registry. 
+// ~/.plugman/config overides the above config if it exists. 
--- End diff --

isn't this by design? Wouldn't you still want to be able to set 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-8551 npm registry integration for fet...

2015-03-03 Thread agrieve
Github user agrieve commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/175#discussion_r25691265
  
--- 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.string} 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.string} 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.string} 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-]*[^@])/;
--- End diff --

I think we require only two dots. Might make sense to be a bit less strict 
here: `/([^@]*\.[^@]*\.[^@]*)/.exec(plugin)`


---
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...

2015-03-03 Thread agrieve
Github user agrieve commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/175#discussion_r25691366
  
--- 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.string} 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.string} 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.string} Promised path to fetched package.
+ */
+function checkPluginID(plugin) {
--- End diff --

checkPluginId doesn't make this method's purpose obvious. How splitting 
it into `isValidCprName()` and `warnIfRenamed()`.

Would also be a bit more obvious if it returned true/false instead of a 
promise, and to do the logging at the call site.


---
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...

2015-03-03 Thread agrieve
Github user agrieve commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/175#discussion_r25691445
  
--- 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 --

Where is the source for 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-8499 `cordova platform save` should s...

2015-03-02 Thread agrieve
Github user agrieve commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/170#discussion_r25604353
  
--- Diff: cordova-lib/src/cordova/platform_metadata.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 path = require('path'),
+cordova_util = require('./util'),
+fs   = require('fs'),
+Q= require('q'),
+shelljs  = require('shelljs');
+
+
+// Retrieves the platforms and their versions from the platforms.json file
+// Returns an array of {platform: platform, version: version} ...
+// ... where version could be '3.4.0', '/path/to/platform' or 'git://...'
+function getVersions(projectRoot) {
+var platformsDir = path.join(projectRoot, 'platforms');
+var platformsJsonFile = path.join(platformsDir, 'platforms.json');
+
+// If the platforms.json file doesn't exist, retrieve versions from 
platforms installed on the filesystem...
+// ...Note that in this case, we won't be able to know what 
source(folder, git-url) the platform came from, we'll just use versions
+return getPlatVersionsFromFile(platformsJsonFile).fail(function(){
+return getPlatVersionsFromFileSystem(projectRoot);
+});
+}
+
+// Returns a promise
+function getPlatVersionsFromFile(platformsJsonFile){
+var platformData = require(platformsJsonFile);
--- End diff --

It is a bit confusing to use require() on non-app code 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-lib pull request: CB-8499 `cordova platform save` should s...

2015-03-02 Thread agrieve
Github user agrieve commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/170#discussion_r25629266
  
--- Diff: cordova-lib/src/cordova/platform_metadata.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 path = require('path'),
+cordova_util = require('./util'),
+fs   = require('fs'),
+Q= require('q'),
+shelljs  = require('shelljs');
+
+
+// Retrieves the platforms and their versions from the platforms.json file
+// Returns an array of {platform: platform, version: version} ...
+// ... where version could be '3.4.0', '/path/to/platform' or 'git://...'
+function getVersions(projectRoot) {
+var platformsDir = path.join(projectRoot, 'platforms');
+var platformsJsonFile = path.join(platformsDir, 'platforms.json');
+
+// If the platforms.json file doesn't exist, retrieve versions from 
platforms installed on the filesystem...
+// ...Note that in this case, we won't be able to know what 
source(folder, git-url) the platform came from, we'll just use versions
+return getPlatVersionsFromFile(platformsJsonFile).fail(function(){
+return getPlatVersionsFromFileSystem(projectRoot);
+});
+}
+
+// Returns a promise
+function getPlatVersionsFromFile(platformsJsonFile){
+var platformData = require(platformsJsonFile);
+var platformVersions = [];
+
+platformVersions = Object.keys(platformData).map(function(p){
+return {platform: p, version: platformData[p]};
+});
+
+return Q(platformVersions);
+}
+
+// Returns a promise
+function getPlatVersionsFromFileSystem(projectRoot){
+var platformVersions = [];
+var platforms_on_fs = cordova_util.listPlatforms(projectRoot);
+platforms_on_fs.forEach(function(platform){
+var script = path.join(projectRoot, 'platforms', platform, 
'cordova', 'version');
+
+// Use version script to retrieve installed version of the platform
+var version = shelljs.exec(script).output;
+
+// clean the version we get back from the script
+// This is necessary because the version script uses console.log 
to pass back
+// the version. Using console.log ends up adding additional line 
breaks/newlines to the value returned. 
+// ToDO: version scripts should be refactored to not use 
console.log()
+var versionCleaned = version.replace(/\r?\n|\r/g, '');
+platformVersions.push({platform: platform, version: 
versionCleaned});
+});
+
+return Q(platformVersions); 
+}
+
+// Saves platform@version into platforms.json
+function save(projectRoot, platform, version) {
+var platformsDir = path.join(projectRoot, 'platforms');
+var platformJsonFile = path.join(platformsDir, 'platforms.json');
+if(!fs.existsSync(platformJsonFile)){
+fs.writeFileSync(platformJsonFile, JSON.stringify({}, null, 4), 
'utf-8');
--- End diff --

writing to a non-existing file should work fine. do you mean read from it?

You're already checking if it exists, so you might as well:

var data = {}
if (exists) {
data = getJson();
}



---
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-7827: Allow user to specify andro...

2015-02-25 Thread agrieve
Github user agrieve commented on the pull request:

https://github.com/apache/cordova-android/pull/160#issuecomment-76036058
  
Merged. dcff8794adb5a8e772a


---
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-8255 Use properties rather than e...

2015-02-25 Thread agrieve
Github user agrieve closed the pull request at:

https://github.com/apache/cordova-android/pull/145


---
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-8382 Make CordovaActivity not imp...

2015-02-25 Thread agrieve
Github user agrieve closed the pull request at:

https://github.com/apache/cordova-android/pull/152


---
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: Update gitignore

2015-02-25 Thread agrieve
Github user agrieve commented on the pull request:

https://github.com/apache/cordova-android/pull/161#issuecomment-76035301
  
This is generated when using the cordova npm tool, but it's still a valid 
workflow to use the `bin/create` script directly. I don't think we'd want to 
add this in that case.


---
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-7827: Allow user to specify andro...

2015-02-23 Thread agrieve
Github user agrieve commented on a diff in the pull request:

https://github.com/apache/cordova-android/pull/160#discussion_r25186024
  
--- Diff: bin/create ---
@@ -23,14 +23,15 @@ var create = require('./lib/create');
 var args  = require('./lib/simpleargs').getArgs(process.argv);
 
 if (args['--help'] || args._.length === 0) {
-console.log('Usage: ' + path.relative(process.cwd(), 
path.join(__dirname, 'create')) + ' path_to_new_project package_name 
project_name [template_path] [--link]');
+console.log('Usage: ' + path.relative(process.cwd(), 
path.join(__dirname, 'create')) + ' path_to_new_project package_name 
project_name activity_name [template_path] [--link]');
--- End diff --

Shifting the `template_path` arg will break scripts that currently use 
it. Maybe use `--activity-name=` 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-lib pull request: CB-7827: Allow user to specify android a...

2015-02-23 Thread agrieve
Github user agrieve commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/171#discussion_r25186284
  
--- Diff: cordova-lib/src/cordova/platform.js ---
@@ -521,19 +521,27 @@ function getCreateArgs(platDetails, projectRoot, cfg, 
template_dir, opts) {
 if (/android|ios/.exec(platDetails.platform)  
semver.gt(platDetails.version, '3.3.0')) {
 args.push('--cli');
 }
-
+
 var pkg = cfg.packageName().replace(/[^\w.]/g,'_');
 // 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
 var name = platDetails.platform == 'ios' ? unorm.nfd(cfg.name()) : 
cfg.name();
 args.push(output, pkg, name);
+
+var activityName = cfg.android_activityName();
+if (activityName  platDetails.platform === 'android') {
+activityName = activityName.replace(/W/g, '');
+   args.push(activityName);
--- End diff --

We need this to continue to work for older versions of cordova-android. You 
should add this arg only if:

semver.gte(platDetails.version, '4.0.0-dev')


---
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-8499 `cordova platform save` should s...

2015-02-23 Thread agrieve
Github user agrieve commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/170#discussion_r25224748
  
--- Diff: cordova-lib/src/cordova/platform_metadata.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 path = require('path'),
+cordova_util = require('./util'),
+fs   = require('fs'),
+Q= require('q'),
+shelljs  = require('shelljs');
+
+
+// Retrieves the platforms and their versions from the platforms.json file
+// Returns an array of {platform: platform, version: version} ...
+// ... where version could be '3.4.0', '/path/to/platform' or 'git://...'
+function getVersions(projectRoot) {
+var platformsDir = path.join(projectRoot, 'platforms');
+var platformsJsonFile = path.join(platformsDir, 'platforms.json');
+
+// If the platforms.json file doesn't exist, retrieve versions from 
platforms installed on the filesystem...
+// ...Note that in this case, we won't be able to know what 
source(folder, git-url) the platform came from, we'll just use versions
+return getPlatVersionsFromFile(platformsJsonFile).fail(function(){
+return getPlatVersionsFromFileSystem(projectRoot);
+});
+}
+
+// Returns a promise
+function getPlatVersionsFromFile(platformsJsonFile){
+var platformData = require(platformsJsonFile);
+var platformVersions = [];
+
+platformVersions = Object.keys(platformData).map(function(p){
+return {platform: p, version: platformData[p]};
+});
+
+return Q(platformVersions);
+}
+
+// Returns a promise
+function getPlatVersionsFromFileSystem(projectRoot){
+var platformVersions = [];
+var platforms_on_fs = cordova_util.listPlatforms(projectRoot);
+platforms_on_fs.forEach(function(platform){
+var script = path.join(projectRoot, 'platforms', platform, 
'cordova', 'version');
+
+// Use version script to retrieve installed version of the platform
+var version = shelljs.exec(script).output;
+
+// clean the version we get back from the script
+// This is necessary because the version script uses console.log 
to pass back
+// the version. Using console.log ends up adding additional line 
breaks/newlines to the value returned. 
+// ToDO: version scripts should be refactored to not use 
console.log()
+var versionCleaned = version.replace(/\r?\n|\r/g, '');
+platformVersions.push({platform: platform, version: 
versionCleaned});
+});
+
+return Q(platformVersions); 
+}
+
+// Saves platform@version into platforms.json
+function save(projectRoot, platform, version) {
+var platformsDir = path.join(projectRoot, 'platforms');
+var platformJsonFile = path.join(platformsDir, 'platforms.json');
+if(!fs.existsSync(platformJsonFile)){
+fs.writeFileSync(platformJsonFile, JSON.stringify({}, null, 4), 
'utf-8');
--- End diff --

unless I'm missing something, you overwrite this file a few lines below. No 
need to write an empty file 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-lib pull request: CB-8499 `cordova platform save` should s...

2015-02-23 Thread agrieve
Github user agrieve commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/170#discussion_r25224693
  
--- Diff: cordova-lib/src/cordova/platform_metadata.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 path = require('path'),
+cordova_util = require('./util'),
+fs   = require('fs'),
+Q= require('q'),
+shelljs  = require('shelljs');
+
+
+// Retrieves the platforms and their versions from the platforms.json file
+// Returns an array of {platform: platform, version: version} ...
+// ... where version could be '3.4.0', '/path/to/platform' or 'git://...'
+function getVersions(projectRoot) {
+var platformsDir = path.join(projectRoot, 'platforms');
+var platformsJsonFile = path.join(platformsDir, 'platforms.json');
+
+// If the platforms.json file doesn't exist, retrieve versions from 
platforms installed on the filesystem...
+// ...Note that in this case, we won't be able to know what 
source(folder, git-url) the platform came from, we'll just use versions
+return getPlatVersionsFromFile(platformsJsonFile).fail(function(){
+return getPlatVersionsFromFileSystem(projectRoot);
+});
+}
+
+// Returns a promise
+function getPlatVersionsFromFile(platformsJsonFile){
+var platformData = require(platformsJsonFile);
+var platformVersions = [];
+
+platformVersions = Object.keys(platformData).map(function(p){
+return {platform: p, version: platformData[p]};
+});
+
+return Q(platformVersions);
+}
+
+// Returns a promise
+function getPlatVersionsFromFileSystem(projectRoot){
+var platformVersions = [];
+var platforms_on_fs = cordova_util.listPlatforms(projectRoot);
+platforms_on_fs.forEach(function(platform){
+var script = path.join(projectRoot, 'platforms', platform, 
'cordova', 'version');
+
+// Use version script to retrieve installed version of the platform
+var version = shelljs.exec(script).output;
--- End diff --

`shelljs.exec` is known to have poor performance and lead to EMFILE 
exceptions. Please use child_process.exec() 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



  1   2   3   4   >