Details

    • Type: Improvement
    • Status: Open
    • Priority: Major
    • Resolution: Unresolved
    • Affects Version/s: Master
    • Fix Version/s: Master
    • Component/s: CordovaCommon, CordovaLib
    • Labels:
      None

      Description

      edit-config tag in plugin.xml allows users to modify xml elements from plugins.

      This feature should be added to config.xml as well, so that users wouldn't need a plugin in order to modify xml elements.

      *config.xml changes takes precedence over plugin changes.

        Issue Links

          Activity

          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user ktop opened a pull request:

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

          CB-11908 Add edit-config to config.xml

          Handles edit-config for config.xml.

          Fixed up the conflict checking so that config.xml will take precedence over plugin.xml changes.
          Added a remove mode to config.xml only, explained more below.

          To test out these changes, remember to copy it to the platform's cordova-common.

          A couple things to discuss/clarify:
          1. These changes will make config.xml changes take precedence over plugin.xml changes even if --force is used. Is this alright? Or should --force actually force add the plugin anyways?

          2. There's no real way to "undo" edit-config changes in config.xml. As a fix, I've added a "remove" mode that will remove the attributes specified. Do you think this is useful to have? There's no such thing as restoring/reverting the xml elements to what they were before. What was initially the old list of attributes from before the edit-config change will get overwritten on every prepare.

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

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

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

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


          commit d0ce4c506d2f7f24bda145e21f9b9af1543b1ecd
          Author: ktop <ktop500@gmail.com>
          Date: 2016-09-26T21:07:09Z

          CB-11908 Add edit-config to config.xml


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user ktop opened a pull request: https://github.com/apache/cordova-lib/pull/492 CB-11908 Add edit-config to config.xml Handles edit-config for config.xml. Fixed up the conflict checking so that config.xml will take precedence over plugin.xml changes. Added a remove mode to config.xml only, explained more below. To test out these changes, remember to copy it to the platform's cordova-common. A couple things to discuss/clarify: 1. These changes will make config.xml changes take precedence over plugin.xml changes even if --force is used. Is this alright? Or should --force actually force add the plugin anyways? 2. There's no real way to "undo" edit-config changes in config.xml. As a fix, I've added a "remove" mode that will remove the attributes specified. Do you think this is useful to have? There's no such thing as restoring/reverting the xml elements to what they were before. What was initially the old list of attributes from before the edit-config change will get overwritten on every prepare. You can merge this pull request into a Git repository by running: $ git pull https://github.com/ktop/cordova-lib editconfigxml Alternatively you can review and apply these changes as the patch at: https://github.com/apache/cordova-lib/pull/492.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 #492 commit d0ce4c506d2f7f24bda145e21f9b9af1543b1ecd Author: ktop <ktop500@gmail.com> Date: 2016-09-26T21:07:09Z CB-11908 Add edit-config to config.xml
          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user ktop opened a pull request:

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

          CB-11908 Handle edit-config in config.xml on prepare

          Needs this PR: https://github.com/apache/cordova-lib/pull/492

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

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

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

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


          commit 34b50acee4fa9eb27d7225a3e4497c7917570667
          Author: ktop <ktop500@gmail.com>
          Date: 2016-09-26T21:10:15Z

          CB-11908 Handle edit-config in config.xml on prepare


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user ktop opened a pull request: https://github.com/apache/cordova-lib/pull/493 CB-11908 Handle edit-config in config.xml on prepare Needs this PR: https://github.com/apache/cordova-lib/pull/492 You can merge this pull request into a Git repository by running: $ git pull https://github.com/ktop/cordova-lib libEditConfig Alternatively you can review and apply these changes as the patch at: https://github.com/apache/cordova-lib/pull/493.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 #493 commit 34b50acee4fa9eb27d7225a3e4497c7917570667 Author: ktop <ktop500@gmail.com> Date: 2016-09-26T21:10:15Z CB-11908 Handle edit-config in config.xml on prepare
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user stevengill commented on the issue:

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

          JSHint is failing for you

          Show
          githubbot ASF GitHub Bot added a comment - Github user stevengill commented on the issue: https://github.com/apache/cordova-lib/pull/492 JSHint is failing for you
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user stevengill commented on the issue:

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

          Travis and appveyor failures are likely due to missing https://github.com/apache/cordova-lib/pull/492

          Show
          githubbot ASF GitHub Bot added a comment - Github user stevengill commented on the issue: https://github.com/apache/cordova-lib/pull/493 Travis and appveyor failures are likely due to missing https://github.com/apache/cordova-lib/pull/492
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user ktop commented on the issue:

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

          Thanks, I fixed the errors.

          Show
          githubbot ASF GitHub Bot added a comment - Github user ktop commented on the issue: https://github.com/apache/cordova-lib/pull/492 Thanks, I fixed the errors.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user codecov-io commented on the issue:

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

            1. [Current coverage](https://codecov.io/gh/apache/cordova-lib/pull/492?src=pr) is 80.38% (diff: 100%)
              > Merging 492(https://codecov.io/gh/apache/cordova-lib/pull/492?src=pr) into [master](https://codecov.io/gh/apache/cordova-lib/branch/master?src=pr) will not change coverage

          ```diff
          @@ master #492 diff @@
          ==========================================
          Files 67 67
          Lines 5184 5184
          Methods 835 835
          Messages 0 0
          Branches 1005 1005
          ==========================================
          Hits 4167 4167
          Misses 1017 1017
          Partials 0 0
          ```

          > Powered by [Codecov](https://codecov.io?src=pr). Last update [63efd21...de8a367](https://codecov.io/gh/apache/cordova-lib/compare/63efd2174fe20b9c573b23a741257a6170dec3b6...de8a367fe4fe553ace4a72c0b5edb12135589270?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/492 [Current coverage] ( https://codecov.io/gh/apache/cordova-lib/pull/492?src=pr ) is 80.38% (diff: 100%) > Merging 492 ( https://codecov.io/gh/apache/cordova-lib/pull/492?src=pr ) into [master] ( https://codecov.io/gh/apache/cordova-lib/branch/master?src=pr ) will not change coverage ```diff @@ master #492 diff @@ ========================================== Files 67 67 Lines 5184 5184 Methods 835 835 Messages 0 0 Branches 1005 1005 ========================================== Hits 4167 4167 Misses 1017 1017 Partials 0 0 ``` > Powered by [Codecov] ( https://codecov.io?src=pr ). Last update [63efd21...de8a367] ( https://codecov.io/gh/apache/cordova-lib/compare/63efd2174fe20b9c573b23a741257a6170dec3b6...de8a367fe4fe553ace4a72c0b5edb12135589270?src=pr )
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user stevengill commented on the issue:

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

          @ktop so if I remove an `edit-config` tag from my `config.xml`, it won't actually remove it from `AndroidManifest.xml`. Guessing the solution for users in this case would be to rm and re-add the android platform. That seems fine to me.

          This is only for merging and overwriting. For adding new elements, we should use `config-file` still?

          Show
          githubbot ASF GitHub Bot added a comment - Github user stevengill commented on the issue: https://github.com/apache/cordova-lib/pull/492 @ktop so if I remove an `edit-config` tag from my `config.xml`, it won't actually remove it from `AndroidManifest.xml`. Guessing the solution for users in this case would be to rm and re-add the android platform. That seems fine to me. This is only for merging and overwriting. For adding new elements, we should use `config-file` still?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user stevengill commented on the issue:

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

          do you have a usecase example when remove might be useful?

          Show
          githubbot ASF GitHub Bot added a comment - Github user stevengill commented on the issue: https://github.com/apache/cordova-lib/pull/492 do you have a usecase example when remove might be useful?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user stevengill commented on the issue:

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

          `remove` wouldn't be a revert right. Since you have no way to store what got overwritten/merged

          Show
          githubbot ASF GitHub Bot added a comment - Github user stevengill commented on the issue: https://github.com/apache/cordova-lib/pull/492 `remove` wouldn't be a revert right. Since you have no way to store what got overwritten/merged
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user stevengill commented on the issue:

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

          I'm going to merge this in for now so I can get the common release out. But it is definitely worth discussing some of these points.

          Show
          githubbot ASF GitHub Bot added a comment - Github user stevengill commented on the issue: https://github.com/apache/cordova-lib/pull/492 I'm going to merge this in for now so I can get the common release out. But it is definitely worth discussing some of these points.
          Hide
          jira-bot ASF subversion and git services added a comment -

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

          CB-11908 Handle edit-config in config.xml on prepare

          This closes #493

          Show
          jira-bot ASF subversion and git services added a comment - Commit 59faf88024faa697647dec1ddda045db8038edab in cordova-lib's branch refs/heads/master from Karen Tran [ https://git-wip-us.apache.org/repos/asf?p=cordova-lib.git;h=59faf88 ] CB-11908 Handle edit-config in config.xml on prepare This closes #493
          Hide
          jira-bot ASF subversion and git services added a comment -

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

          CB-11908 Add edit-config to config.xml

          This closes #492

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

          Github user asfgit closed the pull request at:

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

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

          Github user asfgit closed the pull request at:

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

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

          Github user stevengill commented on the issue:

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

          Thanks for sending this pr @ktop! I've merged it in.

          Show
          githubbot ASF GitHub Bot added a comment - Github user stevengill commented on the issue: https://github.com/apache/cordova-lib/pull/492 Thanks for sending this pr @ktop! I've merged it in.
          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user ktop opened a pull request:

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

          CB-11908 Add tests for edit-config in config.xml and fix typo

          <!--
          Please make sure the checklist boxes are all checked before submitting the PR. The checklist
          is intended as a quick reference, for complete details please see our Contributor Guidelines:

          http://cordova.apache.org/contribute/contribute_guidelines.html

          Thanks!
          -->

              1. Platforms affected
                cordova-common
              1. What does this PR do?
                I had this commit ready to go, but forgot to commit and push it. Here I'm adding tests for the 'edit-config' functionality that was added to config.xml.
              1. What testing has been done on this change?
                Manual tests and adding unit tests.
              1. Checklist
          • [x ] [Reported an issue](http://cordova.apache.org/contribute/issues.html) in the JIRA database
          • [x ] Commit message follows the format: "CB-3232: (android) Fix bug with resolving file paths", where CB-xxxx is the JIRA ID & "android" is the platform affected.
          • [x ] Added automated test coverage as appropriate for this change.

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

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

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

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


          commit aa4c6e8380c6e5edf1bf20a508b450e64a1d6e7c
          Author: ktop <ktop500@gmail.com>
          Date: 2016-12-09T19:53:50Z

          CB-11908 Add tests for edit-config in config.xml and fix typo

          Add one more test case


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user ktop opened a pull request: https://github.com/apache/cordova-lib/pull/508 CB-11908 Add tests for edit-config in config.xml and fix typo <!-- Please make sure the checklist boxes are all checked before submitting the PR. The checklist is intended as a quick reference, for complete details please see our Contributor Guidelines: http://cordova.apache.org/contribute/contribute_guidelines.html Thanks! --> Platforms affected cordova-common What does this PR do? I had this commit ready to go, but forgot to commit and push it. Here I'm adding tests for the 'edit-config' functionality that was added to config.xml. What testing has been done on this change? Manual tests and adding unit tests. Checklist [x ] [Reported an issue] ( http://cordova.apache.org/contribute/issues.html ) in the JIRA database [x ] Commit message follows the format: " CB-3232 : (android) Fix bug with resolving file paths", where CB-xxxx is the JIRA ID & "android" is the platform affected. [x ] Added automated test coverage as appropriate for this change. You can merge this pull request into a Git repository by running: $ git pull https://github.com/ktop/cordova-lib editconfigxml Alternatively you can review and apply these changes as the patch at: https://github.com/apache/cordova-lib/pull/508.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 #508 commit aa4c6e8380c6e5edf1bf20a508b450e64a1d6e7c Author: ktop <ktop500@gmail.com> Date: 2016-12-09T19:53:50Z CB-11908 Add tests for edit-config in config.xml and fix typo Add one more test case
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user stevengill commented on the issue:

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

          Hey @ktop!

          Any chance you can rebase this? If not, let us know and I can take a look at reimplementing.

          Show
          githubbot ASF GitHub Bot added a comment - Github user stevengill commented on the issue: https://github.com/apache/cordova-lib/pull/508 Hey @ktop! Any chance you can rebase this? If not, let us know and I can take a look at reimplementing.

            People

            • Assignee:
              ktop500 Karen Tran
              Reporter:
              ktop500 Karen Tran
            • Votes:
              2 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:

                Development