You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cordova.apache.org by ogoguel <gi...@git.apache.org> on 2015/06/12 08:58:28 UTC

[GitHub] cordova-lib pull request: CB-9162 - Default Variables

GitHub user ogoguel opened a pull request:

    https://github.com/apache/cordova-lib/pull/247

    CB-9162 - Default Variables

    

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

    $ git pull https://github.com/ogoguel/cordova-lib variables

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

    https://github.com/apache/cordova-lib/pull/247.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 #247
    
----
commit 14e225be9eabae899796e63b6c1925bc7aab7e0d
Author: Olivier Goguel <og...@macbook-pro-de-olivier-bto.local>
Date:   2015-06-12T06:57:02Z

    CB-9162 - Default Variables

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] cordova-lib pull request: CB-9162 - Default Variables

Posted by TimBarham <gi...@git.apache.org>.
Github user TimBarham commented on the pull request:

    https://github.com/apache/cordova-lib/pull/247#issuecomment-112186148
  
    Thanks @ogoguel! A couple of things need fixing - I'll merge this once they're sorted.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] cordova-lib pull request: CB-9162 - Default Variables

Posted by ogoguel <gi...@git.apache.org>.
Github user ogoguel commented on the pull request:

    https://github.com/apache/cordova-lib/pull/247#issuecomment-112212662
  
    Fixed requiredVariables
    Fixed whitespace in plugin.xml



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] cordova-lib pull request: CB-9162 - Default Variables

Posted by TimBarham <gi...@git.apache.org>.
Github user TimBarham commented on a diff in the pull request:

    https://github.com/apache/cordova-lib/pull/247#discussion_r32479018
  
    --- Diff: cordova-lib/src/cordova/plugin.js ---
    @@ -168,10 +168,19 @@ module.exports = function plugin(command, targets, opts) {
                         })
                         .then(function(dir) {
                             // Validate top-level required variables
    -                        var pluginVariables = pluginInfoProvider.get(dir).getPreferences(),
    -                            missingVariables = pluginVariables.filter(function (v) {
    +                        var pluginVariables = pluginInfoProvider.get(dir).getPreferences();
    +                        var requiredVariables = [];
    +                        for(var i in pluginVariables)
    +                        {
    +                            var v = pluginVariables[i];
    +                            // discard variables with default value
    +                            if (!v.default)
    --- End diff --
    
    Have a look in `spec-cordova/plugin.spec.js`. You should be able to add something there. Create a `plugin.xml` with the required bits and add it under `spec-cordova/fixtures/plugins`.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] cordova-lib pull request: CB-9162 - Default Variables

Posted by TimBarham <gi...@git.apache.org>.
Github user TimBarham commented on a diff in the pull request:

    https://github.com/apache/cordova-lib/pull/247#discussion_r35482469
  
    --- Diff: cordova-lib/spec-plugman/plugins/org.test.defaultvariables/plugin.xml ---
    @@ -0,0 +1,16 @@
    +<?xml version="1.0" encoding="UTF-8"?>
    +
    +<plugin xmlns="http://cordova.apache.org/ns/plugins/1.0"
    +    xmlns:android="http://schemas.android.com/apk/res/android"
    +    id="org.test.defaultvariables"
    +    version="3.0.0">
    +
    +    <name>Use Default Variables</name>
    +
    +    <!-- android -->
    +    <platform name="android">
    +            <preference name="ENABLE_LOG" default="yes" />
    +            <preference name="API_KEY" />
    +    </platform>
    --- End diff --
    
    If you are creating a plugin, and need to use variables because some values will need to be defined by the app developer, you *use* the variable by including a reference to it in values in your plugin's `plugin.xml`. The value of the variable (whether the default value specified in the `preference` element, or a value supplied by the app developer when the plugin is added to their project) is substituted for references to the variable in values in `plugin.xml` when those values are used. See [here](https://cordova.apache.org/docs/en/5.1.1/plugin_ref_spec.md.html#variables) for some more info.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] cordova-lib pull request: CB-9162 - Default Variables

Posted by TimBarham <gi...@git.apache.org>.
Github user TimBarham commented on the pull request:

    https://github.com/apache/cordova-lib/pull/247#issuecomment-112552487
  
    Thanks for the update, @ogoguel. Squashed and 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] cordova-lib pull request: CB-9162 - Default Variables

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/cordova-lib/pull/247


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] cordova-lib pull request: CB-9162 - Default Variables

Posted by ogoguel <gi...@git.apache.org>.
Github user ogoguel commented on the pull request:

    https://github.com/apache/cordova-lib/pull/247#issuecomment-112522204
  
    Added missing test for variables parsing


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] cordova-lib pull request: CB-9162 - Default Variables

Posted by fujunwei <gi...@git.apache.org>.
Github user fujunwei commented on a diff in the pull request:

    https://github.com/apache/cordova-lib/pull/247#discussion_r35388232
  
    --- Diff: cordova-lib/spec-plugman/plugins/org.test.defaultvariables/plugin.xml ---
    @@ -0,0 +1,16 @@
    +<?xml version="1.0" encoding="UTF-8"?>
    +
    +<plugin xmlns="http://cordova.apache.org/ns/plugins/1.0"
    +    xmlns:android="http://schemas.android.com/apk/res/android"
    +    id="org.test.defaultvariables"
    +    version="3.0.0">
    +
    +    <name>Use Default Variables</name>
    +
    +    <!-- android -->
    +    <platform name="android">
    +            <preference name="ENABLE_LOG" default="yes" />
    +            <preference name="API_KEY" />
    +    </platform>
    --- End diff --
    
    I only found the default `ENABLE_LOG` in `cordova/plugins/android.json`, but there is no similar preference `<preference name="ENABLE_LOG" value="yes" />` in config.xml, how to use the Default variable.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] cordova-lib pull request: CB-9162 - Default Variables

