[GitHub] cordova-android pull request: CB-9149: Make gradle alias subprojec...
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...
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...
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...
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 ...
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
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
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
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
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
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
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
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
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
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...
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...
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...
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...
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...
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...
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...
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
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...
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
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....
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....
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....
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....
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
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...
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...
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...
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
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...
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...
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...
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...
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...
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
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...
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...
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...
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
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...
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...
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
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 ...
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 ...
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 ...
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 ...
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 ...
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
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 ...
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 ...
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
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...
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...
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...
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 ...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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 ...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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...
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
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...
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...
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...
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...
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