You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cordova.apache.org by vladimir-kotikov <gi...@git.apache.org> on 2016/02/11 15:59:57 UTC
[GitHub] cordova-lib pull request: CB-10592 Don't quote platform specific a...
GitHub user vladimir-kotikov opened a pull request:
https://github.com/apache/cordova-lib/pull/386
CB-10592 Don't quote platform specific args values
See JIRA [CB-10592](https://issues.apache.org/jira/browse/CB-10592)
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/MSOpenTech/cordova-lib CB-10592
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cordova-lib/pull/386.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 #386
----
commit 7e3100492adfc87af6175117e9765cccc65e27a3
Author: Vladimir Kotikov <v-...@microsoft.com>
Date: 2016-02-11T12:28:03Z
CB-10592 Don't quote platform specific args values
commit 87b8bedc29e867d7da2745ff36a3107c7b15bc80
Author: Vladimir Kotikov <v-...@microsoft.com>
Date: 2016-02-11T14:03:45Z
CB-10592 Add tests to cover the argiments compatibility logic
----
---
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: CB-10592 Don't quote platform specific a...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/cordova-lib/pull/386
---
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: CB-10592 Don't quote platform specific a...
Posted by dblotsky <gi...@git.apache.org>.
Github user dblotsky commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/386#discussion_r52726089
--- Diff: cordova-lib/src/cordova/util.js ---
@@ -297,7 +297,7 @@ function ensurePlatformOptionsCompatible (platformOptions) {
}).map(function (arg) {
return opts[arg] === true ?
'--' + arg :
- '--' + arg + '="' + opts[arg].toString() + '"';
+ '--' + arg + '=' + opts[arg].toString();
--- End diff --
Why are we passing string CLI arguments in a programmatic API? If this is because we're using `cordova.raw`, then can we refactor to not use 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-lib pull request: CB-10592 Don't quote platform specific a...
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-lib/pull/386#discussion_r52712187
--- Diff: cordova-lib/src/cordova/util.js ---
@@ -297,7 +297,7 @@ function ensurePlatformOptionsCompatible (platformOptions) {
}).map(function (arg) {
return opts[arg] === true ?
'--' + arg :
- '--' + arg + '="' + opts[arg].toString() + '"';
+ '--' + arg + '=' + opts[arg].toString();
--- End diff --
Because these quotes distorts arguments values e.g. `['--appx=uap']` becomes `['--appx="uap"']` and then (after parsing by `nopt` in platform) - `{appx: "\"uap"\"}` - notice that quotes now included as a part of `appx` option value.
---
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: CB-10592 Don't quote platform specific a...
Posted by dblotsky <gi...@git.apache.org>.
Github user dblotsky commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/386#discussion_r52794591
--- Diff: cordova-lib/src/cordova/util.js ---
@@ -297,7 +297,7 @@ function ensurePlatformOptionsCompatible (platformOptions) {
}).map(function (arg) {
return opts[arg] === true ?
'--' + arg :
- '--' + arg + '="' + opts[arg].toString() + '"';
+ '--' + arg + '=' + opts[arg].toString();
--- End diff --
Alright, fair enough. Do you think we can deprecate this usage in the future though? It's a really awkward (and also insecure) API.
---
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: CB-10592 Don't quote platform specific a...
Posted by dblotsky <gi...@git.apache.org>.
Github user dblotsky commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/386#discussion_r52677192
--- Diff: cordova-lib/src/cordova/util.js ---
@@ -297,7 +297,7 @@ function ensurePlatformOptionsCompatible (platformOptions) {
}).map(function (arg) {
return opts[arg] === true ?
'--' + arg :
- '--' + arg + '="' + opts[arg].toString() + '"';
+ '--' + arg + '=' + opts[arg].toString();
--- End diff --
Why is this no longer escaped?
---
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: CB-10592 Don't quote platform specific a...
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-lib/pull/386#discussion_r52796519
--- Diff: cordova-lib/src/cordova/util.js ---
@@ -297,7 +297,7 @@ function ensurePlatformOptionsCompatible (platformOptions) {
}).map(function (arg) {
return opts[arg] === true ?
'--' + arg :
- '--' + arg + '="' + opts[arg].toString() + '"';
+ '--' + arg + '=' + opts[arg].toString();
--- End diff --
I hope so. We're actually already emitting a warning when arguments used this way, so we could try to get rid of this in next major 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-lib pull request: CB-10592 Don't quote platform specific a...
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-lib/pull/386#discussion_r52731347
--- Diff: cordova-lib/src/cordova/util.js ---
@@ -297,7 +297,7 @@ function ensurePlatformOptionsCompatible (platformOptions) {
}).map(function (arg) {
return opts[arg] === true ?
'--' + arg :
- '--' + arg + '="' + opts[arg].toString() + '"';
+ '--' + arg + '=' + opts[arg].toString();
--- End diff --
If you're talking about this usage
```javascript
cordova.raw.build({
platforms: ["android"],
options: ["--debug", "--gradleArg=--no-daemon"]
})
```
i think we shouldn't do that but still need to maintain this scenario for compatibility.
---
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