Details
-
Type:
Improvement
-
Status: Open
-
Priority:
Major
-
Resolution: Unresolved
-
Affects Version/s: Master
-
Fix Version/s: Master
-
Component/s: cordova-common, cordova-lib
-
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
- is related to
-
CB-11023
plugin.xml should be able to add attribute to an existing element in AndroidManifest.xml
-
- Resolved
-
- relates to
-
CB-11968
add <config-file> support to config.xml
-
- Open
-
- links to
Activity
- All
- Comments
- Work Log
- History
- Activity
- Transitions
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
Github user stevengill commented on the issue:
https://github.com/apache/cordova-lib/pull/492
JSHint is failing for you
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
Github user ktop commented on the issue:
https://github.com/apache/cordova-lib/pull/492
Thanks, I fixed the errors.
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
- [Current coverage](https://codecov.io/gh/apache/cordova-lib/pull/492?src=pr) is 80.38% (diff: 100%)
```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)
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?
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?
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
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.
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
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
Github user asfgit closed the pull request at:
Github user asfgit closed the pull request at:
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.
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
- Platforms affected
-
-
-
- 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 does this PR do?
-
-
-
- What testing has been done on this change?
Manual tests and adding unit tests.
- What testing has been done on this change?
-
-
-
- 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
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.
GitHub user audreyso opened a pull request:
https://github.com/apache/cordova-lib/pull/537
CB-11908: Add tests for edit-config in config.xml, fix typo, and rebased.
Add one more test case
<!--
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
-
-
-
- What does this PR do?
Rebased CB-11908 (add tests for edit-config in config.xml and fix typo).
- What does this PR do?
-
-
-
- What testing has been done on this change?
-
-
-
- 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/audreyso/cordova-lib rebaseCB-11908
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cordova-lib/pull/537.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 #537
commit 116f06f2090342c9914cd9ec3a1b17d422d8ffbf
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
commit a50bbebf416ffb0c3813f7d4bdec1f1c1861bf9c
Author: Audrey So <audreyso@apache.org>
Date: 2017-04-05T22:11:10Z
CB-11908 : updated tests to use jasmine 2.0
Github user codecov-io commented on the issue:
https://github.com/apache/cordova-lib/pull/537
- [Codecov](https://codecov.io/gh/apache/cordova-lib/pull/537?src=pr&el=h1) Report
> Merging 537(https://codecov.io/gh/apache/cordova-lib/pull/537?src=pr&el=desc) into [master](https://codecov.io/gh/apache/cordova-lib/commit/0d637f2d67a95ef709443eaa9d7481eb14f79b8d?src=pr&el=desc) will *not change* coverage.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/cordova-lib/pull/537?src=pr&el=tree)
](https://codecov.io/gh/apache/cordova-lib/pull/537?src=pr&el=tree){kind=link}
```diff
@@ Coverage Diff @@
-
- master #537 +/- ##
=======================================
Coverage 82.42% 82.42%
=======================================
Files 68 68
Lines 5462 5462
Branches 1072 1072
=======================================
Hits 4502 4502
Misses 960 960
```
- master #537 +/- ##
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/cordova-lib/pull/537?src=pr&el=continue).
> *Legend* - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/cordova-lib/pull/537?src=pr&el=footer). Last update [0d637f2...a50bbeb](https://codecov.io/gh/apache/cordova-lib/pull/537?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
Commit 48e65621d64a75c5d6d4bf53406e83a566daf0da in cordova-lib's branch refs/heads/master from Karen Tran
[ https://git-wip-us.apache.org/repos/asf?p=cordova-lib.git;h=48e6562 ]
CB-11908 Add tests for edit-config in config.xml and fix typo
Add one more test case
Commit 93672b7fe9644e8b02d352364d7848deca3c3c69 in cordova-lib's branch refs/heads/master from Audrey So
[ https://git-wip-us.apache.org/repos/asf?p=cordova-lib.git;h=93672b7 ]
CB-11908 : updated tests to use jasmine 2.0
This closes #537, this closes #508
Github user asfgit closed the pull request at:
Github user asfgit closed the pull request at:
This has been notified as public feature already https://cordova.apache.org/news/2016/10/25/tools-release.html.
Any special reason this isn't closed? Also, there is no documentation for it anywhere.
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