You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@cordova.apache.org by GitBox <gi...@apache.org> on 2022/10/19 12:47:00 UTC
[GitHub] [cordova-common] TylerBreau opened a new pull request, #181: Updated PluginInfo to pass through XML attributes.
TylerBreau opened a new pull request, #181:
URL: https://github.com/apache/cordova-common/pull/181
<!--
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
All
### Motivation and Context
<!-- Why is this change required? What problem does it solve? -->
<!-- If it fixes an open issue, please link to the issue here. -->
Currently, adding attributes to the XML tags requires a lot of repos to update.
cordova-common to read the new XML tag.
cordova-lib and cordova cli need to update their dependencies.
People need to update cordova-cli to have the cordova-common update.
Each platform that wants to use the new attribute needs to be updated.
cordova-docs also needs to be updated.
Depending on the number of platforms that needs the update you're looking at 5 or more repos requiring an update, even when it's simply add a brand new attribute to an XML tag.
This PR updates the XML parsing code to passthrough all attributes.
Existing attributes are parsed in the same way there parsed before. New attributes will be passed through without any addition parsing.
The intention is to stop requiring cordova-common updates in order to add attributes to XML tags.
The benefit of this PR is that it removes need to update 3 repos when modifying supported XML tags. Those 3 repos being cordova-common, cordova-lib, and cordova-cli.
### Description
<!-- Describe your changes in detail -->
Copies all attributes from XML tags to the return object in the respective XML tag get functions.
Existing attributes are still parsed in the exact same way as before.
E.X framework attribute "embed" is still passed through isStrTrue.
I don't believe any cordova-docs updates are required.
### Testing
<!-- Please describe in detail how you tested your changes. -->
I've ran npm test on cordova-common, cordova-lib, cordova-cli. lib and cli were updated with the cordova-common change.
I've ran these changes on my company's app.
An update in my company's attribute is requiring me to add an new attribute to the framework tag. I've tested adding a new attribute to cordova-ios with my company's app using these changes.
### Checklist
- [x] I've run the tests to see all new and existing tests pass
- [ ] I added automated test coverage as appropriate for this change
- [x] Commit is prefixed with `(platform)` if this change only applies to one platform (e.g. `(android)`)
- [x] If this Pull Request resolves an issue, I linked to the issue in the text above (and used the correct [keyword to close issues using keywords](https://help.github.com/articles/closing-issues-using-keywords/))
- [x] I've updated the documentation if necessary
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-common] dpogue commented on pull request #181: Updated PluginInfo to pass through XML attributes.
Posted by GitBox <gi...@apache.org>.
dpogue commented on PR #181:
URL: https://github.com/apache/cordova-common/pull/181#issuecomment-1284092885
yeah, it looks like preferences doesn't return an object per-preference tag, it returns an object of all the preferences as key-value pairs.
The preferences stuff is designed to be pretty strictly key/value, so maybe we don't need to handle extra attributes in that particular case.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-common] TylerBreau commented on pull request #181: Updated PluginInfo to pass through XML attributes.
Posted by GitBox <gi...@apache.org>.
TylerBreau commented on PR #181:
URL: https://github.com/apache/cordova-common/pull/181#issuecomment-1284002004
> This looks good, and definitely helps make it easier for platforms to support new attributes in the future!
>
> It would be good to add unit tests to show that custom attributes do, in fact, get passed through and returned by the methods after this change.
Yea I was just looking at the unit tests, seeing there's basically no detailed testing for the XML parsing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-common] breautek merged pull request #181: feat(PluginInfo): pass through XML attributes
Posted by GitBox <gi...@apache.org>.
breautek merged PR #181:
URL: https://github.com/apache/cordova-common/pull/181
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-common] TylerBreau commented on pull request #181: Updated PluginInfo to pass through XML attributes.
Posted by GitBox <gi...@apache.org>.
TylerBreau commented on PR #181:
URL: https://github.com/apache/cordova-common/pull/181#issuecomment-1284383977
> FWIW, I _think_ config.xml is parsed in each platform using its own cordova-common dependency (rather than the one provided by cordova-lib), specifically to work around the issues of versioning in cordova-cli and using older CLI versions with newer platform versions
In my experience with https://github.com/apache/cordova-ios/pull/1266/, cordova-ios seems to rely on the cordova-cli when parsing plugin.xml.
cordova-ios unit tests seemed to rely on its own cordova-common dependency.
Not sure if config.xml is handled differently than plugin.xml.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-common] breautek commented on pull request #181: Updated PluginInfo to pass through XML attributes.
Posted by GitBox <gi...@apache.org>.
breautek commented on PR #181:
URL: https://github.com/apache/cordova-common/pull/181#issuecomment-1284375922
Requested dpogue for review to give him a chance to followup on his suggestion.
Added this to the 5.x milestone, cause I don't think we are planning on doing a 4.1 release, but I don't think this is a breaking change so we could move this to a 4.1 feature release.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-common] dpogue commented on pull request #181: Updated PluginInfo to pass through XML attributes.
Posted by GitBox <gi...@apache.org>.
dpogue commented on PR #181:
URL: https://github.com/apache/cordova-common/pull/181#issuecomment-1283999176
This looks good, and definitely helps make it easier for platforms to support new attributes in the future!
It would be good to add unit tests to show that custom attributes do, in fact, get passed through and returned by the methods after this change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-common] TylerBreau commented on pull request #181: Updated PluginInfo to pass through XML attributes.
Posted by GitBox <gi...@apache.org>.
TylerBreau commented on PR #181:
URL: https://github.com/apache/cordova-common/pull/181#issuecomment-1284098781
> yeah, it looks like preferences doesn't return an object per-preference tag, it returns an object of all the preferences as key-value pairs.
>
> The preferences stuff is designed to be pretty strictly key/value, so maybe we don't need to handle extra attributes in that particular case.
I'm thinking XML passthrough won't deal with preferences. I'll create an issue for documentation and the potential flaw but for this PR i'll just be undoing the support and moving on. Probably leave a comment as well.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-common] breautek commented on pull request #181: Updated PluginInfo to pass through XML attributes.
Posted by GitBox <gi...@apache.org>.
breautek commented on PR #181:
URL: https://github.com/apache/cordova-common/pull/181#issuecomment-1290611088
Talking with Erisu, and we decided to get this into a 4.1 release, so that we can patch up the other dependency PRs faster, rather than only including this in a major release.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-common] TylerBreau commented on pull request #181: Updated PluginInfo to pass through XML attributes.
Posted by GitBox <gi...@apache.org>.
TylerBreau commented on PR #181:
URL: https://github.com/apache/cordova-common/pull/181#issuecomment-1284078001
Seems like it is not possible to support additional attributes on preferences.
describe('Preference', () => {
it('Test 007: Preference supports xml passthrough', function () {
const preferences = pluginPassthrough.getPreferences('android');
console.log(preferences);
expect(preferences.passthroughpref.anattrib).toBe('value');
});
});
<preference name="passthroughpref" anattrib="value" />
<!-- <preference name="passthroughpref2" anattrib="value2" /> -->
{ name: 'passthroughpref', anattrib: 'value', PASSTHROUGHPREF: null }
<preference name="passthroughpref" anattrib="value" />
<preference name="passthroughpref2" anattrib="value2" />
{
name: 'passthroughpref2',
anattrib: 'value2',
PASSTHROUGHPREF: null,
PASSTHROUGHPREF2: null
}
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-common] dpogue commented on pull request #181: Updated PluginInfo to pass through XML attributes.
Posted by GitBox <gi...@apache.org>.
dpogue commented on PR #181:
URL: https://github.com/apache/cordova-common/pull/181#issuecomment-1284379617
FWIW, I _think_ config.xml is parsed in each platform using its own cordova-common dependency (rather than the one provided by cordova-lib), specifically to work around the issues of versioning in cordova-cli and using older CLI versions with newer platform versions
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org