You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cordova.apache.org by matrosov-nikita <gi...@git.apache.org> on 2016/10/31 14:15:02 UTC
[GitHub] cordova-windows pull request #207: Move windows-specific logic from cordova-...
GitHub user matrosov-nikita opened a pull request:
https://github.com/apache/cordova-windows/pull/207
Move windows-specific logic from cordova-common
<!--
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!
-->
### What does this PR do?
This PR moves windows-specific logic from cordova-common to cordova-windows. This logic is associated with demuxing 'package.appxmanifest' into relevant platform-specific appx manifests. Also auto-test was moved and edited according changes.
### What testing has been done on this change?
Auto test
### 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/matrosov-nikita/cordova-windows remove-windows-logic
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cordova-windows/pull/207.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 #207
----
commit b101deda784bfdb3ad45c2f86b830c6cc356a72b
Author: Nikita Matrosov <ma...@gmail.com>
Date: 2016-10-31T13:57:24Z
Move windows-specific logic from 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-windows pull request #207: CB-12142: Move windows-specific logic fro...
Posted by vladimir-kotikov <gi...@git.apache.org>.
Github user vladimir-kotikov commented on a diff in the pull request:
https://github.com/apache/cordova-windows/pull/207#discussion_r87808967
--- Diff: template/cordova/lib/PluginInfo.js ---
@@ -0,0 +1,150 @@
+var semver = require('semver');
+var CommonPluginInfo = require('cordova-common').PluginInfo;
+
+var MANIFESTS = {
+ 'windows': {
+ '8.1.0': 'package.windows.appxmanifest',
+ '10.0.0': 'package.windows10.appxmanifest'
+ },
+ 'phone': {
+ '8.1.0': 'package.phone.appxmanifest',
+ '10.0.0': 'package.windows10.appxmanifest'
+ },
+ 'all': {
+ '8.1.0': ['package.windows.appxmanifest', 'package.phone.appxmanifest'],
+ '10.0.0': 'package.windows10.appxmanifest'
+ }
+};
+
+var SUBSTS = ['package.phone.appxmanifest', 'package.windows.appxmanifest', 'package.windows10.appxmanifest'];
+var TARGETS = ['windows', 'phone', 'all'];
+/*
+A class for holidng the information currently stored in plugin.xml
+It's inherited from cordova-common's PluginInfo class
+In addition it overrides getConfigFiles method to use windows-specific logic
+ */
+function PluginInfo(dirname) {
+ // We needn't use 'util.inherit' in this case because there aren't any methods on prototype in cordova-common's PluginInfo class
+ CommonPluginInfo.apply(this, arguments);
+ var parentGetConfigFiles = this.getConfigFiles;
+ var parentGetEditConfigs = this.getEditConfigs;
+
+ this.getEditConfigs = function(platform) {
+ var editConfigs = parentGetEditConfigs(platform);
+ return processChanges(editConfigs);
+ };
+
+ this.getConfigFiles = function(platform) {
+ var configFiles = parentGetConfigFiles(platform);
+ return processChanges(configFiles);
+ };
+
+ function processChanges(changes) {
+ var hasManifestChanges = checkManifestChanges(changes);
+
+ // Demux 'package.appxmanifest' into relevant platform-specific appx manifests.
+ // Only spend the cycles if there are version-specific plugin settings
+ if (hasManifestChanges)
+ {
+ var oldChanges = changes;
+ changes = [];
+
+ oldChanges.forEach(function(change, changeIndex) {
+ // Only support semver/device-target demux for package.appxmanifest
+ // Pass through in case something downstream wants to use it
+ if (change.target !== 'package.appxmanifest') {
+ changes.push(change);
+ return;
+ }
+
+ // No semver/device-target for this config-file, pass it through
+ if (!(hasChangeVersion(change) || hasChangeTarget(change))) {
+ // New windows template separate manifest files for Windows10, Windows8.1 and WP8.1
+ changes = changes.concat(demuxChangeWithSubsts(change));
+ return;
+ }
+
+ var targetDeviceSet = getDeviceTargetSetForChange(change);
+ changes = replaceChangeTarget(targetDeviceSet, change, changes);
+ });
+ }
+ return changes;
+ }
+
+ function checkManifestChanges(changes) {
--- End diff --
You're calling this only once - is there a real need to wrap this into function?
---
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-windows pull request #207: Move windows-specific logic from cordova-...
Posted by vladimir-kotikov <gi...@git.apache.org>.
Github user vladimir-kotikov commented on a diff in the pull request:
https://github.com/apache/cordova-windows/pull/207#discussion_r87752005
--- Diff: template/cordova/lib/PluginInfo.js ---
@@ -0,0 +1,109 @@
+var semver = require('semver');
+var CommonPluginInfo = require('cordova-common').PluginInfo;
+
+var MANIFESTS = {
+ 'windows': {
+ '8.1.0': 'package.windows.appxmanifest',
+ '10.0.0': 'package.windows10.appxmanifest'
+ },
+ 'phone': {
+ '8.1.0': 'package.phone.appxmanifest',
+ '10.0.0': 'package.windows10.appxmanifest'
+ },
+ 'all': {
+ '8.1.0': ['package.windows.appxmanifest', 'package.phone.appxmanifest'],
+ '10.0.0': 'package.windows10.appxmanifest'
+ }
+};
+
+/*
+A class for holidng the information currently stored in plugin.xml
+It's inherited from cordova-common's PluginInfo class
+In addition it overrides getConfigFiles method to use windows-specific logic
+ */
+function PluginInfo(dirname) {
+ CommonPluginInfo.apply(this, arguments);
+ var parentGetConfigFiles = this.getConfigFiles;
+ this.getConfigFiles = function(platform) {
+ var changes = parentGetConfigFiles(platform);
+ var editChanges = this.getEditConfigs(platform);
+ if (editChanges && editChanges.length !== 0) {
+ changes.push(editChanges);
+ }
+ // Demux 'package.appxmanifest' into relevant platform-specific appx manifests.
+ // Only spend the cycles if there are version-specific plugin settings
+ if (changes.some(function(change) {
+ return change.target === 'package.appxmanifest';
--- End diff --
@daserge, i think this condition has been moved to [line 50](https://github.com/apache/cordova-windows/pull/207/files#diff-93a26ea6e269125a5b151c4176a3b42bR50)
---
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-windows issue #207: Move windows-specific logic from cordova-common
Posted by codecov-io <gi...@git.apache.org>.
Github user codecov-io commented on the issue:
https://github.com/apache/cordova-windows/pull/207
## [Current coverage](https://codecov.io/gh/apache/cordova-windows/pull/207?src=pr) is 75.35% (diff: 95.77%)
> Merging [#207](https://codecov.io/gh/apache/cordova-windows/pull/207?src=pr) into [master](https://codecov.io/gh/apache/cordova-windows/branch/master?src=pr) will increase coverage by **0.70%**
```diff
@@ master #207 diff @@
==========================================
Files 15 16 +1
Lines 2067 2138 +71
Methods 390 409 +19
Messages 0 0
Branches 403 410 +7
==========================================
+ Hits 1543 1611 +68
- Misses 524 527 +3
Partials 0 0
```
> Powered by [Codecov](https://codecov.io?src=pr). Last update [7ed3c99...6019414](https://codecov.io/gh/apache/cordova-windows/compare/7ed3c997af55d818354f5b5c38453b7d76ec4aa7...601941453fa52b7715674baf0a57a9430c692a20?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-windows pull request #207: CB-12142: Move windows-specific logic fro...
Posted by vladimir-kotikov <gi...@git.apache.org>.
Github user vladimir-kotikov commented on a diff in the pull request:
https://github.com/apache/cordova-windows/pull/207#discussion_r87809027
--- Diff: template/cordova/lib/PluginInfo.js ---
@@ -0,0 +1,150 @@
+var semver = require('semver');
+var CommonPluginInfo = require('cordova-common').PluginInfo;
+
+var MANIFESTS = {
+ 'windows': {
+ '8.1.0': 'package.windows.appxmanifest',
+ '10.0.0': 'package.windows10.appxmanifest'
+ },
+ 'phone': {
+ '8.1.0': 'package.phone.appxmanifest',
+ '10.0.0': 'package.windows10.appxmanifest'
+ },
+ 'all': {
+ '8.1.0': ['package.windows.appxmanifest', 'package.phone.appxmanifest'],
+ '10.0.0': 'package.windows10.appxmanifest'
+ }
+};
+
+var SUBSTS = ['package.phone.appxmanifest', 'package.windows.appxmanifest', 'package.windows10.appxmanifest'];
+var TARGETS = ['windows', 'phone', 'all'];
+/*
+A class for holidng the information currently stored in plugin.xml
+It's inherited from cordova-common's PluginInfo class
+In addition it overrides getConfigFiles method to use windows-specific logic
+ */
+function PluginInfo(dirname) {
+ // We needn't use 'util.inherit' in this case because there aren't any methods on prototype in cordova-common's PluginInfo class
+ CommonPluginInfo.apply(this, arguments);
+ var parentGetConfigFiles = this.getConfigFiles;
+ var parentGetEditConfigs = this.getEditConfigs;
+
+ this.getEditConfigs = function(platform) {
+ var editConfigs = parentGetEditConfigs(platform);
+ return processChanges(editConfigs);
+ };
+
+ this.getConfigFiles = function(platform) {
+ var configFiles = parentGetConfigFiles(platform);
+ return processChanges(configFiles);
+ };
+
+ function processChanges(changes) {
+ var hasManifestChanges = checkManifestChanges(changes);
+
+ // Demux 'package.appxmanifest' into relevant platform-specific appx manifests.
+ // Only spend the cycles if there are version-specific plugin settings
+ if (hasManifestChanges)
+ {
+ var oldChanges = changes;
+ changes = [];
+
+ oldChanges.forEach(function(change, changeIndex) {
+ // Only support semver/device-target demux for package.appxmanifest
+ // Pass through in case something downstream wants to use it
+ if (change.target !== 'package.appxmanifest') {
+ changes.push(change);
+ return;
+ }
+
+ // No semver/device-target for this config-file, pass it through
+ if (!(hasChangeVersion(change) || hasChangeTarget(change))) {
+ // New windows template separate manifest files for Windows10, Windows8.1 and WP8.1
+ changes = changes.concat(demuxChangeWithSubsts(change));
+ return;
+ }
+
+ var targetDeviceSet = getDeviceTargetSetForChange(change);
+ changes = replaceChangeTarget(targetDeviceSet, change, changes);
+ });
+ }
+ return changes;
+ }
+
+ function checkManifestChanges(changes) {
+ return changes.some(function(change) {
+ return change.target === 'package.appxmanifest';
+ });
+ }
+ function hasChangeVersion(change) {
+ return (typeof change.versions !== 'undefined');
+ }
+
+ function hasChangeTarget(change) {
--- End diff --
Same here
---
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-windows pull request #207: Move windows-specific logic from cordova-...
Posted by vladimir-kotikov <gi...@git.apache.org>.
Github user vladimir-kotikov commented on a diff in the pull request:
https://github.com/apache/cordova-windows/pull/207#discussion_r87752779
--- Diff: template/cordova/lib/PluginInfo.js ---
@@ -0,0 +1,109 @@
+var semver = require('semver');
+var CommonPluginInfo = require('cordova-common').PluginInfo;
+
+var MANIFESTS = {
+ 'windows': {
+ '8.1.0': 'package.windows.appxmanifest',
+ '10.0.0': 'package.windows10.appxmanifest'
+ },
+ 'phone': {
+ '8.1.0': 'package.phone.appxmanifest',
+ '10.0.0': 'package.windows10.appxmanifest'
+ },
+ 'all': {
+ '8.1.0': ['package.windows.appxmanifest', 'package.phone.appxmanifest'],
+ '10.0.0': 'package.windows10.appxmanifest'
+ }
+};
+
+/*
+A class for holidng the information currently stored in plugin.xml
+It's inherited from cordova-common's PluginInfo class
+In addition it overrides getConfigFiles method to use windows-specific logic
+ */
+function PluginInfo(dirname) {
+ CommonPluginInfo.apply(this, arguments);
--- End diff --
Pls add a comment, clarifying why you're not using `util.inherit` to build a children class
---
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-windows issue #207: Move windows-specific logic from cordova-common
Posted by vladimir-kotikov <gi...@git.apache.org>.
Github user vladimir-kotikov commented on the issue:
https://github.com/apache/cordova-windows/pull/207
> Do I get it right that it will require us to release cordova-common before platform release?
@matrosov-nikita, I think the idea was to make it so updated cordova-windows will continue to work w/ current cordova-common, right?
---
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-windows issue #207: Move windows-specific logic from cordova-common
Posted by matrosov-nikita <gi...@git.apache.org>.
Github user matrosov-nikita commented on the issue:
https://github.com/apache/cordova-windows/pull/207
@vladimir-kotikov, yes, you are right. Have you any questions or concerns about this matter?
---
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-windows pull request #207: Move windows-specific logic from cordova-...
Posted by vladimir-kotikov <gi...@git.apache.org>.
Github user vladimir-kotikov commented on a diff in the pull request:
https://github.com/apache/cordova-windows/pull/207#discussion_r87752512
--- Diff: template/cordova/lib/PluginInfo.js ---
@@ -0,0 +1,109 @@
+var semver = require('semver');
+var CommonPluginInfo = require('cordova-common').PluginInfo;
+
+var MANIFESTS = {
+ 'windows': {
+ '8.1.0': 'package.windows.appxmanifest',
+ '10.0.0': 'package.windows10.appxmanifest'
+ },
+ 'phone': {
+ '8.1.0': 'package.phone.appxmanifest',
+ '10.0.0': 'package.windows10.appxmanifest'
+ },
+ 'all': {
+ '8.1.0': ['package.windows.appxmanifest', 'package.phone.appxmanifest'],
+ '10.0.0': 'package.windows10.appxmanifest'
+ }
+};
+
+/*
+A class for holidng the information currently stored in plugin.xml
+It's inherited from cordova-common's PluginInfo class
+In addition it overrides getConfigFiles method to use windows-specific logic
+ */
+function PluginInfo(dirname) {
+ CommonPluginInfo.apply(this, arguments);
+ var parentGetConfigFiles = this.getConfigFiles;
+ this.getConfigFiles = function(platform) {
+ var changes = parentGetConfigFiles(platform);
+ var editChanges = this.getEditConfigs(platform);
+ if (editChanges && editChanges.length !== 0) {
+ changes.push(editChanges);
+ }
+ // Demux 'package.appxmanifest' into relevant platform-specific appx manifests.
+ // Only spend the cycles if there are version-specific plugin settings
+ if (changes.some(function(change) {
+ return change.target === 'package.appxmanifest';
+ }))
+ {
+ var oldChanges = changes;
+ changes = [];
+
+ oldChanges.forEach(function(change, changeIndex) {
+ // Only support semver/device-target demux for package.appxmanifest
+ // Pass through in case something downstream wants to use it
+ if (change.target !== 'package.appxmanifest') {
+ changes.push(change);
+ return;
+ }
+
+ var hasVersion = (typeof change.versions !== 'undefined');
+ var hasTargets = (typeof change.deviceTarget !== 'undefined');
+
+ // No semver/device-target for this config-file, pass it through
+ if (!(hasVersion || hasTargets)) {
+ // New windows template separate manifest files for Windows10, Windows8.1 and WP8.1
+ var substs = ['package.phone.appxmanifest', 'package.windows.appxmanifest', 'package.windows10.appxmanifest'];
+ changes = changes.concat(substs.map(function(subst) {
+ return Object.assign({}, change, {target: subst});
+ }));
+ return changes;
--- End diff --
Nit: you can just `return` here - result of this function is not used anywhere
---
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-windows pull request #207: Move windows-specific logic from cordova-...
Posted by matrosov-nikita <gi...@git.apache.org>.
Github user matrosov-nikita commented on a diff in the pull request:
https://github.com/apache/cordova-windows/pull/207#discussion_r87713717
--- Diff: template/cordova/lib/PluginInfo.js ---
@@ -0,0 +1,109 @@
+var semver = require('semver');
+var CommonPluginInfo = require('cordova-common').PluginInfo;
+
+var MANIFESTS = {
+ 'windows': {
+ '8.1.0': 'package.windows.appxmanifest',
+ '10.0.0': 'package.windows10.appxmanifest'
+ },
+ 'phone': {
+ '8.1.0': 'package.phone.appxmanifest',
+ '10.0.0': 'package.windows10.appxmanifest'
+ },
+ 'all': {
+ '8.1.0': ['package.windows.appxmanifest', 'package.phone.appxmanifest'],
+ '10.0.0': 'package.windows10.appxmanifest'
+ }
+};
+
+/*
+A class for holidng the information currently stored in plugin.xml
+It's inherited from cordova-common's PluginInfo class
+In addition it overrides getConfigFiles method to use windows-specific logic
+ */
+function PluginInfo(dirname) {
+ CommonPluginInfo.apply(this, arguments);
+ var parentGetConfigFiles = this.getConfigFiles;
+ this.getConfigFiles = function(platform) {
+ var changes = parentGetConfigFiles(platform);
+ var editChanges = this.getEditConfigs(platform);
+ if (editChanges && editChanges.length !== 0) {
+ changes.push(editChanges);
+ }
+ // Demux 'package.appxmanifest' into relevant platform-specific appx manifests.
+ // Only spend the cycles if there are version-specific plugin settings
+ if (changes.some(function(change) {
+ return change.target === 'package.appxmanifest';
--- End diff --
In cordova-common we'll get ready changes and push them again. (see also https://github.com/apache/cordova-lib/blob/master/cordova-common/src/ConfigChanges/ConfigChanges.js#L365). So, I suppose, that we needn't release cordova-common before platform release.
---
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-windows issue #207: CB-12142: Move windows-specific logic from cordo...
Posted by vladimir-kotikov <gi...@git.apache.org>.
Github user vladimir-kotikov commented on the issue:
https://github.com/apache/cordova-windows/pull/207
Looks good. Let's wait for tomorrow and merge it.
---
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-windows pull request #207: Move windows-specific logic from cordova-...
Posted by matrosov-nikita <gi...@git.apache.org>.
Github user matrosov-nikita commented on a diff in the pull request:
https://github.com/apache/cordova-windows/pull/207#discussion_r87713341
--- Diff: template/cordova/lib/PluginInfo.js ---
@@ -0,0 +1,109 @@
+var semver = require('semver');
+var CommonPluginInfo = require('cordova-common').PluginInfo;
+
+var MANIFESTS = {
+ 'windows': {
+ '8.1.0': 'package.windows.appxmanifest',
+ '10.0.0': 'package.windows10.appxmanifest'
+ },
+ 'phone': {
+ '8.1.0': 'package.phone.appxmanifest',
+ '10.0.0': 'package.windows10.appxmanifest'
+ },
+ 'all': {
+ '8.1.0': ['package.windows.appxmanifest', 'package.phone.appxmanifest'],
+ '10.0.0': 'package.windows10.appxmanifest'
+ }
+};
+
+/*
+A class for holidng the information currently stored in plugin.xml
+It's inherited from cordova-common's PluginInfo class
+In addition it overrides getConfigFiles method to use windows-specific logic
+ */
+function PluginInfo(dirname) {
+ CommonPluginInfo.apply(this, arguments);
+ var parentGetConfigFiles = this.getConfigFiles;
+ this.getConfigFiles = function(platform) {
+ var changes = parentGetConfigFiles(platform);
+ var editChanges = this.getEditConfigs(platform);
+ if (editChanges && editChanges.length !== 0) {
+ changes.push(editChanges);
+ }
+ // Demux 'package.appxmanifest' into relevant platform-specific appx manifests.
+ // Only spend the cycles if there are version-specific plugin settings
+ if (changes.some(function(change) {
+ return change.target === 'package.appxmanifest';
--- End diff --
Changes w/ target 'package.appxmanifest' and w/o versions and deviceTarget used to demux into manifests in remove_plugin_changes and munge_helper methods. This logic is moved in PluginInfo.js (see also https://github.com/apache/cordova-windows/pull/207/commits/b101deda784bfdb3ad45c2f86b830c6cc356a72b?diff=unified#diff-93a26ea6e269125a5b151c4176a3b42bR54).
Once windows-specific logic is completed there aren't any changes with 'package.appxmanifest' target.
---
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-windows pull request #207: Move windows-specific logic from cordova-...
Posted by vladimir-kotikov <gi...@git.apache.org>.
Github user vladimir-kotikov commented on a diff in the pull request:
https://github.com/apache/cordova-windows/pull/207#discussion_r87751321
--- Diff: template/cordova/lib/PluginInfo.js ---
@@ -0,0 +1,109 @@
+var semver = require('semver');
+var CommonPluginInfo = require('cordova-common').PluginInfo;
+
+var MANIFESTS = {
+ 'windows': {
+ '8.1.0': 'package.windows.appxmanifest',
+ '10.0.0': 'package.windows10.appxmanifest'
+ },
+ 'phone': {
+ '8.1.0': 'package.phone.appxmanifest',
+ '10.0.0': 'package.windows10.appxmanifest'
+ },
+ 'all': {
+ '8.1.0': ['package.windows.appxmanifest', 'package.phone.appxmanifest'],
+ '10.0.0': 'package.windows10.appxmanifest'
+ }
+};
+
+/*
+A class for holidng the information currently stored in plugin.xml
+It's inherited from cordova-common's PluginInfo class
+In addition it overrides getConfigFiles method to use windows-specific logic
+ */
+function PluginInfo(dirname) {
+ CommonPluginInfo.apply(this, arguments);
+ var parentGetConfigFiles = this.getConfigFiles;
--- End diff --
Rather than saving this into a variable i'd propose to call `ThisClass.super_.prototype.generate_plugin_config_munge` method directly
---
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-windows pull request #207: Move windows-specific logic from cordova-...
Posted by vladimir-kotikov <gi...@git.apache.org>.
Github user vladimir-kotikov commented on a diff in the pull request:
https://github.com/apache/cordova-windows/pull/207#discussion_r87752877
--- Diff: template/cordova/lib/PluginInfo.js ---
@@ -0,0 +1,109 @@
+var semver = require('semver');
+var CommonPluginInfo = require('cordova-common').PluginInfo;
+
+var MANIFESTS = {
+ 'windows': {
+ '8.1.0': 'package.windows.appxmanifest',
+ '10.0.0': 'package.windows10.appxmanifest'
+ },
+ 'phone': {
+ '8.1.0': 'package.phone.appxmanifest',
+ '10.0.0': 'package.windows10.appxmanifest'
+ },
+ 'all': {
+ '8.1.0': ['package.windows.appxmanifest', 'package.phone.appxmanifest'],
+ '10.0.0': 'package.windows10.appxmanifest'
+ }
+};
+
+/*
+A class for holidng the information currently stored in plugin.xml
+It's inherited from cordova-common's PluginInfo class
+In addition it overrides getConfigFiles method to use windows-specific logic
+ */
+function PluginInfo(dirname) {
+ CommonPluginInfo.apply(this, arguments);
+ var parentGetConfigFiles = this.getConfigFiles;
+ this.getConfigFiles = function(platform) {
+ var changes = parentGetConfigFiles(platform);
+ var editChanges = this.getEditConfigs(platform);
+ if (editChanges && editChanges.length !== 0) {
+ changes.push(editChanges);
+ }
+ // Demux 'package.appxmanifest' into relevant platform-specific appx manifests.
+ // Only spend the cycles if there are version-specific plugin settings
+ if (changes.some(function(change) {
+ return change.target === 'package.appxmanifest';
+ }))
+ {
+ var oldChanges = changes;
+ changes = [];
+
+ oldChanges.forEach(function(change, changeIndex) {
--- End diff --
A general comment - can you please divide this function into smaller pieces, so it'd be easier to review and test?
---
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-windows pull request #207: Move windows-specific logic from cordova-...
Posted by vladimir-kotikov <gi...@git.apache.org>.
Github user vladimir-kotikov commented on a diff in the pull request:
https://github.com/apache/cordova-windows/pull/207#discussion_r87751494
--- Diff: template/cordova/lib/PluginInfo.js ---
@@ -0,0 +1,109 @@
+var semver = require('semver');
+var CommonPluginInfo = require('cordova-common').PluginInfo;
+
+var MANIFESTS = {
+ 'windows': {
+ '8.1.0': 'package.windows.appxmanifest',
+ '10.0.0': 'package.windows10.appxmanifest'
+ },
+ 'phone': {
+ '8.1.0': 'package.phone.appxmanifest',
+ '10.0.0': 'package.windows10.appxmanifest'
+ },
+ 'all': {
+ '8.1.0': ['package.windows.appxmanifest', 'package.phone.appxmanifest'],
+ '10.0.0': 'package.windows10.appxmanifest'
+ }
+};
+
+/*
+A class for holidng the information currently stored in plugin.xml
+It's inherited from cordova-common's PluginInfo class
+In addition it overrides getConfigFiles method to use windows-specific logic
+ */
+function PluginInfo(dirname) {
+ CommonPluginInfo.apply(this, arguments);
+ var parentGetConfigFiles = this.getConfigFiles;
+ this.getConfigFiles = function(platform) {
+ var changes = parentGetConfigFiles(platform);
+ var editChanges = this.getEditConfigs(platform);
--- End diff --
I don't think that we should do something with `<edit-config>` tags here. You'd probably need to override parent `getEditConfigs` method in a same way as `getConfigFiles` possibly sharing the similar logic between them.
---
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-windows pull request #207: CB-12142: Move windows-specific logic fro...
Posted by vladimir-kotikov <gi...@git.apache.org>.
Github user vladimir-kotikov commented on a diff in the pull request:
https://github.com/apache/cordova-windows/pull/207#discussion_r87810296
--- Diff: template/cordova/lib/PluginInfo.js ---
@@ -0,0 +1,150 @@
+var semver = require('semver');
+var CommonPluginInfo = require('cordova-common').PluginInfo;
+
+var MANIFESTS = {
+ 'windows': {
+ '8.1.0': 'package.windows.appxmanifest',
+ '10.0.0': 'package.windows10.appxmanifest'
+ },
+ 'phone': {
+ '8.1.0': 'package.phone.appxmanifest',
+ '10.0.0': 'package.windows10.appxmanifest'
+ },
+ 'all': {
+ '8.1.0': ['package.windows.appxmanifest', 'package.phone.appxmanifest'],
+ '10.0.0': 'package.windows10.appxmanifest'
+ }
+};
+
+var SUBSTS = ['package.phone.appxmanifest', 'package.windows.appxmanifest', 'package.windows10.appxmanifest'];
+var TARGETS = ['windows', 'phone', 'all'];
+/*
+A class for holidng the information currently stored in plugin.xml
+It's inherited from cordova-common's PluginInfo class
+In addition it overrides getConfigFiles method to use windows-specific logic
+ */
+function PluginInfo(dirname) {
+ // We needn't use 'util.inherit' in this case because there aren't any methods on prototype in cordova-common's PluginInfo class
+ CommonPluginInfo.apply(this, arguments);
+ var parentGetConfigFiles = this.getConfigFiles;
+ var parentGetEditConfigs = this.getEditConfigs;
+
+ this.getEditConfigs = function(platform) {
+ var editConfigs = parentGetEditConfigs(platform);
+ return processChanges(editConfigs);
+ };
+
+ this.getConfigFiles = function(platform) {
+ var configFiles = parentGetConfigFiles(platform);
+ return processChanges(configFiles);
+ };
+
+ function processChanges(changes) {
+ var hasManifestChanges = checkManifestChanges(changes);
+
+ // Demux 'package.appxmanifest' into relevant platform-specific appx manifests.
+ // Only spend the cycles if there are version-specific plugin settings
+ if (hasManifestChanges)
+ {
+ var oldChanges = changes;
+ changes = [];
+
+ oldChanges.forEach(function(change, changeIndex) {
+ // Only support semver/device-target demux for package.appxmanifest
+ // Pass through in case something downstream wants to use it
+ if (change.target !== 'package.appxmanifest') {
+ changes.push(change);
+ return;
+ }
+
+ // No semver/device-target for this config-file, pass it through
+ if (!(hasChangeVersion(change) || hasChangeTarget(change))) {
+ // New windows template separate manifest files for Windows10, Windows8.1 and WP8.1
+ changes = changes.concat(demuxChangeWithSubsts(change));
+ return;
+ }
+
+ var targetDeviceSet = getDeviceTargetSetForChange(change);
+ changes = replaceChangeTarget(targetDeviceSet, change, changes);
--- End diff --
Consider renaming `replaceChangeTarget` to something more appropriate as it doesn't really replace anything in the original array
---
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-windows pull request #207: Move windows-specific logic from cordova-...
Posted by vladimir-kotikov <gi...@git.apache.org>.
Github user vladimir-kotikov commented on a diff in the pull request:
https://github.com/apache/cordova-windows/pull/207#discussion_r87751813
--- Diff: template/cordova/lib/PluginInfo.js ---
@@ -0,0 +1,109 @@
+var semver = require('semver');
+var CommonPluginInfo = require('cordova-common').PluginInfo;
+
+var MANIFESTS = {
+ 'windows': {
+ '8.1.0': 'package.windows.appxmanifest',
+ '10.0.0': 'package.windows10.appxmanifest'
+ },
+ 'phone': {
+ '8.1.0': 'package.phone.appxmanifest',
+ '10.0.0': 'package.windows10.appxmanifest'
+ },
+ 'all': {
+ '8.1.0': ['package.windows.appxmanifest', 'package.phone.appxmanifest'],
+ '10.0.0': 'package.windows10.appxmanifest'
+ }
+};
+
+/*
+A class for holidng the information currently stored in plugin.xml
+It's inherited from cordova-common's PluginInfo class
+In addition it overrides getConfigFiles method to use windows-specific logic
+ */
+function PluginInfo(dirname) {
+ CommonPluginInfo.apply(this, arguments);
+ var parentGetConfigFiles = this.getConfigFiles;
+ this.getConfigFiles = function(platform) {
+ var changes = parentGetConfigFiles(platform);
+ var editChanges = this.getEditConfigs(platform);
+ if (editChanges && editChanges.length !== 0) {
+ changes.push(editChanges);
+ }
+ // Demux 'package.appxmanifest' into relevant platform-specific appx manifests.
+ // Only spend the cycles if there are version-specific plugin settings
+ if (changes.some(function(change) {
--- End diff --
Nit: could you please invert `if` condition here? Also it might be more convenient to assign this condition to intermediate variable, something like `var hasManifestChanges = changes.some(function(change) { return change.target === 'package.appxmanifest' })`
---
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-windows pull request #207: CB-12142: Move windows-specific logic fro...
Posted by vladimir-kotikov <gi...@git.apache.org>.
Github user vladimir-kotikov commented on a diff in the pull request:
https://github.com/apache/cordova-windows/pull/207#discussion_r87807973
--- Diff: template/cordova/lib/PluginInfo.js ---
@@ -0,0 +1,150 @@
+var semver = require('semver');
+var CommonPluginInfo = require('cordova-common').PluginInfo;
+
+var MANIFESTS = {
+ 'windows': {
+ '8.1.0': 'package.windows.appxmanifest',
+ '10.0.0': 'package.windows10.appxmanifest'
+ },
+ 'phone': {
+ '8.1.0': 'package.phone.appxmanifest',
+ '10.0.0': 'package.windows10.appxmanifest'
+ },
+ 'all': {
+ '8.1.0': ['package.windows.appxmanifest', 'package.phone.appxmanifest'],
+ '10.0.0': 'package.windows10.appxmanifest'
+ }
+};
+
+var SUBSTS = ['package.phone.appxmanifest', 'package.windows.appxmanifest', 'package.windows10.appxmanifest'];
+var TARGETS = ['windows', 'phone', 'all'];
+/*
+A class for holidng the information currently stored in plugin.xml
+It's inherited from cordova-common's PluginInfo class
+In addition it overrides getConfigFiles method to use windows-specific logic
+ */
+function PluginInfo(dirname) {
+ // We needn't use 'util.inherit' in this case because there aren't any methods on prototype in cordova-common's PluginInfo class
+ CommonPluginInfo.apply(this, arguments);
+ var parentGetConfigFiles = this.getConfigFiles;
+ var parentGetEditConfigs = this.getEditConfigs;
+
+ this.getEditConfigs = function(platform) {
+ var editConfigs = parentGetEditConfigs(platform);
+ return processChanges(editConfigs);
+ };
+
+ this.getConfigFiles = function(platform) {
+ var configFiles = parentGetConfigFiles(platform);
+ return processChanges(configFiles);
+ };
+
+ function processChanges(changes) {
--- End diff --
I'd suggest to move this out of `PluginInfo` - this way it'd be possible to use this function in tests (using `rewire`)
---
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-windows pull request #207: CB-12142: Move windows-specific logic fro...
Posted by vladimir-kotikov <gi...@git.apache.org>.
Github user vladimir-kotikov commented on a diff in the pull request:
https://github.com/apache/cordova-windows/pull/207#discussion_r87809002
--- Diff: template/cordova/lib/PluginInfo.js ---
@@ -0,0 +1,150 @@
+var semver = require('semver');
+var CommonPluginInfo = require('cordova-common').PluginInfo;
+
+var MANIFESTS = {
+ 'windows': {
+ '8.1.0': 'package.windows.appxmanifest',
+ '10.0.0': 'package.windows10.appxmanifest'
+ },
+ 'phone': {
+ '8.1.0': 'package.phone.appxmanifest',
+ '10.0.0': 'package.windows10.appxmanifest'
+ },
+ 'all': {
+ '8.1.0': ['package.windows.appxmanifest', 'package.phone.appxmanifest'],
+ '10.0.0': 'package.windows10.appxmanifest'
+ }
+};
+
+var SUBSTS = ['package.phone.appxmanifest', 'package.windows.appxmanifest', 'package.windows10.appxmanifest'];
+var TARGETS = ['windows', 'phone', 'all'];
+/*
+A class for holidng the information currently stored in plugin.xml
+It's inherited from cordova-common's PluginInfo class
+In addition it overrides getConfigFiles method to use windows-specific logic
+ */
+function PluginInfo(dirname) {
+ // We needn't use 'util.inherit' in this case because there aren't any methods on prototype in cordova-common's PluginInfo class
+ CommonPluginInfo.apply(this, arguments);
+ var parentGetConfigFiles = this.getConfigFiles;
+ var parentGetEditConfigs = this.getEditConfigs;
+
+ this.getEditConfigs = function(platform) {
+ var editConfigs = parentGetEditConfigs(platform);
+ return processChanges(editConfigs);
+ };
+
+ this.getConfigFiles = function(platform) {
+ var configFiles = parentGetConfigFiles(platform);
+ return processChanges(configFiles);
+ };
+
+ function processChanges(changes) {
+ var hasManifestChanges = checkManifestChanges(changes);
+
+ // Demux 'package.appxmanifest' into relevant platform-specific appx manifests.
+ // Only spend the cycles if there are version-specific plugin settings
+ if (hasManifestChanges)
+ {
+ var oldChanges = changes;
+ changes = [];
+
+ oldChanges.forEach(function(change, changeIndex) {
+ // Only support semver/device-target demux for package.appxmanifest
+ // Pass through in case something downstream wants to use it
+ if (change.target !== 'package.appxmanifest') {
+ changes.push(change);
+ return;
+ }
+
+ // No semver/device-target for this config-file, pass it through
+ if (!(hasChangeVersion(change) || hasChangeTarget(change))) {
+ // New windows template separate manifest files for Windows10, Windows8.1 and WP8.1
+ changes = changes.concat(demuxChangeWithSubsts(change));
+ return;
+ }
+
+ var targetDeviceSet = getDeviceTargetSetForChange(change);
+ changes = replaceChangeTarget(targetDeviceSet, change, changes);
+ });
+ }
+ return changes;
+ }
+
+ function checkManifestChanges(changes) {
+ return changes.some(function(change) {
+ return change.target === 'package.appxmanifest';
+ });
+ }
+ function hasChangeVersion(change) {
--- End diff --
Ditto
---
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-windows issue #207: CB-12142: Move windows-specific logic from cordo...
Posted by matrosov-nikita <gi...@git.apache.org>.
Github user matrosov-nikita commented on the issue:
https://github.com/apache/cordova-windows/pull/207
@vladimir-kotikov @daserge, I've created a Jira issue: https://issues.apache.org/jira/browse/CB-12142, fixed all your comments. Also I think it would be better to add prefix to commit after squashing. Pls, review.
---
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-windows pull request #207: Move windows-specific logic from cordova-...
Posted by vladimir-kotikov <gi...@git.apache.org>.
Github user vladimir-kotikov commented on a diff in the pull request:
https://github.com/apache/cordova-windows/pull/207#discussion_r87750373
--- Diff: template/cordova/lib/ConfigChanges.js ---
@@ -159,4 +160,16 @@ function generateUapCapabilities(capabilities) {
}
+/**
+ * This is an override of generate_plugin_config_munge method from cordova-common's PluginInfo class.
+ * @param {[Object]} pluginInfo instance of 'PluginInfo'
+ * @param {[Object]} vars plugin's variables
+ * @param {[Object]} edit_config_changes
+ * @return {[Object]} plugin's munge
+ */
+PlatformMunger.prototype.generate_plugin_config_munge = function (pluginInfo, vars, edit_config_changes) {
+ var self = this;
--- End diff --
This doesn't seem to be requried - calling a prototype is not a closure so you can safely use `this`
---
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-windows pull request #207: CB-12142: Move windows-specific logic fro...
Posted by vladimir-kotikov <gi...@git.apache.org>.
Github user vladimir-kotikov commented on a diff in the pull request:
https://github.com/apache/cordova-windows/pull/207#discussion_r87810709
--- Diff: template/cordova/lib/PluginInfo.js ---
@@ -0,0 +1,150 @@
+var semver = require('semver');
+var CommonPluginInfo = require('cordova-common').PluginInfo;
+
+var MANIFESTS = {
+ 'windows': {
+ '8.1.0': 'package.windows.appxmanifest',
+ '10.0.0': 'package.windows10.appxmanifest'
+ },
+ 'phone': {
+ '8.1.0': 'package.phone.appxmanifest',
+ '10.0.0': 'package.windows10.appxmanifest'
+ },
+ 'all': {
+ '8.1.0': ['package.windows.appxmanifest', 'package.phone.appxmanifest'],
+ '10.0.0': 'package.windows10.appxmanifest'
+ }
+};
+
+var SUBSTS = ['package.phone.appxmanifest', 'package.windows.appxmanifest', 'package.windows10.appxmanifest'];
+var TARGETS = ['windows', 'phone', 'all'];
+/*
+A class for holidng the information currently stored in plugin.xml
+It's inherited from cordova-common's PluginInfo class
+In addition it overrides getConfigFiles method to use windows-specific logic
+ */
+function PluginInfo(dirname) {
+ // We needn't use 'util.inherit' in this case because there aren't any methods on prototype in cordova-common's PluginInfo class
+ CommonPluginInfo.apply(this, arguments);
+ var parentGetConfigFiles = this.getConfigFiles;
+ var parentGetEditConfigs = this.getEditConfigs;
+
+ this.getEditConfigs = function(platform) {
+ var editConfigs = parentGetEditConfigs(platform);
+ return processChanges(editConfigs);
+ };
+
+ this.getConfigFiles = function(platform) {
+ var configFiles = parentGetConfigFiles(platform);
+ return processChanges(configFiles);
+ };
+
+ function processChanges(changes) {
+ var hasManifestChanges = checkManifestChanges(changes);
+
+ // Demux 'package.appxmanifest' into relevant platform-specific appx manifests.
+ // Only spend the cycles if there are version-specific plugin settings
+ if (hasManifestChanges)
+ {
+ var oldChanges = changes;
+ changes = [];
+
+ oldChanges.forEach(function(change, changeIndex) {
+ // Only support semver/device-target demux for package.appxmanifest
+ // Pass through in case something downstream wants to use it
+ if (change.target !== 'package.appxmanifest') {
+ changes.push(change);
+ return;
+ }
+
+ // No semver/device-target for this config-file, pass it through
+ if (!(hasChangeVersion(change) || hasChangeTarget(change))) {
+ // New windows template separate manifest files for Windows10, Windows8.1 and WP8.1
+ changes = changes.concat(demuxChangeWithSubsts(change));
+ return;
+ }
+
+ var targetDeviceSet = getDeviceTargetSetForChange(change);
+ changes = replaceChangeTarget(targetDeviceSet, change, changes);
+ });
+ }
+ return changes;
+ }
+
+ function checkManifestChanges(changes) {
+ return changes.some(function(change) {
+ return change.target === 'package.appxmanifest';
+ });
+ }
+ function hasChangeVersion(change) {
+ return (typeof change.versions !== 'undefined');
+ }
+
+ function hasChangeTarget(change) {
+ return (typeof change.deviceTarget !== 'undefined');
+ }
+
+ function demuxChangeWithSubsts(change) {
+ return SUBSTS.map(function(subst) {
+ return createChangeWithNewTarget(change, subst);
+ });
+ }
+
+ function createChangeWithNewTarget(change, substTarget) {
+ var changeWithNewTarget = {};
+ Object.keys(change).forEach(function(changeKey) {
+ changeWithNewTarget[changeKey] = change[changeKey];
+ });
+ changeWithNewTarget.target = substTarget;
+ return changeWithNewTarget;
+ }
+
+ function getDeviceTargetSetForChange(change) {
+ var targetDeviceSet = hasChangeTarget(change) ? change.deviceTarget : 'all';
+ if (TARGETS.indexOf(targetDeviceSet) === -1) {
+ // target-device couldn't be resolved, fix it up here to a valid value
+ targetDeviceSet = 'all';
+ }
+ return targetDeviceSet;
+ }
+
+ // This is a local function that creates the new replacement representing the
+ // mutation. Used to save code further down.
+ function createReplacement(manifestFile, originalChange) {
--- End diff --
This looks very similar to `createChangeWithNewTarget` function - consider merging into one function
---
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-windows pull request #207: CB-12142: Move windows-specific logic fro...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/cordova-windows/pull/207
---
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-windows issue #207: Move windows-specific logic from cordova-common
Posted by daserge <gi...@git.apache.org>.
Github user daserge commented on the issue:
https://github.com/apache/cordova-windows/pull/207
Also, @matrosov-nikita could you please create a Jira issue for tracking (http://issues.cordova.io), rename the PR title and add the commit prefix?
---
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-windows pull request #207: Move windows-specific logic from cordova-...
Posted by daserge <gi...@git.apache.org>.
Github user daserge commented on a diff in the pull request:
https://github.com/apache/cordova-windows/pull/207#discussion_r87596875
--- Diff: template/cordova/lib/PluginInfo.js ---
@@ -0,0 +1,109 @@
+var semver = require('semver');
+var CommonPluginInfo = require('cordova-common').PluginInfo;
+
+var MANIFESTS = {
+ 'windows': {
+ '8.1.0': 'package.windows.appxmanifest',
+ '10.0.0': 'package.windows10.appxmanifest'
+ },
+ 'phone': {
+ '8.1.0': 'package.phone.appxmanifest',
+ '10.0.0': 'package.windows10.appxmanifest'
+ },
+ 'all': {
+ '8.1.0': ['package.windows.appxmanifest', 'package.phone.appxmanifest'],
+ '10.0.0': 'package.windows10.appxmanifest'
+ }
+};
+
+/*
+A class for holidng the information currently stored in plugin.xml
+It's inherited from cordova-common's PluginInfo class
+In addition it overrides getConfigFiles method to use windows-specific logic
+ */
+function PluginInfo(dirname) {
+ CommonPluginInfo.apply(this, arguments);
+ var parentGetConfigFiles = this.getConfigFiles;
+ this.getConfigFiles = function(platform) {
+ var changes = parentGetConfigFiles(platform);
+ var editChanges = this.getEditConfigs(platform);
+ if (editChanges && editChanges.length !== 0) {
+ changes.push(editChanges);
+ }
+ // Demux 'package.appxmanifest' into relevant platform-specific appx manifests.
+ // Only spend the cycles if there are version-specific plugin settings
+ if (changes.some(function(change) {
+ return change.target === 'package.appxmanifest';
--- End diff --
@matrosov-nikita could you explain the code change here from
```javascript
changes.some(function(change) {
return ((typeof change.versions !== 'undefined') ||
(typeof change.deviceTarget !== 'undefined'));
}))
```
?
---
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-windows pull request #207: Move windows-specific logic from cordova-...
Posted by daserge <gi...@git.apache.org>.
Github user daserge commented on a diff in the pull request:
https://github.com/apache/cordova-windows/pull/207#discussion_r87598394
--- Diff: spec/unit/ConfigChanges.spec.js ---
@@ -161,3 +165,40 @@ describe('Capabilities within package.windows.appxmanifest', function() {
});
});
+describe('generate_plugin_config_munge for windows project', function() {
+ beforeEach(function() {
+ shell.mkdir('-p', tempDir);
+ shell.cp('-rf', windows_testapp_jsproj, tempDir);
+ });
+
+ afterEach(function() {
+ shell.rm('-rf', tempDir);
+ });
+
+ it('should special case config-file elements for windows', function() {
+ var pluginInfoProvider = new PluginInfoProvider();
+ var munger = new configChanges.PlatformMunger('windows', tempDir, 'unused', null, pluginInfoProvider);
+
+ var munge = munger.generate_plugin_config_munge(pluginInfoProvider.get(configplugin), {});
+ var windows81AppxManifest = munge.files['package.windows.appxmanifest'];
+ var winphone81AppxManifest = munge.files['package.phone.appxmanifest'];
+ var windows10AppxManifest = munge.files['package.windows10.appxmanifest'];
+
+ // 1 comes from versions="=8.1.0" + 1 from versions="=8.1.0" device-target="windows"
+ expect(windows81AppxManifest.parents['/Parent/Capabilities'][0].xml).toBe('<Capability Note="should-exist-for-all-appxmanifest-target-files" />');
+ expect(windows81AppxManifest.parents['/Parent/Capabilities'][1].xml).toBe('<Capability Note="should-exist-for-win81-win-and-phone" />');
+ expect(windows81AppxManifest.parents['/Parent/Capabilities'][2].xml).toBe('<Capability Note="should-exist-for-win81-win-only" />');
+ expect(windows81AppxManifest.parents['/Parent/Capabilities'][3].xml).toBe('<Capability Note="should-exist-for-win10-and-win81-win-and-phone" />');
--- End diff --
Please add checks for capabilities count back - they could prevent situation when we have something extraneous.
`expect(windows81AppxManifest.parents['/Parent/Capabilities'][0].count).toBe(3);`
---
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