Uploaded image for project: 'Apache Cordova'
  1. Apache Cordova
  2. CB-11023

plugin.xml should be able to add attribute to an existing element in AndroidManifest.xml

    Details

    • Type: Improvement
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: None
    • Labels:
      None

      Description

      We have a need for a plugin we develop to add the attribute android:name to the <application> element to be able to point to a Class that tracks app sessions for analytics.

      plugin.xml only support via <config-file> only to add elements to AndroidManfist.xml this request is to add an enhancement to also handle adding an attribute to an existing element.

        Issue Links

          Activity

          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user asfgit closed the pull request at:

          https://github.com/apache/cordova-docs/pull/614

          Show
          githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/cordova-docs/pull/614
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit af2ede15180d202e59d576f8f44e967018a268d1 in cordova-docs's branch refs/heads/master from Karen Tran
          [ https://git-wip-us.apache.org/repos/asf?p=cordova-docs.git;h=af2ede1 ]

          CB-11023 New edit-config tag in plugin.xml

          This closes #614

          Show
          jira-bot ASF subversion and git services added a comment - Commit af2ede15180d202e59d576f8f44e967018a268d1 in cordova-docs's branch refs/heads/master from Karen Tran [ https://git-wip-us.apache.org/repos/asf?p=cordova-docs.git;h=af2ede1 ] CB-11023 New edit-config tag in plugin.xml This closes #614
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user asfgit closed the pull request at:

          https://github.com/apache/cordova-cli/pull/258

          Show
          githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/cordova-cli/pull/258
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 83c4c6bcc04bd5b4d7c22971010b8ba7e9ac7893 in cordova-cli's branch refs/heads/master from Karen Tran
          [ https://git-wip-us.apache.org/repos/asf?p=cordova-cli.git;h=83c4c6b ]

          CB-11023 Add doc for conflicting plugins

          Show
          jira-bot ASF subversion and git services added a comment - Commit 83c4c6bcc04bd5b4d7c22971010b8ba7e9ac7893 in cordova-cli's branch refs/heads/master from Karen Tran [ https://git-wip-us.apache.org/repos/asf?p=cordova-cli.git;h=83c4c6b ] CB-11023 Add doc for conflicting plugins
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user riknoll commented on the issue:

          https://github.com/apache/cordova-docs/pull/614

          @macdonst any chance you can take over merging this? Been sick and swamped this week and I don't think I'll have a chance to test out the site before merging it in.

          Show
          githubbot ASF GitHub Bot added a comment - Github user riknoll commented on the issue: https://github.com/apache/cordova-docs/pull/614 @macdonst any chance you can take over merging this? Been sick and swamped this week and I don't think I'll have a chance to test out the site before merging it in.
          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user ktop opened a pull request:

          https://github.com/apache/cordova-cli/pull/258

          CB-11023 Add doc for conflicting plugins

          Added a section under plugins for dealing with plugin conflicts involving the new edit-config tag. References the plugin.xml guide for more information and examples.

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

          $ git pull https://github.com/ktop/cordova-cli cb-11023

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

          https://github.com/apache/cordova-cli/pull/258.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 #258


          commit 83c4c6bcc04bd5b4d7c22971010b8ba7e9ac7893
          Author: ktop <ktop500@gmail.com>
          Date: 2016-07-18T18:29:00Z

          CB-11023 Add doc for conflicting plugins


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user ktop opened a pull request: https://github.com/apache/cordova-cli/pull/258 CB-11023 Add doc for conflicting plugins Added a section under plugins for dealing with plugin conflicts involving the new edit-config tag. References the plugin.xml guide for more information and examples. You can merge this pull request into a Git repository by running: $ git pull https://github.com/ktop/cordova-cli cb-11023 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/cordova-cli/pull/258.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 #258 commit 83c4c6bcc04bd5b4d7c22971010b8ba7e9ac7893 Author: ktop <ktop500@gmail.com> Date: 2016-07-18T18:29:00Z CB-11023 Add doc for conflicting plugins
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user ktop commented on the issue:

          https://github.com/apache/cordova-docs/pull/614

          @riknoll I've made the changes here and will open a PR in the CLI doc to mention the plugin conflicts and link to the plugin.xml doc for reference.

          Show
          githubbot ASF GitHub Bot added a comment - Github user ktop commented on the issue: https://github.com/apache/cordova-docs/pull/614 @riknoll I've made the changes here and will open a PR in the CLI doc to mention the plugin conflicts and link to the plugin.xml doc for reference.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user riknoll commented on the issue:

          https://github.com/apache/cordova-docs/pull/614

          That make sense to me. I added a few more comments, but otherwise LGTM.

          Show
          githubbot ASF GitHub Bot added a comment - Github user riknoll commented on the issue: https://github.com/apache/cordova-docs/pull/614 That make sense to me. I added a few more comments, but otherwise LGTM.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user riknoll commented on a diff in the pull request:

          https://github.com/apache/cordova-docs/pull/614#discussion_r70540948

          — Diff: www/docs/en/dev/plugin_ref/spec.md —
          @@ -365,6 +365,111 @@ For windows-specific attributes:
          ```
          The above example will set pre-8.1 platforms (Windows 8, specifically) to require the `webcam` device capability and the `picturesLibrary` general capability, and apply the `webcam` device capability only to Windows 8.1 projects that build for Windows Phone. Windows desktop 8.1 systems are unmodified.

          +### edit-config
          +Similar to `config-file`, `edit-config` identifies an XML-based configuration file to be modified, where in that document the modification should take place, and what should be modified. Instead of appending new children to an XML document tree, `edit-config` makes modifications to attributes of XML elements. There are two modes which will determine what type of attribute modification will be made, `merge` or `overwrite`. `edit-config` has one child and that child will contain the attributes to be added.
          +
          +Attributes(type) <br/> <span class="sub-header">Only for platform:</span> | Description
          +---------------- | ------------
          +file(string) | The file to be modified, and the path relative to the root of the Cordova project. If the specified file does not exist, the tool ignores the configuration change and continues installation. <br/> The target can include wildcard (`*`) elements. In this case, the CLI recursively searches through the project directory structure and uses the first match. <br/> On iOS, the location of configuration files relative to the project directory root is not known, so specifying a target of `config.xml` resolves to `cordova-ios-project/MyAppName/config.xml`.
          +target(string) | An XPath selector referencing the target element to make attribute modifications to. If you use absolute selectors, you can use a wildcard (``) to specify the root element, e.g., `//plugins`. If the selector does not resolve to a child of the specified document, the tool stops and reverses the installation process, issues a warning, and exits with a non-zero code.
          +mode(string) | The mode that determines what type of attribute modifications will be made. <br/> `merge` - Adds the specified attributes to the target element. Will replace the attribute values if the specified attributes already exist in the target element. <br/> `overwrite` - Replaces all attributes in the target element with the attributes specified.
          +
          +Example:
          +
          +```xml
          +<!-- plugin-1 -->
          +<edit-config file="AndroidManifest.xml" target="/manifest/uses-sdk" mode="merge">
          + <uses-sdk android:minSdkVersion="16" android:maxSdkVersion="23" />
          +</edit-config>
          +<edit-config file="AndroidManifest.xml" target="/manifest/application/activity[@android:name='MainActivity']" mode="overwrite">
          + <activity android:name="MainActivity" android:label="NewLabel" android:configChanges="orientation|keyboardHidden" />
          +</edit-config>
          +```
          +
          +AndroidManifest.xml before adding plugin-1:
          +```xml
          +<manifest android:hardwareAccelerated="true" android:versionCode="1" android:versionName="0.0.1" package="io.cordova.hellocordova" xmlns:android="http://schemas.android.com/apk/res/android">
          + ...
          + <activity android:configChanges="orientation|keyboardHidden|keyboard|screenSize|locale" android:label="@string/activity_name" android:launchMode="singleTop" android:name="MainActivity" android:theme="@android:style/Theme.DeviceDefault.NoActionBar" android:windowSoftInputMode="adjustResize">
          + ...
          + </activity>
          + ...
          + <uses-sdk android:minSdkVersion="14" android:targetSdkVersion="23" />
          +</manifest>
          +```
          +
          +AndroidManifest.xml after adding plugin-1:
          +```xml
          +<manifest android:hardwareAccelerated="true" android:versionCode="1" android:versionName="0.0.1" package="io.cordova.hellocordova" xmlns:android="http://schemas.android.com/apk/res/android">
          + ...
          + <activity android:configChanges="orientation|keyboardHidden" android:label="NewLabel" android:name="MainActivity">
          + ...
          + </activity>
          + ...
          + <uses-sdk android:maxSdkVersion="23" android:minSdkVersion="16" android:targetSdkVersion="23" />
          +</manifest>
          +```
          +
          +Multiple plugins will not be able to to modify the same attributes because it may cause problems with the application. An error will be thrown and plugin install will fail. The conflicting `edit-config` tags must be resolved before the plugin can be added. Make modifications to conflicting tags to resolve the conflict, then remove and re-add those updated plugins.
          +
          +There is an option for those who are certain that the plugin should be installed despite the conflicts. The `-force` flag can be used with `cordova plugin add`. Force adding the plugin will revert conflicting changes of other plugins so that it can be added without issues. `-force` should be used with caution as reverting changes of other plugins may cause the application to not work as expected.
          +
          +If the plugins ever get in a weird state, remove all plugins and re-add them.
          +
          +Example:
          +
          +Assume plugin-1 from above is already installed. Trying to install plugin-2 below will cause an error because plugin-1 has modified `uses-sdk` element in AndroidManifest.xml already.
          +
          +```xml
          +<!-- plugin-2 -->
          +<edit-config file="AndroidManifest.xml" target="/manifest/uses-sdk" mode="merge">
          + <uses-sdk android:minSdkVersion="15" />
          +</edit-config>
          +```
          +
          +There are a couple ways plugin-2 can be added:
          +
          +One possible resolution of the conflict is to remove the `edit-config` tag from plugin-2 and merge it into plugin-1's `edit-config` tag (assuming plugin-1 has no issue with this change). Remove both plugins and re-add them with these changes. plugin-2 should be added cleanly this time.
          +
          +Removing `edit-config` from plugin-2 and merging it into plugin-1:
          +```xml
          +<!-- plugin-1 -->
          +<edit-config file="AndroidManifest.xml" target="/manifest/uses-sdk" mode="merge">
          + <uses-sdk android:minSdkVersion="15" android:maxSdkVersion="23" />
          +</edit-config>
          +<edit-config file="AndroidManifest.xml" target="/manifest/application/activity[@android:name='MainActivity']" mode="overwrite">
          + <activity android:name="MainActivity" android:label="NewLabel" android:configChanges="orientation|keyboardHidden" />
          +</edit-config>
          +```
          +
          +The resulting AndroidManifest.xml after removing and re-adding both plugins:
          +```xml
          +<manifest android:hardwareAccelerated="true" android:versionCode="1" android:versionName="0.0.1" package="io.cordova.hellocordova" xmlns:android="http://schemas.android.com/apk/res/android">
          + ...
          + <activity android:configChanges="orientation|keyboardHidden" android:label="NewLabel" android:name="MainActivity">
          + ...
          + </activity>
          + ...
          + <uses-sdk android:maxSdkVersion="23" android:minSdkVersion="15" android:targetSdkVersion="23" />
          +</manifest>
          +```
          +
          +The second way to add plugin-2 involves adding the plugin with `--force`. The conflicting `edit-config` change from plugin-1 will be reverted and plugin-2's change will be applied. The resulting AndroidManifest.xml will have the `uses-sdk` change from plugin-2 and the `activity` change from plugin-1. Notice only the `uses-sdk` change from plugin-1 is gone since it was the only conflicting change.
          +
          +The resulting AndroidManifest.xml after force adding plugin-2:
          +```xml
          +<manifest android:hardwareAccelerated="true" android:versionCode="1" android:versionName="0.0.1" package="io.cordova.hellocordova" xmlns:android="http://schemas.android.com/apk/res/android">
          + ...
          + <activity android:configChanges="orientation|keyboardHidden" android:label="NewLabel" android:name="MainActivity">
          + ...
          + </activity>
          + ...
          + <uses-sdk android:minSdkVersion="15" android:targetSdkVersion="23" />
          +</manifest>
          +```
          +
          +Note: Reverted changes from `--force` are gone for good. They will not reappear after removing the plugin that was force added. If the reverted changes are needed, the associated plugins should be removed and re-added.
          — End diff –

          "the associated plugins" => "all associated plugins"

          Show
          githubbot ASF GitHub Bot added a comment - Github user riknoll commented on a diff in the pull request: https://github.com/apache/cordova-docs/pull/614#discussion_r70540948 — Diff: www/docs/en/dev/plugin_ref/spec.md — @@ -365,6 +365,111 @@ For windows-specific attributes: ``` The above example will set pre-8.1 platforms (Windows 8, specifically) to require the `webcam` device capability and the `picturesLibrary` general capability, and apply the `webcam` device capability only to Windows 8.1 projects that build for Windows Phone. Windows desktop 8.1 systems are unmodified. +### edit-config +Similar to `config-file`, `edit-config` identifies an XML-based configuration file to be modified, where in that document the modification should take place, and what should be modified. Instead of appending new children to an XML document tree, `edit-config` makes modifications to attributes of XML elements. There are two modes which will determine what type of attribute modification will be made, `merge` or `overwrite`. `edit-config` has one child and that child will contain the attributes to be added. + +Attributes(type) <br/> <span class="sub-header">Only for platform:</span> | Description +---------------- | ------------ +file(string) | The file to be modified, and the path relative to the root of the Cordova project. If the specified file does not exist, the tool ignores the configuration change and continues installation. <br/> The target can include wildcard (`*`) elements. In this case, the CLI recursively searches through the project directory structure and uses the first match. <br/> On iOS, the location of configuration files relative to the project directory root is not known, so specifying a target of `config.xml` resolves to `cordova-ios-project/MyAppName/config.xml`. +target(string) | An XPath selector referencing the target element to make attribute modifications to. If you use absolute selectors, you can use a wildcard (` `) to specify the root element, e.g., `/ /plugins`. If the selector does not resolve to a child of the specified document, the tool stops and reverses the installation process, issues a warning, and exits with a non-zero code. +mode(string) | The mode that determines what type of attribute modifications will be made. <br/> `merge` - Adds the specified attributes to the target element. Will replace the attribute values if the specified attributes already exist in the target element. <br/> `overwrite` - Replaces all attributes in the target element with the attributes specified. + +Example: + +```xml +<!-- plugin-1 --> +<edit-config file="AndroidManifest.xml" target="/manifest/uses-sdk" mode="merge"> + <uses-sdk android:minSdkVersion="16" android:maxSdkVersion="23" /> +</edit-config> +<edit-config file="AndroidManifest.xml" target="/manifest/application/activity [@android:name='MainActivity'] " mode="overwrite"> + <activity android:name="MainActivity" android:label="NewLabel" android:configChanges="orientation|keyboardHidden" /> +</edit-config> +``` + +AndroidManifest.xml before adding plugin-1: +```xml +<manifest android:hardwareAccelerated="true" android:versionCode="1" android:versionName="0.0.1" package="io.cordova.hellocordova" xmlns:android="http://schemas.android.com/apk/res/android"> + ... + <activity android:configChanges="orientation|keyboardHidden|keyboard|screenSize|locale" android:label="@string/activity_name" android:launchMode="singleTop" android:name="MainActivity" android:theme="@android:style/Theme.DeviceDefault.NoActionBar" android:windowSoftInputMode="adjustResize"> + ... + </activity> + ... + <uses-sdk android:minSdkVersion="14" android:targetSdkVersion="23" /> +</manifest> +``` + +AndroidManifest.xml after adding plugin-1: +```xml +<manifest android:hardwareAccelerated="true" android:versionCode="1" android:versionName="0.0.1" package="io.cordova.hellocordova" xmlns:android="http://schemas.android.com/apk/res/android"> + ... + <activity android:configChanges="orientation|keyboardHidden" android:label="NewLabel" android:name="MainActivity"> + ... + </activity> + ... + <uses-sdk android:maxSdkVersion="23" android:minSdkVersion="16" android:targetSdkVersion="23" /> +</manifest> +``` + +Multiple plugins will not be able to to modify the same attributes because it may cause problems with the application. An error will be thrown and plugin install will fail. The conflicting `edit-config` tags must be resolved before the plugin can be added. Make modifications to conflicting tags to resolve the conflict, then remove and re-add those updated plugins. + +There is an option for those who are certain that the plugin should be installed despite the conflicts. The `- force` flag can be used with `cordova plugin add`. Force adding the plugin will revert conflicting changes of other plugins so that it can be added without issues. ` -force` should be used with caution as reverting changes of other plugins may cause the application to not work as expected. + +If the plugins ever get in a weird state, remove all plugins and re-add them. + +Example: + +Assume plugin-1 from above is already installed. Trying to install plugin-2 below will cause an error because plugin-1 has modified `uses-sdk` element in AndroidManifest.xml already. + +```xml +<!-- plugin-2 --> +<edit-config file="AndroidManifest.xml" target="/manifest/uses-sdk" mode="merge"> + <uses-sdk android:minSdkVersion="15" /> +</edit-config> +``` + +There are a couple ways plugin-2 can be added: + +One possible resolution of the conflict is to remove the `edit-config` tag from plugin-2 and merge it into plugin-1's `edit-config` tag (assuming plugin-1 has no issue with this change). Remove both plugins and re-add them with these changes. plugin-2 should be added cleanly this time. + +Removing `edit-config` from plugin-2 and merging it into plugin-1: +```xml +<!-- plugin-1 --> +<edit-config file="AndroidManifest.xml" target="/manifest/uses-sdk" mode="merge"> + <uses-sdk android:minSdkVersion="15" android:maxSdkVersion="23" /> +</edit-config> +<edit-config file="AndroidManifest.xml" target="/manifest/application/activity [@android:name='MainActivity'] " mode="overwrite"> + <activity android:name="MainActivity" android:label="NewLabel" android:configChanges="orientation|keyboardHidden" /> +</edit-config> +``` + +The resulting AndroidManifest.xml after removing and re-adding both plugins: +```xml +<manifest android:hardwareAccelerated="true" android:versionCode="1" android:versionName="0.0.1" package="io.cordova.hellocordova" xmlns:android="http://schemas.android.com/apk/res/android"> + ... + <activity android:configChanges="orientation|keyboardHidden" android:label="NewLabel" android:name="MainActivity"> + ... + </activity> + ... + <uses-sdk android:maxSdkVersion="23" android:minSdkVersion="15" android:targetSdkVersion="23" /> +</manifest> +``` + +The second way to add plugin-2 involves adding the plugin with `--force`. The conflicting `edit-config` change from plugin-1 will be reverted and plugin-2's change will be applied. The resulting AndroidManifest.xml will have the `uses-sdk` change from plugin-2 and the `activity` change from plugin-1. Notice only the `uses-sdk` change from plugin-1 is gone since it was the only conflicting change. + +The resulting AndroidManifest.xml after force adding plugin-2: +```xml +<manifest android:hardwareAccelerated="true" android:versionCode="1" android:versionName="0.0.1" package="io.cordova.hellocordova" xmlns:android="http://schemas.android.com/apk/res/android"> + ... + <activity android:configChanges="orientation|keyboardHidden" android:label="NewLabel" android:name="MainActivity"> + ... + </activity> + ... + <uses-sdk android:minSdkVersion="15" android:targetSdkVersion="23" /> +</manifest> +``` + +Note: Reverted changes from `--force` are gone for good. They will not reappear after removing the plugin that was force added. If the reverted changes are needed, the associated plugins should be removed and re-added. — End diff – "the associated plugins" => "all associated plugins"
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user riknoll commented on a diff in the pull request:

          https://github.com/apache/cordova-docs/pull/614#discussion_r70540822

          — Diff: www/docs/en/dev/plugin_ref/spec.md —
          @@ -365,6 +365,111 @@ For windows-specific attributes:
          ```
          The above example will set pre-8.1 platforms (Windows 8, specifically) to require the `webcam` device capability and the `picturesLibrary` general capability, and apply the `webcam` device capability only to Windows 8.1 projects that build for Windows Phone. Windows desktop 8.1 systems are unmodified.

          +### edit-config
          +Similar to `config-file`, `edit-config` identifies an XML-based configuration file to be modified, where in that document the modification should take place, and what should be modified. Instead of appending new children to an XML document tree, `edit-config` makes modifications to attributes of XML elements. There are two modes which will determine what type of attribute modification will be made, `merge` or `overwrite`. `edit-config` has one child and that child will contain the attributes to be added.
          +
          +Attributes(type) <br/> <span class="sub-header">Only for platform:</span> | Description
          +---------------- | ------------
          +file(string) | The file to be modified, and the path relative to the root of the Cordova project. If the specified file does not exist, the tool ignores the configuration change and continues installation. <br/> The target can include wildcard (`*`) elements. In this case, the CLI recursively searches through the project directory structure and uses the first match. <br/> On iOS, the location of configuration files relative to the project directory root is not known, so specifying a target of `config.xml` resolves to `cordova-ios-project/MyAppName/config.xml`.
          +target(string) | An XPath selector referencing the target element to make attribute modifications to. If you use absolute selectors, you can use a wildcard (``) to specify the root element, e.g., `//plugins`. If the selector does not resolve to a child of the specified document, the tool stops and reverses the installation process, issues a warning, and exits with a non-zero code.
          +mode(string) | The mode that determines what type of attribute modifications will be made. <br/> `merge` - Adds the specified attributes to the target element. Will replace the attribute values if the specified attributes already exist in the target element. <br/> `overwrite` - Replaces all attributes in the target element with the attributes specified.
          +
          +Example:
          +
          +```xml
          +<!-- plugin-1 -->
          +<edit-config file="AndroidManifest.xml" target="/manifest/uses-sdk" mode="merge">
          + <uses-sdk android:minSdkVersion="16" android:maxSdkVersion="23" />
          +</edit-config>
          +<edit-config file="AndroidManifest.xml" target="/manifest/application/activity[@android:name='MainActivity']" mode="overwrite">
          + <activity android:name="MainActivity" android:label="NewLabel" android:configChanges="orientation|keyboardHidden" />
          +</edit-config>
          +```
          +
          +AndroidManifest.xml before adding plugin-1:
          +```xml
          +<manifest android:hardwareAccelerated="true" android:versionCode="1" android:versionName="0.0.1" package="io.cordova.hellocordova" xmlns:android="http://schemas.android.com/apk/res/android">
          + ...
          + <activity android:configChanges="orientation|keyboardHidden|keyboard|screenSize|locale" android:label="@string/activity_name" android:launchMode="singleTop" android:name="MainActivity" android:theme="@android:style/Theme.DeviceDefault.NoActionBar" android:windowSoftInputMode="adjustResize">
          + ...
          + </activity>
          + ...
          + <uses-sdk android:minSdkVersion="14" android:targetSdkVersion="23" />
          +</manifest>
          +```
          +
          +AndroidManifest.xml after adding plugin-1:
          +```xml
          +<manifest android:hardwareAccelerated="true" android:versionCode="1" android:versionName="0.0.1" package="io.cordova.hellocordova" xmlns:android="http://schemas.android.com/apk/res/android">
          + ...
          + <activity android:configChanges="orientation|keyboardHidden" android:label="NewLabel" android:name="MainActivity">
          + ...
          + </activity>
          + ...
          + <uses-sdk android:maxSdkVersion="23" android:minSdkVersion="16" android:targetSdkVersion="23" />
          +</manifest>
          +```
          +
          +Multiple plugins will not be able to to modify the same attributes because it may cause problems with the application. An error will be thrown and plugin install will fail. The conflicting `edit-config` tags must be resolved before the plugin can be added. Make modifications to conflicting tags to resolve the conflict, then remove and re-add those updated plugins.
          +
          +There is an option for those who are certain that the plugin should be installed despite the conflicts. The `-force` flag can be used with `cordova plugin add`. Force adding the plugin will revert conflicting changes of other plugins so that it can be added without issues. `-force` should be used with caution as reverting changes of other plugins may cause the application to not work as expected.
          +
          +If the plugins ever get in a weird state, remove all plugins and re-add them.
          +
          +Example:
          — End diff –

          Maybe make this a sub-header?

          Show
          githubbot ASF GitHub Bot added a comment - Github user riknoll commented on a diff in the pull request: https://github.com/apache/cordova-docs/pull/614#discussion_r70540822 — Diff: www/docs/en/dev/plugin_ref/spec.md — @@ -365,6 +365,111 @@ For windows-specific attributes: ``` The above example will set pre-8.1 platforms (Windows 8, specifically) to require the `webcam` device capability and the `picturesLibrary` general capability, and apply the `webcam` device capability only to Windows 8.1 projects that build for Windows Phone. Windows desktop 8.1 systems are unmodified. +### edit-config +Similar to `config-file`, `edit-config` identifies an XML-based configuration file to be modified, where in that document the modification should take place, and what should be modified. Instead of appending new children to an XML document tree, `edit-config` makes modifications to attributes of XML elements. There are two modes which will determine what type of attribute modification will be made, `merge` or `overwrite`. `edit-config` has one child and that child will contain the attributes to be added. + +Attributes(type) <br/> <span class="sub-header">Only for platform:</span> | Description +---------------- | ------------ +file(string) | The file to be modified, and the path relative to the root of the Cordova project. If the specified file does not exist, the tool ignores the configuration change and continues installation. <br/> The target can include wildcard (`*`) elements. In this case, the CLI recursively searches through the project directory structure and uses the first match. <br/> On iOS, the location of configuration files relative to the project directory root is not known, so specifying a target of `config.xml` resolves to `cordova-ios-project/MyAppName/config.xml`. +target(string) | An XPath selector referencing the target element to make attribute modifications to. If you use absolute selectors, you can use a wildcard (` `) to specify the root element, e.g., `/ /plugins`. If the selector does not resolve to a child of the specified document, the tool stops and reverses the installation process, issues a warning, and exits with a non-zero code. +mode(string) | The mode that determines what type of attribute modifications will be made. <br/> `merge` - Adds the specified attributes to the target element. Will replace the attribute values if the specified attributes already exist in the target element. <br/> `overwrite` - Replaces all attributes in the target element with the attributes specified. + +Example: + +```xml +<!-- plugin-1 --> +<edit-config file="AndroidManifest.xml" target="/manifest/uses-sdk" mode="merge"> + <uses-sdk android:minSdkVersion="16" android:maxSdkVersion="23" /> +</edit-config> +<edit-config file="AndroidManifest.xml" target="/manifest/application/activity [@android:name='MainActivity'] " mode="overwrite"> + <activity android:name="MainActivity" android:label="NewLabel" android:configChanges="orientation|keyboardHidden" /> +</edit-config> +``` + +AndroidManifest.xml before adding plugin-1: +```xml +<manifest android:hardwareAccelerated="true" android:versionCode="1" android:versionName="0.0.1" package="io.cordova.hellocordova" xmlns:android="http://schemas.android.com/apk/res/android"> + ... + <activity android:configChanges="orientation|keyboardHidden|keyboard|screenSize|locale" android:label="@string/activity_name" android:launchMode="singleTop" android:name="MainActivity" android:theme="@android:style/Theme.DeviceDefault.NoActionBar" android:windowSoftInputMode="adjustResize"> + ... + </activity> + ... + <uses-sdk android:minSdkVersion="14" android:targetSdkVersion="23" /> +</manifest> +``` + +AndroidManifest.xml after adding plugin-1: +```xml +<manifest android:hardwareAccelerated="true" android:versionCode="1" android:versionName="0.0.1" package="io.cordova.hellocordova" xmlns:android="http://schemas.android.com/apk/res/android"> + ... + <activity android:configChanges="orientation|keyboardHidden" android:label="NewLabel" android:name="MainActivity"> + ... + </activity> + ... + <uses-sdk android:maxSdkVersion="23" android:minSdkVersion="16" android:targetSdkVersion="23" /> +</manifest> +``` + +Multiple plugins will not be able to to modify the same attributes because it may cause problems with the application. An error will be thrown and plugin install will fail. The conflicting `edit-config` tags must be resolved before the plugin can be added. Make modifications to conflicting tags to resolve the conflict, then remove and re-add those updated plugins. + +There is an option for those who are certain that the plugin should be installed despite the conflicts. The `- force` flag can be used with `cordova plugin add`. Force adding the plugin will revert conflicting changes of other plugins so that it can be added without issues. ` -force` should be used with caution as reverting changes of other plugins may cause the application to not work as expected. + +If the plugins ever get in a weird state, remove all plugins and re-add them. + +Example: — End diff – Maybe make this a sub-header?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user riknoll commented on a diff in the pull request:

          https://github.com/apache/cordova-docs/pull/614#discussion_r70540748

          — Diff: www/docs/en/dev/plugin_ref/spec.md —
          @@ -365,6 +365,111 @@ For windows-specific attributes:
          ```
          The above example will set pre-8.1 platforms (Windows 8, specifically) to require the `webcam` device capability and the `picturesLibrary` general capability, and apply the `webcam` device capability only to Windows 8.1 projects that build for Windows Phone. Windows desktop 8.1 systems are unmodified.

          +### edit-config
          +Similar to `config-file`, `edit-config` identifies an XML-based configuration file to be modified, where in that document the modification should take place, and what should be modified. Instead of appending new children to an XML document tree, `edit-config` makes modifications to attributes of XML elements. There are two modes which will determine what type of attribute modification will be made, `merge` or `overwrite`. `edit-config` has one child and that child will contain the attributes to be added.
          +
          +Attributes(type) <br/> <span class="sub-header">Only for platform:</span> | Description
          +---------------- | ------------
          +file(string) | The file to be modified, and the path relative to the root of the Cordova project. If the specified file does not exist, the tool ignores the configuration change and continues installation. <br/> The target can include wildcard (`*`) elements. In this case, the CLI recursively searches through the project directory structure and uses the first match. <br/> On iOS, the location of configuration files relative to the project directory root is not known, so specifying a target of `config.xml` resolves to `cordova-ios-project/MyAppName/config.xml`.
          +target(string) | An XPath selector referencing the target element to make attribute modifications to. If you use absolute selectors, you can use a wildcard (``) to specify the root element, e.g., `//plugins`. If the selector does not resolve to a child of the specified document, the tool stops and reverses the installation process, issues a warning, and exits with a non-zero code.
          +mode(string) | The mode that determines what type of attribute modifications will be made. <br/> `merge` - Adds the specified attributes to the target element. Will replace the attribute values if the specified attributes already exist in the target element. <br/> `overwrite` - Replaces all attributes in the target element with the attributes specified.
          +
          +Example:
          +
          +```xml
          +<!-- plugin-1 -->
          +<edit-config file="AndroidManifest.xml" target="/manifest/uses-sdk" mode="merge">
          + <uses-sdk android:minSdkVersion="16" android:maxSdkVersion="23" />
          +</edit-config>
          +<edit-config file="AndroidManifest.xml" target="/manifest/application/activity[@android:name='MainActivity']" mode="overwrite">
          + <activity android:name="MainActivity" android:label="NewLabel" android:configChanges="orientation|keyboardHidden" />
          +</edit-config>
          +```
          +
          +AndroidManifest.xml before adding plugin-1:
          +```xml
          +<manifest android:hardwareAccelerated="true" android:versionCode="1" android:versionName="0.0.1" package="io.cordova.hellocordova" xmlns:android="http://schemas.android.com/apk/res/android">
          + ...
          + <activity android:configChanges="orientation|keyboardHidden|keyboard|screenSize|locale" android:label="@string/activity_name" android:launchMode="singleTop" android:name="MainActivity" android:theme="@android:style/Theme.DeviceDefault.NoActionBar" android:windowSoftInputMode="adjustResize">
          + ...
          + </activity>
          + ...
          + <uses-sdk android:minSdkVersion="14" android:targetSdkVersion="23" />
          +</manifest>
          +```
          +
          +AndroidManifest.xml after adding plugin-1:
          +```xml
          +<manifest android:hardwareAccelerated="true" android:versionCode="1" android:versionName="0.0.1" package="io.cordova.hellocordova" xmlns:android="http://schemas.android.com/apk/res/android">
          + ...
          + <activity android:configChanges="orientation|keyboardHidden" android:label="NewLabel" android:name="MainActivity">
          + ...
          + </activity>
          + ...
          + <uses-sdk android:maxSdkVersion="23" android:minSdkVersion="16" android:targetSdkVersion="23" />
          +</manifest>
          +```
          +
          +Multiple plugins will not be able to to modify the same attributes because it may cause problems with the application. An error will be thrown and plugin install will fail. The conflicting `edit-config` tags must be resolved before the plugin can be added. Make modifications to conflicting tags to resolve the conflict, then remove and re-add those updated plugins.
          — End diff –

          "Multiple plugins will not be able to to modify " => "Multiple plugins can not modify"

          "then remove and re-add those updated plugins" => "then remove and re-add the updated plugins"

          Show
          githubbot ASF GitHub Bot added a comment - Github user riknoll commented on a diff in the pull request: https://github.com/apache/cordova-docs/pull/614#discussion_r70540748 — Diff: www/docs/en/dev/plugin_ref/spec.md — @@ -365,6 +365,111 @@ For windows-specific attributes: ``` The above example will set pre-8.1 platforms (Windows 8, specifically) to require the `webcam` device capability and the `picturesLibrary` general capability, and apply the `webcam` device capability only to Windows 8.1 projects that build for Windows Phone. Windows desktop 8.1 systems are unmodified. +### edit-config +Similar to `config-file`, `edit-config` identifies an XML-based configuration file to be modified, where in that document the modification should take place, and what should be modified. Instead of appending new children to an XML document tree, `edit-config` makes modifications to attributes of XML elements. There are two modes which will determine what type of attribute modification will be made, `merge` or `overwrite`. `edit-config` has one child and that child will contain the attributes to be added. + +Attributes(type) <br/> <span class="sub-header">Only for platform:</span> | Description +---------------- | ------------ +file(string) | The file to be modified, and the path relative to the root of the Cordova project. If the specified file does not exist, the tool ignores the configuration change and continues installation. <br/> The target can include wildcard (`*`) elements. In this case, the CLI recursively searches through the project directory structure and uses the first match. <br/> On iOS, the location of configuration files relative to the project directory root is not known, so specifying a target of `config.xml` resolves to `cordova-ios-project/MyAppName/config.xml`. +target(string) | An XPath selector referencing the target element to make attribute modifications to. If you use absolute selectors, you can use a wildcard (` `) to specify the root element, e.g., `/ /plugins`. If the selector does not resolve to a child of the specified document, the tool stops and reverses the installation process, issues a warning, and exits with a non-zero code. +mode(string) | The mode that determines what type of attribute modifications will be made. <br/> `merge` - Adds the specified attributes to the target element. Will replace the attribute values if the specified attributes already exist in the target element. <br/> `overwrite` - Replaces all attributes in the target element with the attributes specified. + +Example: + +```xml +<!-- plugin-1 --> +<edit-config file="AndroidManifest.xml" target="/manifest/uses-sdk" mode="merge"> + <uses-sdk android:minSdkVersion="16" android:maxSdkVersion="23" /> +</edit-config> +<edit-config file="AndroidManifest.xml" target="/manifest/application/activity [@android:name='MainActivity'] " mode="overwrite"> + <activity android:name="MainActivity" android:label="NewLabel" android:configChanges="orientation|keyboardHidden" /> +</edit-config> +``` + +AndroidManifest.xml before adding plugin-1: +```xml +<manifest android:hardwareAccelerated="true" android:versionCode="1" android:versionName="0.0.1" package="io.cordova.hellocordova" xmlns:android="http://schemas.android.com/apk/res/android"> + ... + <activity android:configChanges="orientation|keyboardHidden|keyboard|screenSize|locale" android:label="@string/activity_name" android:launchMode="singleTop" android:name="MainActivity" android:theme="@android:style/Theme.DeviceDefault.NoActionBar" android:windowSoftInputMode="adjustResize"> + ... + </activity> + ... + <uses-sdk android:minSdkVersion="14" android:targetSdkVersion="23" /> +</manifest> +``` + +AndroidManifest.xml after adding plugin-1: +```xml +<manifest android:hardwareAccelerated="true" android:versionCode="1" android:versionName="0.0.1" package="io.cordova.hellocordova" xmlns:android="http://schemas.android.com/apk/res/android"> + ... + <activity android:configChanges="orientation|keyboardHidden" android:label="NewLabel" android:name="MainActivity"> + ... + </activity> + ... + <uses-sdk android:maxSdkVersion="23" android:minSdkVersion="16" android:targetSdkVersion="23" /> +</manifest> +``` + +Multiple plugins will not be able to to modify the same attributes because it may cause problems with the application. An error will be thrown and plugin install will fail. The conflicting `edit-config` tags must be resolved before the plugin can be added. Make modifications to conflicting tags to resolve the conflict, then remove and re-add those updated plugins. — End diff – "Multiple plugins will not be able to to modify " => "Multiple plugins can not modify" "then remove and re-add those updated plugins" => "then remove and re-add the updated plugins"
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user ktop commented on the issue:

          https://github.com/apache/cordova-docs/pull/614

          I shortened the AndroidManifest to snippets.

          I could see the force add content going into both locations, but in the CLI guide it should be more describing the error about conflicting edit-config and how it is caused. Maybe this section can link to the plugin.xml guide about edit-config for how to resolve the error. It's probably worth it for users to learn about edit-config in the plugin.xml guide before making changes.

          Show
          githubbot ASF GitHub Bot added a comment - Github user ktop commented on the issue: https://github.com/apache/cordova-docs/pull/614 I shortened the AndroidManifest to snippets. I could see the force add content going into both locations, but in the CLI guide it should be more describing the error about conflicting edit-config and how it is caused. Maybe this section can link to the plugin.xml guide about edit-config for how to resolve the error. It's probably worth it for users to learn about edit-config in the plugin.xml guide before making changes.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user riknoll commented on the issue:

          https://github.com/apache/cordova-docs/pull/614

          For the examples, I would just give a snippet of the AndroidMainfest and not the whole thing. There's a lot that doesn't change in the before and after. Secondly, the example of force adding plugins is pretty good, but I wonder if this is the right place for it. Perhaps it is better suited for the CLI guide as a sub-heading under the [plugin command](https://github.com/apache/cordova-cli/blob/master/doc/readme.md#cordova-plugin-command). People who get CLI errors might be more likely to look there since they are probably not plugin authors themselves.

          Show
          githubbot ASF GitHub Bot added a comment - Github user riknoll commented on the issue: https://github.com/apache/cordova-docs/pull/614 For the examples, I would just give a snippet of the AndroidMainfest and not the whole thing. There's a lot that doesn't change in the before and after. Secondly, the example of force adding plugins is pretty good, but I wonder if this is the right place for it. Perhaps it is better suited for the CLI guide as a sub-heading under the [plugin command] ( https://github.com/apache/cordova-cli/blob/master/doc/readme.md#cordova-plugin-command ). People who get CLI errors might be more likely to look there since they are probably not plugin authors themselves.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user macdonst commented on the issue:

          https://github.com/apache/cordova-docs/pull/614

          @ktop looks good but my personal preference would be to show a before and after of the Android Manifest.xml you use in your example to really hammer home how it works.

          Show
          githubbot ASF GitHub Bot added a comment - Github user macdonst commented on the issue: https://github.com/apache/cordova-docs/pull/614 @ktop looks good but my personal preference would be to show a before and after of the Android Manifest.xml you use in your example to really hammer home how it works.
          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user ktop opened a pull request:

          https://github.com/apache/cordova-docs/pull/614

          CB-11023 New edit-config tag in plugin.xml

          Documenting the new edit-config tag in plugin.xml.
          @riknoll can you review? I tried to keep the examples short, but explain enough to get the point across. Let me know if I need to fix anything.

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

          $ git pull https://github.com/ktop/cordova-docs cb-11023

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

          https://github.com/apache/cordova-docs/pull/614.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 #614


          commit 1d8ec834eb5d157ce779cf7517b6670c1ad36a16
          Author: ktop <ktop500@gmail.com>
          Date: 2016-06-23T14:24:09Z

          CB-11023 New edit-config tag in plugin.xml


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user ktop opened a pull request: https://github.com/apache/cordova-docs/pull/614 CB-11023 New edit-config tag in plugin.xml Documenting the new edit-config tag in plugin.xml. @riknoll can you review? I tried to keep the examples short, but explain enough to get the point across. Let me know if I need to fix anything. You can merge this pull request into a Git repository by running: $ git pull https://github.com/ktop/cordova-docs cb-11023 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/cordova-docs/pull/614.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 #614 commit 1d8ec834eb5d157ce779cf7517b6670c1ad36a16 Author: ktop <ktop500@gmail.com> Date: 2016-06-23T14:24:09Z CB-11023 New edit-config tag in plugin.xml
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user ktop commented on the issue:

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

          @riknoll yea sure. Are there instructions on how/where documentation goes?

          Show
          githubbot ASF GitHub Bot added a comment - Github user ktop commented on the issue: https://github.com/apache/cordova-lib/pull/449 @riknoll yea sure. Are there instructions on how/where documentation goes?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user riknoll commented on the issue:

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

          Done! Sorry for the delay. @ktop can you open a PR documenting this feature in the [plugin.xml reference](https://github.com/apache/cordova-docs/blob/master/www/docs/en/dev/plugin_ref/spec.md) as well?

          Show
          githubbot ASF GitHub Bot added a comment - Github user riknoll commented on the issue: https://github.com/apache/cordova-lib/pull/449 Done! Sorry for the delay. @ktop can you open a PR documenting this feature in the [plugin.xml reference] ( https://github.com/apache/cordova-docs/blob/master/www/docs/en/dev/plugin_ref/spec.md ) as well?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user asfgit closed the pull request at:

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

          Show
          githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/cordova-lib/pull/449
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 5fb8dffb2dc92e4d08d286432f23299da0e81249 in cordova-lib's branch refs/heads/master from Karen Tran
          [ https://git-wip-us.apache.org/repos/asf?p=cordova-lib.git;h=5fb8dff ]

          CB-11023 Add edit-config functionality

          This closes #449

          Show
          jira-bot ASF subversion and git services added a comment - Commit 5fb8dffb2dc92e4d08d286432f23299da0e81249 in cordova-lib's branch refs/heads/master from Karen Tran [ https://git-wip-us.apache.org/repos/asf?p=cordova-lib.git;h=5fb8dff ] CB-11023 Add edit-config functionality This closes #449
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user ktop commented on the issue:

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

          @riknoll I've rebased so this should be ready for merge.

          Show
          githubbot ASF GitHub Bot added a comment - Github user ktop commented on the issue: https://github.com/apache/cordova-lib/pull/449 @riknoll I've rebased so this should be ready for merge.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user ktop commented on the issue:

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

          Cool, let me rebase

          Show
          githubbot ASF GitHub Bot added a comment - Github user ktop commented on the issue: https://github.com/apache/cordova-lib/pull/449 Cool, let me rebase
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user macdonst commented on the issue:

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

          LGTM! Sorry for taking so long. My workspace is in a bad way and I think I need to kill it with fire and setup the whole thing from scratch.

          Once this gets into a release I have about 4 plugins I will modify to use this approach.

          Show
          githubbot ASF GitHub Bot added a comment - Github user macdonst commented on the issue: https://github.com/apache/cordova-lib/pull/449 LGTM! Sorry for taking so long. My workspace is in a bad way and I think I need to kill it with fire and setup the whole thing from scratch. Once this gets into a release I have about 4 plugins I will modify to use this approach.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user riknoll commented on the issue:

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

          This is working fine for me! When this feature gets documented, we need to make sure to document how to get your project back in order if you mess it up by adding a plugin with `--force`. I think removing all plugins and re-adding them (minus the conflicting ones) should do the trick.

          I'll wait for @macdonst to take a look before I merge this. This change will require a platform release as well since it is in cordova-common.

          Show
          githubbot ASF GitHub Bot added a comment - Github user riknoll commented on the issue: https://github.com/apache/cordova-lib/pull/449 This is working fine for me! When this feature gets documented, we need to make sure to document how to get your project back in order if you mess it up by adding a plugin with `--force`. I think removing all plugins and re-adding them (minus the conflicting ones) should do the trick. I'll wait for @macdonst to take a look before I merge this. This change will require a platform release as well since it is in cordova-common.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user ktop commented on the issue:

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

          My latest commit will find all conflicts when --force is used and remove them before adding the new plugin. @riknoll or @macdonst can you try it out and let me know if you have any issues?

          Also looks like my git name got fixed in the latest commit, so I think after I rebase and squash, it'll fix the first commit.

          Show
          githubbot ASF GitHub Bot added a comment - Github user ktop commented on the issue: https://github.com/apache/cordova-lib/pull/449 My latest commit will find all conflicts when --force is used and remove them before adding the new plugin. @riknoll or @macdonst can you try it out and let me know if you have any issues? Also looks like my git name got fixed in the latest commit, so I think after I rebase and squash, it'll fix the first commit.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user ktop commented on the issue:

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

          ahh must be because I didn't set up my git config after my computer got re-imaged. I have it set now and hopefully it will appear after I rebase.

          Show
          githubbot ASF GitHub Bot added a comment - Github user ktop commented on the issue: https://github.com/apache/cordova-lib/pull/449 ahh must be because I didn't set up my git config after my computer got re-imaged. I have it set now and hopefully it will appear after I rebase.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user macdonst commented on the issue:

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

          @ktop I also see "unknown" for your name in `git log`.

          Show
          githubbot ASF GitHub Bot added a comment - Github user macdonst commented on the issue: https://github.com/apache/cordova-lib/pull/449 @ktop I also see "unknown" for your name in `git log`.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user riknoll commented on the issue:

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

          @ktop Run `git log` and check out the author field. Your email shows up, but the name is unknown for some reason.

          As for the force add thing, that sounds fine to me. There are two possible behaviors:

          1. When you force add a plugin, remove any existing changes (i.e. the changes of plugin 1) and apply the new changes (from plugin 2)
          2. When you force add a plugin, ignore any conflicting changes (i.e. the changes of plugin 2) and leave the existing ones in place (from plugin 1)

          I believe you are describing the first behavior. The difference between that and the current implementation is that the current implementation shouldn't store both sets of changes in platform.json. If the changes of plugin 1 get overwritten by plugin 2, then they should be undone before the changes of plugin 2 are made. Otherwise, the platform.json gets into a weird state.

          Show
          githubbot ASF GitHub Bot added a comment - Github user riknoll commented on the issue: https://github.com/apache/cordova-lib/pull/449 @ktop Run `git log` and check out the author field. Your email shows up, but the name is unknown for some reason. As for the force add thing, that sounds fine to me. There are two possible behaviors: 1. When you force add a plugin, remove any existing changes (i.e. the changes of plugin 1) and apply the new changes (from plugin 2) 2. When you force add a plugin, ignore any conflicting changes (i.e. the changes of plugin 2) and leave the existing ones in place (from plugin 1) I believe you are describing the first behavior. The difference between that and the current implementation is that the current implementation shouldn't store both sets of changes in platform.json. If the changes of plugin 1 get overwritten by plugin 2, then they should be undone before the changes of plugin 2 are made. Otherwise, the platform.json gets into a weird state.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user ktop commented on the issue:

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

          @riknoll
          Where does it say "unknown"? I don't see it.

          If we have plugin-1, plugin-2 --force, and then add plugin-3 with --force, plugin-1 and plugin-2 should not be applied correct?

          Show
          githubbot ASF GitHub Bot added a comment - Github user ktop commented on the issue: https://github.com/apache/cordova-lib/pull/449 @riknoll Where does it say "unknown"? I don't see it. If we have plugin-1, plugin-2 --force, and then add plugin-3 with --force, plugin-1 and plugin-2 should not be applied correct?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user riknoll commented on the issue:

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

          I've tested it out and I think there needs to be a slight tweak to the `--force` behavior. When a plugin is force added, any conflicting edit-config changes it has should not be applied. Otherwise, you can get into a weird situation where adding conflicting plugins and removing them in a bad order results in the config file being different than when you started.

          For example, If I have two plugins that conflict and I execute the following commands:
          ```
          cordova plugin add plugin-1
          cordova plugin add plugin-2 --force
          cordova plugin rm plugin-1
          cordova plugin rm plugin-2
          ```
          I will be left with a config file that has the changes that `plugin-1` made despite `plugin-1` no longer being in my project. I think it's a good idea to always make it so that removing all plugins will get you back to where you started.

          Show
          githubbot ASF GitHub Bot added a comment - Github user riknoll commented on the issue: https://github.com/apache/cordova-lib/pull/449 I've tested it out and I think there needs to be a slight tweak to the `--force` behavior. When a plugin is force added, any conflicting edit-config changes it has should not be applied. Otherwise, you can get into a weird situation where adding conflicting plugins and removing them in a bad order results in the config file being different than when you started. For example, If I have two plugins that conflict and I execute the following commands: ``` cordova plugin add plugin-1 cordova plugin add plugin-2 --force cordova plugin rm plugin-1 cordova plugin rm plugin-2 ``` I will be left with a config file that has the changes that `plugin-1` made despite `plugin-1` no longer being in my project. I think it's a good idea to always make it so that removing all plugins will get you back to where you started.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user riknoll commented on the issue:

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

          @ktop the author name on the commit is "unknown"

          Show
          githubbot ASF GitHub Bot added a comment - Github user riknoll commented on the issue: https://github.com/apache/cordova-lib/pull/449 @ktop the author name on the commit is "unknown"
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user riknoll commented on a diff in the pull request:

          https://github.com/apache/cordova-lib/pull/449#discussion_r66486389

          — Diff: cordova-common/src/ConfigChanges/ConfigChanges.js —
          @@ -125,12 +128,32 @@ function remove_plugin_changes(pluginInfo, is_top_level) {

          PlatformMunger.prototype.add_plugin_changes = add_plugin_changes;
          -function add_plugin_changes(pluginInfo, plugin_vars, is_top_level, should_increment) {
          +function add_plugin_changes(pluginInfo, plugin_vars, is_top_level, should_increment, plugin_force) {
          var self = this;
          var platform_config = self.platformJson.root;
          + var editConfigChanges = pluginInfo.getEditConfigs(self.platform);
          + var config_munge;

          • // get config munge, aka how should this plugin change various config files
          • var config_munge = self.generate_plugin_config_munge(pluginInfo, plugin_vars);
            + if (!editConfigChanges || editConfigChanges.length === 0) { + // get config munge, aka how should this plugin change various config files + config_munge = self.generate_plugin_config_munge(pluginInfo, plugin_vars); + }

            + else if (plugin_force) {
            + CordovaLogger.get().log(CordovaLogger.WARN, '--force is used. edit-config will overwrite any conflicts');

              • End diff –

          I would add "conflicting plugins may not work as expected" to the end of this warning (or something to that effect)

          Show
          githubbot ASF GitHub Bot added a comment - Github user riknoll commented on a diff in the pull request: https://github.com/apache/cordova-lib/pull/449#discussion_r66486389 — Diff: cordova-common/src/ConfigChanges/ConfigChanges.js — @@ -125,12 +128,32 @@ function remove_plugin_changes(pluginInfo, is_top_level) { PlatformMunger.prototype.add_plugin_changes = add_plugin_changes; -function add_plugin_changes(pluginInfo, plugin_vars, is_top_level, should_increment) { +function add_plugin_changes(pluginInfo, plugin_vars, is_top_level, should_increment, plugin_force) { var self = this; var platform_config = self.platformJson.root; + var editConfigChanges = pluginInfo.getEditConfigs(self.platform); + var config_munge; // get config munge, aka how should this plugin change various config files var config_munge = self.generate_plugin_config_munge(pluginInfo, plugin_vars); + if (!editConfigChanges || editConfigChanges.length === 0) { + // get config munge, aka how should this plugin change various config files + config_munge = self.generate_plugin_config_munge(pluginInfo, plugin_vars); + } + else if (plugin_force) { + CordovaLogger.get().log(CordovaLogger.WARN, '--force is used. edit-config will overwrite any conflicts'); End diff – I would add "conflicting plugins may not work as expected" to the end of this warning (or something to that effect)
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user codecov-io commented on the issue:

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

            1. [Current coverage][cc-pull] is *80.56%*
              > Merging 449[cc-pull] into [master][cc-base-branch] will not change coverage

          ```diff
          @@ master #449 diff @@
          ==========================================
          Files 68 68
          Lines 5387 5387
          Methods 851 851
          Messages 0 0
          Branches 1042 1042
          ==========================================
          Hits 4340 4340
          Misses 1047 1047
          Partials 0 0
          ```

          > Powered by [Codecov](https://codecov.io?src=pr). Last updated by [ca98abf...8c2551d][cc-compare]
          [cc-base-branch]: https://codecov.io/gh/apache/cordova-lib/branch/master?src=pr
          [cc-compare]: https://codecov.io/gh/apache/cordova-lib/compare/ca98abf66ec573873ef0609b42926df7c804f564...8c2551d96d509419dd2dc780b37e0aaf8f2a1529
          [cc-pull]: https://codecov.io/gh/apache/cordova-lib/pull/449?src=pr

          Show
          githubbot ASF GitHub Bot added a comment - Github user codecov-io commented on the issue: https://github.com/apache/cordova-lib/pull/449 [Current coverage] [cc-pull] is * 80.56% * > Merging 449 [cc-pull] into [master] [cc-base-branch] will not change coverage ```diff @@ master #449 diff @@ ========================================== Files 68 68 Lines 5387 5387 Methods 851 851 Messages 0 0 Branches 1042 1042 ========================================== Hits 4340 4340 Misses 1047 1047 Partials 0 0 ``` > Powered by [Codecov] ( https://codecov.io?src=pr ). Last updated by [ca98abf...8c2551d] [cc-compare] [cc-base-branch] : https://codecov.io/gh/apache/cordova-lib/branch/master?src=pr [cc-compare] : https://codecov.io/gh/apache/cordova-lib/compare/ca98abf66ec573873ef0609b42926df7c804f564...8c2551d96d509419dd2dc780b37e0aaf8f2a1529 [cc-pull] : https://codecov.io/gh/apache/cordova-lib/pull/449?src=pr
          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user ktop opened a pull request:

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

          CB-11023 Add edit-config functionality

          New edit-config tag for plugin.xml will allow users to modify xml attributes.
          Ex. Assumes your AndroidManifest.xml has a second activity element with attribute android:name="SecondActivity"
          ```xml
          <!-- Will modify first activity -->
          <edit-config file="AndroidManifest.xml" target="/manifest/application/activity" mode="merge">
          <activity android:enabled="true" android:configChanges="orientation|keyboardHidden" />
          </edit-config>
          <!-- Will modify second activity -->
          <edit-config file="AndroidManifest.xml" target="/manifest/application/activity[@android:name='SecondActivity']" mode="overwrite">
          <activity android:enabled="true" android:name="SecondActivity" />
          </edit-config>
          ```
          file: specifies the file location
          target: specifies an xpath to the element that you want to modify
          modes:

          • merge: add attributes in the target with the ones specified by edit-config and will replace if the attribute names are the same
          • overwrite: replace all of the attributes at the target with the one specified by edit-config

          children: will only modify one element per edit-config tag

          There is conflict checking now....if a plugin wants to modify an attribute another plugin has already modified, an error will be thrown and plugin install will fail. The user must fix the conflict or they can use --force to force add the plugin and overwrite the conflict.

          Lastly, on plugin uninstall, the plugin should restore the attributes to the state it was before installing.

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

          $ git pull https://github.com/ktop/cordova-lib editconfig

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

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


          commit a3864929400f61fc31d03c3bab1d91a9467a3fbf
          Author: unknown <ktop500@gmail.com>
          Date: 2016-05-26T21:36:23Z

          CB-11023 Add edit-config functionality


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user ktop opened a pull request: https://github.com/apache/cordova-lib/pull/449 CB-11023 Add edit-config functionality New edit-config tag for plugin.xml will allow users to modify xml attributes. Ex. Assumes your AndroidManifest.xml has a second activity element with attribute android:name="SecondActivity" ```xml <!-- Will modify first activity --> <edit-config file="AndroidManifest.xml" target="/manifest/application/activity" mode="merge"> <activity android:enabled="true" android:configChanges="orientation|keyboardHidden" /> </edit-config> <!-- Will modify second activity --> <edit-config file="AndroidManifest.xml" target="/manifest/application/activity [@android:name='SecondActivity'] " mode="overwrite"> <activity android:enabled="true" android:name="SecondActivity" /> </edit-config> ``` file: specifies the file location target: specifies an xpath to the element that you want to modify modes: merge: add attributes in the target with the ones specified by edit-config and will replace if the attribute names are the same overwrite: replace all of the attributes at the target with the one specified by edit-config children: will only modify one element per edit-config tag There is conflict checking now....if a plugin wants to modify an attribute another plugin has already modified, an error will be thrown and plugin install will fail. The user must fix the conflict or they can use --force to force add the plugin and overwrite the conflict. Lastly, on plugin uninstall, the plugin should restore the attributes to the state it was before installing. You can merge this pull request into a Git repository by running: $ git pull https://github.com/ktop/cordova-lib editconfig Alternatively you can review and apply these changes as the patch at: https://github.com/apache/cordova-lib/pull/449.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 #449 commit a3864929400f61fc31d03c3bab1d91a9467a3fbf Author: unknown <ktop500@gmail.com> Date: 2016-05-26T21:36:23Z CB-11023 Add edit-config functionality
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user ktop closed the pull request at:

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

          Show
          githubbot ASF GitHub Bot added a comment - Github user ktop closed the pull request at: https://github.com/apache/cordova-lib/pull/432
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user ktop commented on the issue:

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

          I'm gonna open a new pull request with new changes. Closing this one....

          Show
          githubbot ASF GitHub Bot added a comment - Github user ktop commented on the issue: https://github.com/apache/cordova-lib/pull/432 I'm gonna open a new pull request with new changes. Closing this one....
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user riknoll commented on the pull request:

          https://github.com/apache/cordova-lib/pull/432#issuecomment-220658702

          My suggestion was to separate editing and inserting altogether. Continue to have a `config-file` tag that works exactly like before for adding things to XML, and have a new differently named tag for editing existing tags (like `edit-config`). The new tag wouldn't specify a parent, it would use the xpath selector to specify the element to be edited directly. We do lose the ability to edit a bunch of tags all at once like you say, but I don't think it would be too cumbersome to add a separate element for each edit we want to make.

          Show
          githubbot ASF GitHub Bot added a comment - Github user riknoll commented on the pull request: https://github.com/apache/cordova-lib/pull/432#issuecomment-220658702 My suggestion was to separate editing and inserting altogether. Continue to have a `config-file` tag that works exactly like before for adding things to XML, and have a new differently named tag for editing existing tags (like `edit-config`). The new tag wouldn't specify a parent, it would use the xpath selector to specify the element to be edited directly. We do lose the ability to edit a bunch of tags all at once like you say, but I don't think it would be too cumbersome to add a separate element for each edit we want to make.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user ktop commented on the pull request:

          https://github.com/apache/cordova-lib/pull/432#issuecomment-220647997

          I'd like to clear up some confusion I have on adding attributes.
          Before, we had decided on going with this syntax where the child elements of config-file tag would point to the child tags of the parent we want to add attributes to, so that we could add attributes to more than one child in a single config-file tag.

          With the following syntax, I wouldn't be able to at add to the second activity.
          ```xml
          <config-file target="AndroidManifest.xml" parent="/manifest/application" mode="merge" >
          <activity android:enabled="true" />
          </config-file>
          ```

          Now with your suggestion, the tag would look like this:
          Here, I would be able to specify the second activity, but this contradicts with what we had before since activity tag isn't a child of the parent.
          ```xml
          <config-file target="AndroidManifest.xml" parent="/manifest/application/activity[@android:name='SecondActivity']" mode="merge" >
          <activity android:enabled="true" />
          </config-file>
          ```

          Would we lose the ability to add attributes to more than one child? Or should we try to do both? Detect that we want to add to the tag that is the same as the parent as well as child tags. (Not sure if this makes sense)
          ```xml
          <config-file target="AndroidManifest.xml" parent="/manifest/application/activity[@android:name='SecondActivity']" mode="merge" >
          <activity android:enabled="true" />
          <intent-filter android:priority="1" />
          </config-file>
          ```
          Or any other ideas?

          Show
          githubbot ASF GitHub Bot added a comment - Github user ktop commented on the pull request: https://github.com/apache/cordova-lib/pull/432#issuecomment-220647997 I'd like to clear up some confusion I have on adding attributes. Before, we had decided on going with this syntax where the child elements of config-file tag would point to the child tags of the parent we want to add attributes to, so that we could add attributes to more than one child in a single config-file tag. With the following syntax, I wouldn't be able to at add to the second activity. ```xml <config-file target="AndroidManifest.xml" parent="/manifest/application" mode="merge" > <activity android:enabled="true" /> </config-file> ``` Now with your suggestion, the tag would look like this: Here, I would be able to specify the second activity, but this contradicts with what we had before since activity tag isn't a child of the parent. ```xml <config-file target="AndroidManifest.xml" parent="/manifest/application/activity [@android:name='SecondActivity'] " mode="merge" > <activity android:enabled="true" /> </config-file> ``` Would we lose the ability to add attributes to more than one child? Or should we try to do both? Detect that we want to add to the tag that is the same as the parent as well as child tags. (Not sure if this makes sense) ```xml <config-file target="AndroidManifest.xml" parent="/manifest/application/activity [@android:name='SecondActivity'] " mode="merge" > <activity android:enabled="true" /> <intent-filter android:priority="1" /> </config-file> ``` Or any other ideas?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user riknoll commented on the pull request:

          https://github.com/apache/cordova-lib/pull/432#issuecomment-220467601

          I think that the issue might be that we need to separate inserting child elements and editing existing ones. Something like this:

          ```xml
          <!-- Editing attributes in existing elements -->
          <edit-config target="xpath/nonsense" mode="merge|overwrite">
          <attribute name="attr" new-value="edited" />
          </edit-config>

          <!-- Existing config-file insert behavior -->
          <insert-config parent="xpath/nonsense">
          <child-element />
          </insert-config>
          ```

          This is a bit more verbose if you have a lot of edits that need to be performed, but much more flexible because the xpath selector can be used to reference specific elements, even in lists. It might also help with detecting plugin conflicts. I don't care about the names of those tags (we can keep using config-file if we want).

          For plugin conflicts, detection should probably happen at "plugin add" time. The add should fail on conflict and allow the user to override it using `--force` if they want to. As for which plugin gets dibs in the forced add scenario, it doesn't really matter as long as it is consistent. Hopefully the user can determine the correct value on conflict and fix it by editing the config from the project's `config.xml`.

          Show
          githubbot ASF GitHub Bot added a comment - Github user riknoll commented on the pull request: https://github.com/apache/cordova-lib/pull/432#issuecomment-220467601 I think that the issue might be that we need to separate inserting child elements and editing existing ones. Something like this: ```xml <!-- Editing attributes in existing elements --> <edit-config target="xpath/nonsense" mode="merge|overwrite"> <attribute name="attr" new-value="edited" /> </edit-config> <!-- Existing config-file insert behavior --> <insert-config parent="xpath/nonsense"> <child-element /> </insert-config> ``` This is a bit more verbose if you have a lot of edits that need to be performed, but much more flexible because the xpath selector can be used to reference specific elements, even in lists. It might also help with detecting plugin conflicts. I don't care about the names of those tags (we can keep using config-file if we want). For plugin conflicts, detection should probably happen at "plugin add" time. The add should fail on conflict and allow the user to override it using `--force` if they want to. As for which plugin gets dibs in the forced add scenario, it doesn't really matter as long as it is consistent. Hopefully the user can determine the correct value on conflict and fix it by editing the config from the project's `config.xml`.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user ktop commented on the pull request:

          https://github.com/apache/cordova-lib/pull/432#issuecomment-220440531

          @riknoll Vladimir is correct, there is no mechanism for either yet.

          I think warnings are good, but does that mean the plugins that get installed first get first dibs to modifying xml elements? How do we go about keeping track of whether or not an attribute/element was modified or not?

          For the second question, that has actually crossed my mind a few times while testing (how I couldn't modify the second activity tag or add children to it). I'm not really sure the best way to handle it. I've thought about adding an identifier attribute to the config-file tag, but that would limit config-file to modify only one element.

          ```xml
          <config-file target="AndroidManifest.xml" parent="/manifest/application" identifierName="android:name" identifierValue="SecondActivity" mode="merge" >
          <activity android:configChanges="orientation|keyboardHidden|screenSize" />
          </config-file>
          ```

          End goal would be to be able to modify both activity tags in the same config-file tag, but that may be a stretch.

          Show
          githubbot ASF GitHub Bot added a comment - Github user ktop commented on the pull request: https://github.com/apache/cordova-lib/pull/432#issuecomment-220440531 @riknoll Vladimir is correct, there is no mechanism for either yet. I think warnings are good, but does that mean the plugins that get installed first get first dibs to modifying xml elements? How do we go about keeping track of whether or not an attribute/element was modified or not? For the second question, that has actually crossed my mind a few times while testing (how I couldn't modify the second activity tag or add children to it). I'm not really sure the best way to handle it. I've thought about adding an identifier attribute to the config-file tag, but that would limit config-file to modify only one element. ```xml <config-file target="AndroidManifest.xml" parent="/manifest/application" identifierName="android:name" identifierValue="SecondActivity" mode="merge" > <activity android:configChanges="orientation|keyboardHidden|screenSize" /> </config-file> ``` End goal would be to be able to modify both activity tags in the same config-file tag, but that may be a stretch.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user ktop commented on a diff in the pull request:

          https://github.com/apache/cordova-lib/pull/432#discussion_r63922339

          — Diff: cordova-lib/src/cordova/prepare.js —
          @@ -116,6 +117,13 @@ function preparePlatforms (platformList, projectRoot, options)

          { var browserify = require('../plugman/browserify'); return browserify(project, platformApi); }

          + })
          + .then(function () {
          + // Handle config-file in config.xml
          + var platformRoot = path.join(projectRoot, 'platforms', platform);
          + var platformJson = PlatformJson.load(platformRoot, platform);
          + var munger = new PlatformMunger(platform, platformRoot, platformJson);
          + munger.add_config_changes(project.projectConfig, /should_increment=/true).save_all();
          — End diff –

          Thanks for reviewing, I'll make the change.

          Show
          githubbot ASF GitHub Bot added a comment - Github user ktop commented on a diff in the pull request: https://github.com/apache/cordova-lib/pull/432#discussion_r63922339 — Diff: cordova-lib/src/cordova/prepare.js — @@ -116,6 +117,13 @@ function preparePlatforms (platformList, projectRoot, options) { var browserify = require('../plugman/browserify'); return browserify(project, platformApi); } + }) + .then(function () { + // Handle config-file in config.xml + var platformRoot = path.join(projectRoot, 'platforms', platform); + var platformJson = PlatformJson.load(platformRoot, platform); + var munger = new PlatformMunger(platform, platformRoot, platformJson); + munger.add_config_changes(project.projectConfig, / should_increment= /true).save_all(); — End diff – Thanks for reviewing, I'll make the change.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user riknoll commented on the pull request:

          https://github.com/apache/cordova-lib/pull/432#issuecomment-220386776

          @vladimir-kotikov yep, my thought's exactly! We definitely need to handle plugins conflicting with each other because any bugs caused by those conflicts would likely be extremely difficult to debug/trace. +1 to the config.xml warning as well

          Show
          githubbot ASF GitHub Bot added a comment - Github user riknoll commented on the pull request: https://github.com/apache/cordova-lib/pull/432#issuecomment-220386776 @vladimir-kotikov yep, my thought's exactly! We definitely need to handle plugins conflicting with each other because any bugs caused by those conflicts would likely be extremely difficult to debug/trace. +1 to the config.xml warning as well
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user vladimir-kotikov commented on the pull request:

          https://github.com/apache/cordova-lib/pull/432#issuecomment-220254686

          I think there is no mechanism for resolving conflicts, as there was no need for it before.

          One idea on how to deal w/ conflicts is to fail by default in case of conflicting attributes changes made by plugins, because overwriting the attributes possibly would lead to unexpected errors at build and run time. However, the user once notified with appropriate error message still could install the conflicting plugin using `--force` flag.

          Also overriding attributes from `config.xml` is under user control, and should always be allowed (we might want to show warning in this case though).

          Show
          githubbot ASF GitHub Bot added a comment - Github user vladimir-kotikov commented on the pull request: https://github.com/apache/cordova-lib/pull/432#issuecomment-220254686 I think there is no mechanism for resolving conflicts, as there was no need for it before. One idea on how to deal w/ conflicts is to fail by default in case of conflicting attributes changes made by plugins , because overwriting the attributes possibly would lead to unexpected errors at build and run time. However, the user once notified with appropriate error message still could install the conflicting plugin using `--force` flag. Also overriding attributes from `config.xml` is under user control, and should always be allowed (we might want to show warning in this case though).
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user riknoll commented on the pull request:

          https://github.com/apache/cordova-lib/pull/432#issuecomment-220179183

          @ktop I have a couple questions. First, do we have a plan for dealing with plugins that have conflicting attribute edits? For example, if plugin A has this in its plugin.xml:

          ```xml
          <config-file target="AndroidManifest.xml" parent="/manifest" attr="true">
          <uses-sdk android:maxSdkVersion="22" />
          </config-file>
          ```

          and plugin B has this in its plugin.xml:

          ```xml
          <config-file target="AndroidManifest.xml" parent="/manifest" attr="true">
          <uses-sdk android:maxSdkVersion="17" />
          </config-file>
          ```

          Will the user get a build error?

          Second, is it possible to reference a sibling of the same tag type? For example, given this in my AndroidManifest.xml:

          ```xml
          <application android:icon="@drawable/icon" android:label="@string/app_name">
          <activity
          android:label="@string/activity_name"
          android:name="MainActivity" />
          <activity
          android:label="@string/second_activity"
          android:name="SecondActivity" />
          </application>
          ```

          If I wanted to add an attribute in the second Activity, how would I do so? This isn't a real-life example, but you get the idea.

          Show
          githubbot ASF GitHub Bot added a comment - Github user riknoll commented on the pull request: https://github.com/apache/cordova-lib/pull/432#issuecomment-220179183 @ktop I have a couple questions. First, do we have a plan for dealing with plugins that have conflicting attribute edits? For example, if plugin A has this in its plugin.xml: ```xml <config-file target="AndroidManifest.xml" parent="/manifest" attr="true"> <uses-sdk android:maxSdkVersion="22" /> </config-file> ``` and plugin B has this in its plugin.xml: ```xml <config-file target="AndroidManifest.xml" parent="/manifest" attr="true"> <uses-sdk android:maxSdkVersion="17" /> </config-file> ``` Will the user get a build error? Second, is it possible to reference a sibling of the same tag type? For example, given this in my AndroidManifest.xml: ```xml <application android:icon="@drawable/icon" android:label="@string/app_name"> <activity android:label="@string/activity_name" android:name="MainActivity" /> <activity android:label="@string/second_activity" android:name="SecondActivity" /> </application> ``` If I wanted to add an attribute in the second Activity, how would I do so? This isn't a real-life example, but you get the idea.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user nikhilkh commented on a diff in the pull request:

          https://github.com/apache/cordova-lib/pull/432#discussion_r63604254

          — Diff: cordova-lib/src/cordova/prepare.js —
          @@ -116,6 +117,13 @@ function preparePlatforms (platformList, projectRoot, options)

          { var browserify = require('../plugman/browserify'); return browserify(project, platformApi); }

          + })
          + .then(function () {
          + // Handle config-file in config.xml
          + var platformRoot = path.join(projectRoot, 'platforms', platform);
          + var platformJson = PlatformJson.load(platformRoot, platform);
          + var munger = new PlatformMunger(platform, platformRoot, platformJson);
          + munger.add_config_changes(project.projectConfig, /should_increment=/true).save_all();
          — End diff –

          This API cannot be used until cordova-common is released. This needs to go in a separate PR.

          Show
          githubbot ASF GitHub Bot added a comment - Github user nikhilkh commented on a diff in the pull request: https://github.com/apache/cordova-lib/pull/432#discussion_r63604254 — Diff: cordova-lib/src/cordova/prepare.js — @@ -116,6 +117,13 @@ function preparePlatforms (platformList, projectRoot, options) { var browserify = require('../plugman/browserify'); return browserify(project, platformApi); } + }) + .then(function () { + // Handle config-file in config.xml + var platformRoot = path.join(projectRoot, 'platforms', platform); + var platformJson = PlatformJson.load(platformRoot, platform); + var munger = new PlatformMunger(platform, platformRoot, platformJson); + munger.add_config_changes(project.projectConfig, / should_increment= /true).save_all(); — End diff – This API cannot be used until cordova-common is released. This needs to go in a separate PR.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user ktop commented on the pull request:

          https://github.com/apache/cordova-lib/pull/432#issuecomment-217972581

          @macdonst I've added the implementation for config-file to config.xml just to get it working, but I do think it should be refactored (cordova-common/src/ConfigChanges/ConfigChanges.js). I really just wanted to rename `add_plugin_changes` to something more general but that would mean I have to do refactoring in several other repos. I'll take some suggestions on how to refactor, but for now, I just made a similar function that does almost the same thing, `add_config_changes`.

          This config-file will be handled right after platform prepare happens, which is called on every prepare and plugin install/uninstall. I think this should handle the case for config.xml overriding plugin.xml.

          Show
          githubbot ASF GitHub Bot added a comment - Github user ktop commented on the pull request: https://github.com/apache/cordova-lib/pull/432#issuecomment-217972581 @macdonst I've added the implementation for config-file to config.xml just to get it working, but I do think it should be refactored (cordova-common/src/ConfigChanges/ConfigChanges.js). I really just wanted to rename `add_plugin_changes` to something more general but that would mean I have to do refactoring in several other repos. I'll take some suggestions on how to refactor, but for now, I just made a similar function that does almost the same thing, `add_config_changes`. This config-file will be handled right after platform prepare happens, which is called on every prepare and plugin install/uninstall. I think this should handle the case for config.xml overriding plugin.xml.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user macdonst commented on the pull request:

          https://github.com/apache/cordova-lib/pull/432#issuecomment-217228255

          @ktop thanks for the hint about running `cordova-coho/coho npm-link`. Once I did I was able to get your code to run. It seems to be working pretty good to me. So I would love to get this into cordova ASAP.

          Folks do you think we should merge this PR then look at changes to config.xml or land it all in one PR?

          Show
          githubbot ASF GitHub Bot added a comment - Github user macdonst commented on the pull request: https://github.com/apache/cordova-lib/pull/432#issuecomment-217228255 @ktop thanks for the hint about running `cordova-coho/coho npm-link`. Once I did I was able to get your code to run. It seems to be working pretty good to me. So I would love to get this into cordova ASAP. Folks do you think we should merge this PR then look at changes to config.xml or land it all in one PR?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user ktop commented on the pull request:

          https://github.com/apache/cordova-lib/pull/432#issuecomment-216629805

          @macdonst @purplecabbage
          On my other machine (Mac), I see the same behavior that you both were seeing. I was able to fix it by doing a `cordova-coho/coho npm-link` and re-adding the android platform to the project. What was happening was the cordova-common module inside of cordova-android wasn't being linked to the one in cordova-lib, so it was using a cordova-common without my changes in it, thus using the default functionality of config-file.

          On a side note (this only happens for me on Mac, works fine on Windows), when coho replaces the cordova-common module with a symlink, this was treated as deleting the cordova-common module, then when I go to add the android platform, I get an error saying `Cannot find module 'cordova-common'` and when I checked the node modules folder, there's only a symlink file of cordova-common. I'm not really sure how symlinks are supposed to work on Mac since I mainly use a Windows machine. The project still runs and cordova-common is actually linked. I'm not sure if this is a Mac issue or something else.

          Show
          githubbot ASF GitHub Bot added a comment - Github user ktop commented on the pull request: https://github.com/apache/cordova-lib/pull/432#issuecomment-216629805 @macdonst @purplecabbage On my other machine (Mac), I see the same behavior that you both were seeing. I was able to fix it by doing a `cordova-coho/coho npm-link` and re-adding the android platform to the project. What was happening was the cordova-common module inside of cordova-android wasn't being linked to the one in cordova-lib, so it was using a cordova-common without my changes in it, thus using the default functionality of config-file. On a side note (this only happens for me on Mac, works fine on Windows), when coho replaces the cordova-common module with a symlink, this was treated as deleting the cordova-common module, then when I go to add the android platform, I get an error saying `Cannot find module 'cordova-common'` and when I checked the node modules folder, there's only a symlink file of cordova-common. I'm not really sure how symlinks are supposed to work on Mac since I mainly use a Windows machine. The project still runs and cordova-common is actually linked. I'm not sure if this is a Mac issue or something else.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user ktop commented on the pull request:

          https://github.com/apache/cordova-lib/pull/432#issuecomment-216543951

          @purplecabbage
          My workspace is working off of the master branches with the tools npm linked.

          I modified the plugin.xml of cordova-plugin-device and added:
          ```
          <config-file target="AndroidManifest.xml" parent="/manifest" attr="true">
          <application android:name="MyApplication" android:isGame="true" />
          <uses-sdk android:maxSdkVersion="22" />
          </config-file>
          ```

          Then for the project:
          ```
          cordova create myApp
          cd myApp
          cordova platform add ../cordova-android
          cordova plugin add ../cordova-plugin-device
          ```
          I check AndroidManifest.xml and the new attributes are there as expect, without a new tag being added.

          I'll try testing this PR on another machine and see what result I get.

          As for getting config-file tag into config.xml, I've been caught up in some other work so will get to this sometime later in the week.

          Show
          githubbot ASF GitHub Bot added a comment - Github user ktop commented on the pull request: https://github.com/apache/cordova-lib/pull/432#issuecomment-216543951 @purplecabbage My workspace is working off of the master branches with the tools npm linked. I modified the plugin.xml of cordova-plugin-device and added: ``` <config-file target="AndroidManifest.xml" parent="/manifest" attr="true"> <application android:name="MyApplication" android:isGame="true" /> <uses-sdk android:maxSdkVersion="22" /> </config-file> ``` Then for the project: ``` cordova create myApp cd myApp cordova platform add ../cordova-android cordova plugin add ../cordova-plugin-device ``` I check AndroidManifest.xml and the new attributes are there as expect, without a new tag being added. I'll try testing this PR on another machine and see what result I get. As for getting config-file tag into config.xml, I've been caught up in some other work so will get to this sometime later in the week.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user purplecabbage commented on the pull request:

          https://github.com/apache/cordova-lib/pull/432#issuecomment-216424469

          @ktop Can you provide the steps you used to verify that this is working please?

          What I did was create a new project, and added the cordova-plugin-device
          I then modified the plugin.xml to include your additions, and then added android and looked for the changes to show up in androidmanifest.xml

          Show
          githubbot ASF GitHub Bot added a comment - Github user purplecabbage commented on the pull request: https://github.com/apache/cordova-lib/pull/432#issuecomment-216424469 @ktop Can you provide the steps you used to verify that this is working please? What I did was create a new project, and added the cordova-plugin-device I then modified the plugin.xml to include your additions, and then added android and looked for the changes to show up in androidmanifest.xml
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user purplecabbage commented on the pull request:

          https://github.com/apache/cordova-lib/pull/432#issuecomment-216386608

          I too am seeing @macdonst 's issue.
          ```
          ...
          </application>
          <uses-sdk android:minSdkVersion="14" android:targetSdkVersion="23" />
          <application android:isGame="true" android:name="MyApplication" />
          <uses-sdk android:maxSdkVersion="22" />
          </manifest>
          ```

          Show
          githubbot ASF GitHub Bot added a comment - Github user purplecabbage commented on the pull request: https://github.com/apache/cordova-lib/pull/432#issuecomment-216386608 I too am seeing @macdonst 's issue. ``` ... </application> <uses-sdk android:minSdkVersion="14" android:targetSdkVersion="23" /> <application android:isGame="true" android:name="MyApplication" /> <uses-sdk android:maxSdkVersion="22" /> </manifest> ```
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user macdonst commented on the pull request:

          https://github.com/apache/cordova-lib/pull/432#issuecomment-215915464

          @ktop @riknoll yeah, config.xml should be part of this. Anything in config.xml should trump whatever is specified in the plugin.xml. If we do modes then default mode would be `add`, if nothing to `replace` then it becomes an `add` and `delete` could take out the entire element. We can work on flags later but getting attribute level manipulation into config.xml and plugin.xml parsing would be amazing.

          @purplecabbage you need to add:

          ```
          <config-file target="AndroidManifest.xml" parent="/manifest" attr="true">
          <application android:name="MyApplication" android:isGame="true" />
          <uses-sdk android:maxSdkVersion="22" />
          </config-file>
          ```

          into the plugin.xml of a plugin you are including in the project. So...

          ```
          cordova create newproject
          cordova platform add android
          cordova plugin add cordova-plugin-one-I-just-made-up
          // modified the plugin.xml with the fragment from above
          cordova prepare
          // checked androidmanifest.xml for changes
          ```

          Show
          githubbot ASF GitHub Bot added a comment - Github user macdonst commented on the pull request: https://github.com/apache/cordova-lib/pull/432#issuecomment-215915464 @ktop @riknoll yeah, config.xml should be part of this. Anything in config.xml should trump whatever is specified in the plugin.xml. If we do modes then default mode would be `add`, if nothing to `replace` then it becomes an `add` and `delete` could take out the entire element. We can work on flags later but getting attribute level manipulation into config.xml and plugin.xml parsing would be amazing. @purplecabbage you need to add: ``` <config-file target="AndroidManifest.xml" parent="/manifest" attr="true"> <application android:name="MyApplication" android:isGame="true" /> <uses-sdk android:maxSdkVersion="22" /> </config-file> ``` into the plugin.xml of a plugin you are including in the project. So... ``` cordova create newproject cordova platform add android cordova plugin add cordova-plugin-one-I-just-made-up // modified the plugin.xml with the fragment from above cordova prepare // checked androidmanifest.xml for changes ```
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user purplecabbage commented on the pull request:

          https://github.com/apache/cordova-lib/pull/432#issuecomment-215892244

          I would put the mode flag as out-of-scope for now, we can re-asses later if need be.

          I could not get this to work, can you please post instructions for verifying it?
          My steps were:
          ```
          cordova create newproject
          cordova platform add android
          // modified the root config.xml with the fragment from above
          cordova prepare
          // checked androidmanifest.xml for changes, none seen
          ```

          Show
          githubbot ASF GitHub Bot added a comment - Github user purplecabbage commented on the pull request: https://github.com/apache/cordova-lib/pull/432#issuecomment-215892244 I would put the mode flag as out-of-scope for now, we can re-asses later if need be. I could not get this to work, can you please post instructions for verifying it? My steps were: ``` cordova create newproject cordova platform add android // modified the root config.xml with the fragment from above cordova prepare // checked androidmanifest.xml for changes, none seen ```
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user riknoll commented on the pull request:

          https://github.com/apache/cordova-lib/pull/432#issuecomment-215866756

          @ktop config-file wasn't in config.xml before, but we wanted to add it because the number of config.xml attributes that just edit various native xml files is getting unsustainable. I think that config.xml's config-file modifications should happen during the same step as the plugins' modifications do, but they should definitely be applied after all of the plugin.xml ones. That way, the app developer always has the option to override whatever the plugins are doing.

          I think the mode flag is a good idea, but I worry about allowing it in plugin.xml. If plugins are able to do things other than add elements, does that mean we have the opportunity for plugins to overwrite each other's modifications depending on the order in which their config-file attributes are resolved? That might get weird. Definitely makes sense to have it in config.xml though.

          Show
          githubbot ASF GitHub Bot added a comment - Github user riknoll commented on the pull request: https://github.com/apache/cordova-lib/pull/432#issuecomment-215866756 @ktop config-file wasn't in config.xml before, but we wanted to add it because the number of config.xml attributes that just edit various native xml files is getting unsustainable. I think that config.xml's config-file modifications should happen during the same step as the plugins' modifications do, but they should definitely be applied after all of the plugin.xml ones. That way, the app developer always has the option to override whatever the plugins are doing. I think the mode flag is a good idea, but I worry about allowing it in plugin.xml. If plugins are able to do things other than add elements, does that mean we have the opportunity for plugins to overwrite each other's modifications depending on the order in which their config-file attributes are resolved? That might get weird. Definitely makes sense to have it in config.xml though.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user ktop commented on the pull request:

          https://github.com/apache/cordova-lib/pull/432#issuecomment-215857823

          @macdonst
          I had Edna test the first time around before I opened the PR and she was able to get it working, so that is weird that it is adding a new element for you.

          Was config-file functional in config.xml before? Or was that something to be done as part of this jira issue? I was only focused on getting it working in plugin.xml, so I can work on it if it isn't implemented yet. Should config.xml handle config-file during prepare?

          The mode suggestion sounds good. It will give users a lot of utility for modifying xml files.
          I have just few clarification questions:

          • If no mode is specified, is the default to add?
          • For replace, if there is nothing to replace, should it still try to add?
          • And for delete, should it delete an element even if the element has children?
          Show
          githubbot ASF GitHub Bot added a comment - Github user ktop commented on the pull request: https://github.com/apache/cordova-lib/pull/432#issuecomment-215857823 @macdonst I had Edna test the first time around before I opened the PR and she was able to get it working, so that is weird that it is adding a new element for you. Was config-file functional in config.xml before? Or was that something to be done as part of this jira issue? I was only focused on getting it working in plugin.xml, so I can work on it if it isn't implemented yet. Should config.xml handle config-file during prepare? The mode suggestion sounds good. It will give users a lot of utility for modifying xml files. I have just few clarification questions: If no mode is specified, is the default to add? For replace, if there is nothing to replace, should it still try to add? And for delete, should it delete an element even if the element has children?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user macdonst commented on the pull request:

          https://github.com/apache/cordova-lib/pull/432#issuecomment-215811240

          @ktop Yeah, I have the `attr="true"` in my plugin.xml and it still gets added instead of merged.

          ```
          <config-file target="AndroidManifest.xml" parent="/manifest" attr="true">
          <application android:name="MyApplication" android:isGame="true" />
          <uses-sdk android:maxSdkVersion="22" />
          </config-file>
          ```

          I'm not sure why it's not working for me 😢

          Regardless, I have some other comments. What if I put config-file statements in config.xml? Which change takes precedence?

          Also, instead of using `attr="true"` would it make more sense to use a more granular approach like `mode="add|replace|merge|delete"` where:

          • add will append to the inner xml of the parent
          • replace will completely overwrite the parent's inner xml with your declaration
          • merge will attempt to find elements of the same name and merge their attributes
          • delete will search for elements matching the specified name and attributes and delete them
          Show
          githubbot ASF GitHub Bot added a comment - Github user macdonst commented on the pull request: https://github.com/apache/cordova-lib/pull/432#issuecomment-215811240 @ktop Yeah, I have the `attr="true"` in my plugin.xml and it still gets added instead of merged. ``` <config-file target="AndroidManifest.xml" parent="/manifest" attr="true"> <application android:name="MyApplication" android:isGame="true" /> <uses-sdk android:maxSdkVersion="22" /> </config-file> ``` I'm not sure why it's not working for me 😢 Regardless, I have some other comments. What if I put config-file statements in config.xml? Which change takes precedence? Also, instead of using `attr="true"` would it make more sense to use a more granular approach like `mode="add|replace|merge|delete"` where: add will append to the inner xml of the parent replace will completely overwrite the parent's inner xml with your declaration merge will attempt to find elements of the same name and merge their attributes delete will search for elements matching the specified name and attributes and delete them
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user codecov-io commented on the pull request:

          https://github.com/apache/cordova-lib/pull/432#issuecomment-215478064

            1. [Current coverage][cc-pull] is *79.60%*
              > Merging 432[cc-pull] into [master][cc-base-branch] will increase coverage by *-0.49%*

          ```diff
          @@ master #432 diff @@
          ========================================
          Files 69 69
          Lines 5029 5398 +369
          Methods 0 865 +865
          Branches 972 1017 +45
          ========================================
          + Hits 4029 4297 +268

          • Misses 995 1101 +106
            + Partials 5 0 -5
            ```

          1. 4 files in `...dova-lib/src/cordova` were modified. [more](https://codecov.io/gh/apache/cordova-lib/commit/352d71395621245f691ec6143290ccd7774cbae9/changes?src=pr#636F72646F76612D6C69622F7372632F636F72646F7661)

          > Powered by [Codecov](https://codecov.io?src=pr). Last updated by 352d713
          [cc-base-branch]: https://codecov.io/gh/apache/cordova-lib/branch/master?src=pr
          [cc-pull]: https://codecov.io/gh/apache/cordova-lib/pull/432?src=pr

          Show
          githubbot ASF GitHub Bot added a comment - Github user codecov-io commented on the pull request: https://github.com/apache/cordova-lib/pull/432#issuecomment-215478064 [Current coverage] [cc-pull] is * 79.60% * > Merging 432 [cc-pull] into [master] [cc-base-branch] will increase coverage by * -0.49% * ```diff @@ master #432 diff @@ ======================================== Files 69 69 Lines 5029 5398 +369 Methods 0 865 +865 Branches 972 1017 +45 ======================================== + Hits 4029 4297 +268 Misses 995 1101 +106 + Partials 5 0 -5 ``` 1. 4 files in `...dova-lib/src/cordova` were modified. [more] ( https://codecov.io/gh/apache/cordova-lib/commit/352d71395621245f691ec6143290ccd7774cbae9/changes?src=pr#636F72646F76612D6C69622F7372632F636F72646F7661 ) Misses `+1` Hits `+13` 1. 2 files (not in diff) in `...lib/src/util/windows` were modified. [more] ( https://codecov.io/gh/apache/cordova-lib/commit/352d71395621245f691ec6143290ccd7774cbae9/changes?src=pr#636F72646F76612D6C69622F7372632F7574696C2F77696E646F7773 ) Misses `+6` Partials `-1` Hits `+24` 1. 2 files (not in diff) in `cordova-lib/src/util` were modified. [more] ( https://codecov.io/gh/apache/cordova-lib/commit/352d71395621245f691ec6143290ccd7774cbae9/changes?src=pr#636F72646F76612D6C69622F7372632F7574696C ) Misses `+4` Hits `+1` 1. 5 files (not in diff) in `...lib/src/plugman/util` were modified. [more] ( https://codecov.io/gh/apache/cordova-lib/commit/352d71395621245f691ec6143290ccd7774cbae9/changes?src=pr#636F72646F76612D6C69622F7372632F706C75676D616E2F7574696C ) Misses `-1` Hits `+17` 1. 7 files (not in diff) in `...rc/plugman/platforms` were modified. [more] ( https://codecov.io/gh/apache/cordova-lib/commit/352d71395621245f691ec6143290ccd7774cbae9/changes?src=pr#636F72646F76612D6C69622F7372632F706C75676D616E2F706C6174666F726D73 ) Misses `+32` Hits `+63` 1. 7 files (not in diff) in `...dova-lib/src/plugman` were modified. [more] ( https://codecov.io/gh/apache/cordova-lib/commit/352d71395621245f691ec6143290ccd7774cbae9/changes?src=pr#636F72646F76612D6C69622F7372632F706C75676D616E ) Misses `+22` Hits `+36` 1. 3 files (not in diff) in `...ordova-lib/src/hooks` were modified. [more] ( https://codecov.io/gh/apache/cordova-lib/commit/352d71395621245f691ec6143290ccd7774cbae9/changes?src=pr#636F72646F76612D6C69622F7372632F686F6F6B73 ) Misses `+2` Hits `+2` 1. 9 files (not in diff) in `...src/cordova/metadata` were modified. [more] ( https://codecov.io/gh/apache/cordova-lib/commit/352d71395621245f691ec6143290ccd7774cbae9/changes?src=pr#636F72646F76612D6C69622F7372632F636F72646F76612F6D65746164617461 ) Misses `+3` Hits `+12` 1. 10 files (not in diff) in `...dova-lib/src/cordova` were modified. [more] ( https://codecov.io/gh/apache/cordova-lib/commit/352d71395621245f691ec6143290ccd7774cbae9/changes?src=pr#636F72646F76612D6C69622F7372632F636F72646F7661 ) Misses `+31` Partials `-2` Hits `+78` 1. 2 files (not in diff) in `cordova-lib/src` were modified. [more] ( https://codecov.io/gh/apache/cordova-lib/commit/352d71395621245f691ec6143290ccd7774cbae9/changes?src=pr#636F72646F76612D6C69622F737263 ) Misses `+6` Partials `-2` Hits `+18` > Powered by [Codecov] ( https://codecov.io?src=pr ). Last updated by 352d713 [cc-base-branch] : https://codecov.io/gh/apache/cordova-lib/branch/master?src=pr [cc-pull] : https://codecov.io/gh/apache/cordova-lib/pull/432?src=pr
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user ktop commented on the pull request:

          https://github.com/apache/cordova-lib/pull/432#issuecomment-213530432

          If we can modify existing attributes, and then the plugin gets removed, the modified attribute will also get removed. I don't think there is a way to revert it back to what it was before. Is that ok?

          Show
          githubbot ASF GitHub Bot added a comment - Github user ktop commented on the pull request: https://github.com/apache/cordova-lib/pull/432#issuecomment-213530432 If we can modify existing attributes, and then the plugin gets removed, the modified attribute will also get removed. I don't think there is a way to revert it back to what it was before. Is that ok?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user macdonst commented on the pull request:

          https://github.com/apache/cordova-lib/pull/432#issuecomment-213519895

          @ktop I did a quick test of this with one of my existing apps and it does add the following lines to AndroidManifest.xml:

          ```
          <application android:isGame="true" android:name="MyApplication" />
          <uses-sdk android:maxSdkVersion="22" />
          ```

          but unfortunately it already had the following lines:

          ```
          <application android:hardwareAccelerated="true" android:icon="@drawable/icon" android:label="@string/app_name" android:supportsRtl="true">
          <uses-sdk android:minSdkVersion="14" android:targetSdkVersion="23" />
          ```

          So in order for this to get merged it has to be able to modify existing lines instead of adding new ones.

          Show
          githubbot ASF GitHub Bot added a comment - Github user macdonst commented on the pull request: https://github.com/apache/cordova-lib/pull/432#issuecomment-213519895 @ktop I did a quick test of this with one of my existing apps and it does add the following lines to AndroidManifest.xml: ``` <application android:isGame="true" android:name="MyApplication" /> <uses-sdk android:maxSdkVersion="22" /> ``` but unfortunately it already had the following lines: ``` <application android:hardwareAccelerated="true" android:icon="@drawable/icon" android:label="@string/app_name" android:supportsRtl="true"> <uses-sdk android:minSdkVersion="14" android:targetSdkVersion="23" /> ``` So in order for this to get merged it has to be able to modify existing lines instead of adding new ones.
          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user ktop opened a pull request:

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

          CB-11023 Add attribute through config-file tag

          Add functionality to be able to add attributes to xml files through config-file tag.

          Syntax:
          Able to add multiple attributes to multiple elements at once (only to direct children of the parent).
          ```
          <config-file target="AndroidManifest.xml" parent="/manifest" attr="true">
          <application android:name="MyApplication" android:isGame="true" />
          <uses-sdk android:maxSdkVersion="22" />
          </config-file>
          ```

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

          $ git pull https://github.com/ktop/cordova-lib cb-11023

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

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


          commit aa3427ce38ef0bd2e2ac71e5d56d7d9d0dc6b15c
          Author: Karen Tran <ktop500@gmail.com>
          Date: 2016-04-22T14:05:12Z

          CB-11023 Add attribute through config-file tag

          Add back comment and fix format


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user ktop opened a pull request: https://github.com/apache/cordova-lib/pull/432 CB-11023 Add attribute through config-file tag Add functionality to be able to add attributes to xml files through config-file tag. Syntax: Able to add multiple attributes to multiple elements at once (only to direct children of the parent). ``` <config-file target="AndroidManifest.xml" parent="/manifest" attr="true"> <application android:name="MyApplication" android:isGame="true" /> <uses-sdk android:maxSdkVersion="22" /> </config-file> ``` You can merge this pull request into a Git repository by running: $ git pull https://github.com/ktop/cordova-lib cb-11023 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/cordova-lib/pull/432.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 #432 commit aa3427ce38ef0bd2e2ac71e5d56d7d9d0dc6b15c Author: Karen Tran <ktop500@gmail.com> Date: 2016-04-22T14:05:12Z CB-11023 Add attribute through config-file tag Add back comment and fix format

            People

            • Assignee:
              ktop500 Karen Tran
              Reporter:
              csantana Carlos Santana
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development