You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cordova.apache.org by ktop <gi...@git.apache.org> on 2016/06/03 20:42:21 UTC
[GitHub] cordova-lib pull request #449: CB-11023 Add edit-config functionality
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 <kt...@gmail.com>
Date: 2016-05-26T21:36:23Z
CB-11023 Add edit-config functionality
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib issue #449: CB-11023 Add edit-config functionality
Posted by riknoll <gi...@git.apache.org>.
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.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib issue #449: CB-11023 Add edit-config functionality
Posted by macdonst <gi...@git.apache.org>.
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`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib issue #449: CB-11023 Add edit-config functionality
Posted by ktop <gi...@git.apache.org>.
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?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib issue #449: CB-11023 Add edit-config functionality
Posted by ktop <gi...@git.apache.org>.
Github user ktop commented on the issue:
https://github.com/apache/cordova-lib/pull/449
Cool, let me rebase
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib issue #449: CB-11023 Add edit-config functionality
Posted by riknoll <gi...@git.apache.org>.
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.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib issue #449: CB-11023 Add edit-config functionality
Posted by ktop <gi...@git.apache.org>.
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.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib issue #449: CB-11023 Add edit-config functionality
Posted by ktop <gi...@git.apache.org>.
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.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib issue #449: CB-11023 Add edit-config functionality
Posted by riknoll <gi...@git.apache.org>.
Github user riknoll commented on the issue:
https://github.com/apache/cordova-lib/pull/449
Yep! The guide for writing xml references is [here](https://github.com/apache/cordova-docs/blob/master/doc/docs-STYLEGUIDE.md#xml-references). The file you need to edit is the one I linked in my other comment in the `docs/en/dev` path of the cordova-docs repo. You can just add an `edit-config` section next to the `config-file` one. Tag me in the PR once you open and I can merge it. Once we do a Cordova common release, we can snap the docs and update the website with your changes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib issue #449: CB-11023 Add edit-config functionality
Posted by riknoll <gi...@git.apache.org>.
Github user riknoll commented on the issue:
https://github.com/apache/cordova-lib/pull/449
@ktop the author name on the commit is "unknown"
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib issue #449: CB-11023 Add edit-config functionality
Posted by codecov-io <gi...@git.apache.org>.
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
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request #449: CB-11023 Add edit-config functionality
Posted by riknoll <gi...@git.apache.org>.
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)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request #449: CB-11023 Add edit-config functionality
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/cordova-lib/pull/449
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib issue #449: CB-11023 Add edit-config functionality
Posted by riknoll <gi...@git.apache.org>.
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.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib issue #449: CB-11023 Add edit-config functionality
Posted by riknoll <gi...@git.apache.org>.
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?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib issue #449: CB-11023 Add edit-config functionality
Posted by ktop <gi...@git.apache.org>.
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?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib issue #449: CB-11023 Add edit-config functionality
Posted by macdonst <gi...@git.apache.org>.
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.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib issue #449: CB-11023 Add edit-config functionality
Posted by ktop <gi...@git.apache.org>.
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.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org