Posted by TimBarham <gi...@git.apache.org>.
Github user TimBarham commented on a diff in the pull request:

    https://github.com/apache/cordova-lib/pull/247#discussion_r32458071
  
    --- Diff: cordova-lib/spec-plugman/install-browserify.spec.js ---
    @@ -46,6 +46,7 @@ var install = require('../src/plugman/install'),
             'com.cordova.engine-android' : path.join(plugins_dir, 'com.cordova.engine-android'),
             'org.test.plugins.childbrowser' : path.join(plugins_dir, 'org.test.plugins.childbrowser'),
             'com.adobe.vars' : path.join(plugins_dir, 'com.adobe.vars'),
    +        'org.test.defaultvariables' : path.join(plugins_dir, 'org.test.defaultvariables'),
    --- End diff --
    
    @ogoguel - could you make the equivalent change to `install.spec.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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] cordova-lib pull request: CB-9162 - Default Variables

Posted by ogoguel <gi...@git.apache.org>.
Github user ogoguel commented on a diff in the pull request:

    https://github.com/apache/cordova-lib/pull/247#discussion_r35457112
  
    --- Diff: cordova-lib/spec-plugman/plugins/org.test.defaultvariables/plugin.xml ---
    @@ -0,0 +1,16 @@
    +<?xml version="1.0" encoding="UTF-8"?>
    +
    +<plugin xmlns="http://cordova.apache.org/ns/plugins/1.0"
    +    xmlns:android="http://schemas.android.com/apk/res/android"
    +    id="org.test.defaultvariables"
    +    version="3.0.0">
    +
    +    <name>Use Default Variables</name>
    +
    +    <!-- android -->
    +    <platform name="android">
    +            <preference name="ENABLE_LOG" default="yes" />
    +            <preference name="API_KEY" />
    +    </platform>
    --- End diff --
    
    If you want to override the default variable, just specify it during the plugin installation : 
    ex :` cordova plugin add <plugin_id> --variable ENABLE_LOG=no`  
    This will update the `cordova/plugins/android.json`accordingly, as this is where the variables are being stored (and not in the config.xml)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] cordova-lib pull request: CB-9162 - Default Variables

Posted by ogoguel <gi...@git.apache.org>.
Github user ogoguel commented on a diff in the pull request:

    https://github.com/apache/cordova-lib/pull/247#discussion_r32466926
  
    --- Diff: cordova-lib/src/cordova/plugin.js ---
    @@ -168,10 +168,19 @@ module.exports = function plugin(command, targets, opts) {
                         })
                         .then(function(dir) {
                             // Validate top-level required variables
    -                        var pluginVariables = pluginInfoProvider.get(dir).getPreferences(),
    -                            missingVariables = pluginVariables.filter(function (v) {
    +                        var pluginVariables = pluginInfoProvider.get(dir).getPreferences();
    +                        var requiredVariables = [];
    +                        for(var i in pluginVariables)
    +                        {
    +                            var v = pluginVariables[i];
    +                            // discard variables with default value
    +                            if (!v.default)
    --- End diff --
    
    Beside adding a plugin, how this line of code can be unit-tested ?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] cordova-lib pull request: CB-9162 - Default Variables

Posted by TimBarham <gi...@git.apache.org>.
Github user TimBarham commented on a diff in the pull request:

    https://github.com/apache/cordova-lib/pull/247#discussion_r32458159
  
    --- Diff: cordova-lib/spec-plugman/plugins/org.test.defaultvariables/plugin.xml ---
    @@ -0,0 +1,18 @@
    +<?xml version="1.0" encoding="UTF-8"?>
    +
    +
    +<plugin xmlns="http://cordova.apache.org/ns/plugins/1.0"
    +    xmlns:android="http://schemas.android.com/apk/res/android"
    +    id="org.test.defaultvariables"
    +    version="3.0.0">
    +
    +    <name>Use Default Variables</name>
    +
    +    <!-- android -->
    +    <platform name="android">
    +		   <preference name="ENABLE_LOG" default="yes" />
    +            <preference name="API_KEY" />
    +    </platform>
    +
    +   
    +</plugin>
    --- End diff --
    
    Clean up whitespace (mixture of tabs and 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] cordova-lib pull request: CB-9162 - Default Variables

Posted by TimBarham <gi...@git.apache.org>.
Github user TimBarham commented on a diff in the pull request:

    https://github.com/apache/cordova-lib/pull/247#discussion_r32458351
  
    --- Diff: cordova-lib/src/cordova/plugin.js ---
    @@ -168,10 +168,19 @@ module.exports = function plugin(command, targets, opts) {
                         })
                         .then(function(dir) {
                             // Validate top-level required variables
    -                        var pluginVariables = pluginInfoProvider.get(dir).getPreferences(),
    -                            missingVariables = pluginVariables.filter(function (v) {
    +                        var pluginVariables = pluginInfoProvider.get(dir).getPreferences();
    +                        var requiredVariables = [];
    +                        for(var i in pluginVariables)
    +                        {
    +                            var v = pluginVariables[i];
    +                            // discard variables with default value
    +                            if (!v.default)
    --- End diff --
    
    When I test this adding a plugin that requires variables (for example, `com.phonegap.plugins.facebookconnect`) and I don't specify any, this line throws (`v` is `null`). The logic here doesn't seem to coincide with the actual value returned by `getPreferences()`.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] cordova-lib pull request: CB-9162 - Default Variables

Posted by fujunwei <gi...@git.apache.org>.
Github user fujunwei commented on a diff in the pull request:

    https://github.com/apache/cordova-lib/pull/247#discussion_r35481753
  
    --- Diff: cordova-lib/spec-plugman/plugins/org.test.defaultvariables/plugin.xml ---
    @@ -0,0 +1,16 @@
    +<?xml version="1.0" encoding="UTF-8"?>
    +
    +<plugin xmlns="http://cordova.apache.org/ns/plugins/1.0"
    +    xmlns:android="http://schemas.android.com/apk/res/android"
    +    id="org.test.defaultvariables"
    +    version="3.0.0">
    +
    +    <name>Use Default Variables</name>
    +
    +    <!-- android -->
    +    <platform name="android">
    +            <preference name="ENABLE_LOG" default="yes" />
    +            <preference name="API_KEY" />
    +    </platform>
    --- End diff --
    
    How to use the variable updated in the `cordova/plugins/android.json`.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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