[GitHub] cordova-lib pull request: Implementation for Unified platform API

2015-06-22 Thread vladimir-kotikov
Github user vladimir-kotikov closed the pull request at:

https://github.com/apache/cordova-lib/pull/235


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org



[GitHub] cordova-lib pull request: Implementation for Unified platform API

2015-06-22 Thread vladimir-kotikov
Github user vladimir-kotikov commented on the pull request:

https://github.com/apache/cordova-lib/pull/235#issuecomment-114060806
  
Closing this since the PlatformApi needs for more detailed design. The 
discussion is moved to cordova/cordova-discuss#9


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org



[GitHub] cordova-lib pull request: Implementation for Unified platform API

2015-06-11 Thread omefire
Github user omefire commented on the pull request:

https://github.com/apache/cordova-lib/pull/235#issuecomment-58775
  
Since you've already ported the android implementation from cordova-lib to 
cordova-android, shouldn't you delete the android parser file from the metadata 
directory ? 

As long as it exists, BasePlatformApi.update_www (and potentially other 
functions) refers to it instead of the intended new code.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org



[GitHub] cordova-lib pull request: Implementation for Unified platform API

2015-06-11 Thread omefire
Github user omefire commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/235#discussion_r32236650
  
--- Diff: cordova-lib/src/platforms/PlatformApi.js ---
@@ -0,0 +1,414 @@
+/**
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements.  See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership.  The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+License); you may not use this file except in compliance
+with the License.  You may obtain a copy of the License at
+
+http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+AS IS BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied.  See the License for the
+specific language governing permissions and limitations
+under the License.
+*/
+
+var Q = require('q'),
+fs = require('fs'),
+path = require('path'),
+shell = require('shelljs'),
+events = require('../events'),
+util = require('../cordova/util'),
+superspawn = require('../cordova/superspawn'),
+platforms = require('./platformsConfig.json'),
+ConfigParser = require('../configparser/ConfigParser');
+
+var BasePluginHandler = require('../plugman/platforms/PluginHandler');
+var ParserHelper = 
require('../cordova/metadata/parserhelper/ParserHelper');
+
+// Avoid loading the same platform projects more than once (identified by 
path)
+var cachedProjects = {};
+
+/**
+ * A single class that exposes platform-dependent functionality for 
cordova and
+ * plugman. This class works as a base, and provides some default 
implementation
+ * for that functionality. It should not be instantiated directly, only 
through
+ * 'getPlatformApi' module method.
+ * @param  {String} platformName of platform. Should be one of 
names, declared
+ *  in platformsConfig.json
+ * @param  {String} platformRootDir Optional. Path to platform directory. 
If not provided
+ *  default value is set 
tocordova_project/platforms/platform
+ */
+function BasePlatformApi(platform, platformRootDir) {
+this.root = platformRootDir;
+this.platform = platform;
+
+// Parser property of Platform API left for backward compatibility
+// and smooth transition to ne API. It also does the job of requiring
+// and constructing legacy Parser instance, which required for 
platforms
+// that still stores their code in cordova-lib.
+var parser;
+Object.defineProperty(this, 'parser', {
+get: function () {
+if (parser)return parser;
+
+var ParserConstructor;
+try {
+ParserConstructor = 
require(platforms[this.platform].parser_file);
+} catch (e) { }
+
+// parser === null is the special case which means that we've 
tried
+// to get embedded platform parser and failed. In this case 
instead of
+// parser's methods will be called PlatformApi default 
implementations.
+parser = ParserConstructor ? new ParserConstructor(this.root) 
: null;
+return parser;
+},
+configurable: true
+});
+
+// Extend with a ParserHelper instance
+Object.defineProperty(this, 'helper', {
+value: new ParserHelper(this.platform),
+enumerable: true,
+configurable: false,
+writable: false
+});
+}
+
+/**
+ * Gets a plugin handler (former 'handler') for this project's platform.
+ * Platform can provide it's own implementation for PluginHandler by
+ * exposing PlatformApi.PluginHandler constructor. If platform doesn't
+ * provides its own implementation, then embedded PluginHandler will be 
used.
+ * (Taken from platformConfig.json/platform/handler_file field)
+ *
+ * @return {PluginHandler} Instance of PluginHandler class that exposes
+ *  platform-related functionality for 
cordova.
+ */
+BasePlatformApi.prototype.getPluginHandler = function() {
+if (!this._pluginHandler) {
+var PluginHandler = BasePluginHandler;
+var PluginHandlerImpl;
+
+// Try find whether platform exposes its' API via js module
+var platformApiModule = path.join(this.root, 'cordova', 'Api.js');
+if (fs.existsSync(platformApiModule)) {
+PluginHandlerImpl = 

[GitHub] cordova-lib pull request: Implementation for Unified platform API

2015-06-11 Thread omefire
Github user omefire commented on the pull request:

https://github.com/apache/cordova-lib/pull/235#issuecomment-84250
  
cool. (ref: android_parser  backward 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org



[GitHub] cordova-lib pull request: Implementation for Unified platform API

2015-06-11 Thread omefire
Github user omefire commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/235#discussion_r32236936
  
--- Diff: cordova-lib/src/platforms/PlatformApi.js ---
@@ -0,0 +1,414 @@
+/**
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements.  See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership.  The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+License); you may not use this file except in compliance
+with the License.  You may obtain a copy of the License at
+
+http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+AS IS BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied.  See the License for the
+specific language governing permissions and limitations
+under the License.
+*/
+
+var Q = require('q'),
+fs = require('fs'),
+path = require('path'),
+shell = require('shelljs'),
+events = require('../events'),
+util = require('../cordova/util'),
+superspawn = require('../cordova/superspawn'),
+platforms = require('./platformsConfig.json'),
+ConfigParser = require('../configparser/ConfigParser');
+
+var BasePluginHandler = require('../plugman/platforms/PluginHandler');
+var ParserHelper = 
require('../cordova/metadata/parserhelper/ParserHelper');
+
+// Avoid loading the same platform projects more than once (identified by 
path)
+var cachedProjects = {};
+
+/**
+ * A single class that exposes platform-dependent functionality for 
cordova and
+ * plugman. This class works as a base, and provides some default 
implementation
+ * for that functionality. It should not be instantiated directly, only 
through
+ * 'getPlatformApi' module method.
+ * @param  {String} platformName of platform. Should be one of 
names, declared
+ *  in platformsConfig.json
+ * @param  {String} platformRootDir Optional. Path to platform directory. 
If not provided
+ *  default value is set 
tocordova_project/platforms/platform
+ */
+function BasePlatformApi(platform, platformRootDir) {
+this.root = platformRootDir;
+this.platform = platform;
+
+// Parser property of Platform API left for backward compatibility
+// and smooth transition to ne API. It also does the job of requiring
+// and constructing legacy Parser instance, which required for 
platforms
+// that still stores their code in cordova-lib.
+var parser;
+Object.defineProperty(this, 'parser', {
+get: function () {
+if (parser)return parser;
+
+var ParserConstructor;
+try {
+ParserConstructor = 
require(platforms[this.platform].parser_file);
+} catch (e) { }
+
+// parser === null is the special case which means that we've 
tried
+// to get embedded platform parser and failed. In this case 
instead of
+// parser's methods will be called PlatformApi default 
implementations.
+parser = ParserConstructor ? new ParserConstructor(this.root) 
: null;
+return parser;
+},
+configurable: true
+});
+
+// Extend with a ParserHelper instance
+Object.defineProperty(this, 'helper', {
+value: new ParserHelper(this.platform),
+enumerable: true,
+configurable: false,
+writable: false
+});
+}
+
+/**
+ * Gets a plugin handler (former 'handler') for this project's platform.
+ * Platform can provide it's own implementation for PluginHandler by
+ * exposing PlatformApi.PluginHandler constructor. If platform doesn't
+ * provides its own implementation, then embedded PluginHandler will be 
used.
+ * (Taken from platformConfig.json/platform/handler_file field)
+ *
+ * @return {PluginHandler} Instance of PluginHandler class that exposes
+ *  platform-related functionality for 
cordova.
+ */
+BasePlatformApi.prototype.getPluginHandler = function() {
+if (!this._pluginHandler) {
+var PluginHandler = BasePluginHandler;
+var PluginHandlerImpl;
+
+// Try find whether platform exposes its' API via js module
+var platformApiModule = path.join(this.root, 'cordova', 'Api.js');
+if (fs.existsSync(platformApiModule)) {
+PluginHandlerImpl = 

[GitHub] cordova-lib pull request: Implementation for Unified platform API

2015-06-11 Thread omefire
Github user omefire commented on the pull request:

https://github.com/apache/cordova-lib/pull/235#issuecomment-111213610
  
Ah, I see. (ref: android_parser  backward compatibility).

However, in case a platform is implemented via the new way, shouldn't we be 
actually using that ? currently, we're still using the old code, which means 
the new one never runs.

On top of that, if someone wanted to fix or debug an issue, he'd do it on 
the new code instead of the old one, which is confusing.

I suggest, we always prefer the new API when available, and only fall back 
to old one when the new API isn't available.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org



[GitHub] cordova-lib pull request: Implementation for Unified platform API

2015-06-11 Thread omefire
Github user omefire commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/235#discussion_r32246260
  
--- Diff: cordova-lib/src/platforms/PlatformApi.js ---
@@ -0,0 +1,414 @@
+/**
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements.  See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership.  The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+License); you may not use this file except in compliance
+with the License.  You may obtain a copy of the License at
+
+http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+AS IS BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied.  See the License for the
+specific language governing permissions and limitations
+under the License.
+*/
+
+var Q = require('q'),
+fs = require('fs'),
+path = require('path'),
+shell = require('shelljs'),
+events = require('../events'),
+util = require('../cordova/util'),
+superspawn = require('../cordova/superspawn'),
+platforms = require('./platformsConfig.json'),
+ConfigParser = require('../configparser/ConfigParser');
+
+var BasePluginHandler = require('../plugman/platforms/PluginHandler');
+var ParserHelper = 
require('../cordova/metadata/parserhelper/ParserHelper');
+
+// Avoid loading the same platform projects more than once (identified by 
path)
+var cachedProjects = {};
+
+/**
+ * A single class that exposes platform-dependent functionality for 
cordova and
+ * plugman. This class works as a base, and provides some default 
implementation
+ * for that functionality. It should not be instantiated directly, only 
through
+ * 'getPlatformApi' module method.
+ * @param  {String} platformName of platform. Should be one of 
names, declared
+ *  in platformsConfig.json
+ * @param  {String} platformRootDir Optional. Path to platform directory. 
If not provided
+ *  default value is set 
tocordova_project/platforms/platform
+ */
+function BasePlatformApi(platform, platformRootDir) {
+this.root = platformRootDir;
+this.platform = platform;
+
+// Parser property of Platform API left for backward compatibility
+// and smooth transition to ne API. It also does the job of requiring
+// and constructing legacy Parser instance, which required for 
platforms
+// that still stores their code in cordova-lib.
+var parser;
+Object.defineProperty(this, 'parser', {
+get: function () {
+if (parser)return parser;
+
+var ParserConstructor;
+try {
+ParserConstructor = 
require(platforms[this.platform].parser_file);
+} catch (e) { }
+
+// parser === null is the special case which means that we've 
tried
+// to get embedded platform parser and failed. In this case 
instead of
+// parser's methods will be called PlatformApi default 
implementations.
--- End diff --

this sentence is a little confusing. 
Maybe reword it to 'In this case, the default behavior as implemented by 
PlatformApi will be used' ?


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org



[GitHub] cordova-lib pull request: Implementation for Unified platform API

2015-06-11 Thread vladimir-kotikov
Github user vladimir-kotikov commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/235#discussion_r32198647
  
--- Diff: cordova-lib/src/platforms/PlatformApi.js ---
@@ -0,0 +1,365 @@
+/**
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements.  See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership.  The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+License); you may not use this file except in compliance
+with the License.  You may obtain a copy of the License at
+
+http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+AS IS BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied.  See the License for the
+specific language governing permissions and limitations
+under the License.
+*/
+
+var Q = require('q'),
+fs = require('fs'),
+path = require('path'),
+shell = require('shelljs'),
+events = require('../events'),
+util = require('../cordova/util'),
+superspawn = require('../cordova/superspawn'),
+platforms = require('./platformsConfig.json'),
+ConfigParser = require('../configparser/ConfigParser');
+
+var BasePluginHandler = require('../plugman/platforms/PluginHandler');
+var ParserHelper = 
require('../cordova/metadata/parserhelper/ParserHelper');
+
+// Avoid loading the same platform projects more than once (identified by 
path)
+var cachedProjects = {};
+
+// A single class that exposes functionality from platform specific files 
from
--- End diff --

Updated in 
https://github.com/MSOpenTech/cordova-lib/commit/c78eae57929ed541ac2c2a47a164938433133309


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org



[GitHub] cordova-lib pull request: Implementation for Unified platform API

2015-06-11 Thread vladimir-kotikov
Github user vladimir-kotikov commented on the pull request:

https://github.com/apache/cordova-lib/pull/235#issuecomment-70935
  
`android_parser` is left for backward compatibility, since user can install 
older version of cordova-android, which doesn't provides own `PlatformAPI`. In 
this case the code from `android_parser` will be used.

If you're instantiating `PlatformApi` properly - via `getPlatformApi` 
getter, and not directly (`new BasePlatformApi()`) - the `updateWww` will 
_always_ be owerwritten by platform-provided method. (of course if platform 
provides it :smiley:  )


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org



[GitHub] cordova-lib pull request: Implementation for Unified platform API

2015-06-10 Thread omefire
Github user omefire commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/235#discussion_r32148857
  
--- Diff: cordova-lib/src/platforms/PlatformApi.js ---
@@ -0,0 +1,365 @@
+/**
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements.  See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership.  The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+License); you may not use this file except in compliance
+with the License.  You may obtain a copy of the License at
+
+http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+AS IS BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied.  See the License for the
+specific language governing permissions and limitations
+under the License.
+*/
+
+var Q = require('q'),
+fs = require('fs'),
+path = require('path'),
+shell = require('shelljs'),
+events = require('../events'),
+util = require('../cordova/util'),
+superspawn = require('../cordova/superspawn'),
+platforms = require('./platformsConfig.json'),
+ConfigParser = require('../configparser/ConfigParser');
+
+var BasePluginHandler = require('../plugman/platforms/PluginHandler');
+var ParserHelper = 
require('../cordova/metadata/parserhelper/ParserHelper');
+
+// Avoid loading the same platform projects more than once (identified by 
path)
+var cachedProjects = {};
+
+// A single class that exposes functionality from platform specific files 
from
+// both places cordova/metadata and plugman/platforms. Hopefully, to be 
soon
+// replaced by real unified platform specific classes.
+function BasePlatformApi(platform, platformRootDir) {
+this.root = platformRootDir;
+this.platform = platform;
+
+// Parser property of Platform API left for backward compatibility
+// and smooth transition to ne API. It also does the job of requiring
+// and constructing legacy Parser instance, which required for 
platforms
+// that still stores their code in cordova-lib.
+var parser;
+Object.defineProperty(this, 'parser', {
+get: function () {
+if (parser)return parser;
+
+var ParserConstructor;
+try {
+ParserConstructor = 
require(platforms[this.platform].parser_file);
+} catch (e) { }
+
+// parser === null is the special case which means that we've 
tried
+// to get embedded platform parser and failed. In this case 
instead of
+// parser's methods will be called PlatformApi default 
implementations.
+parser = ParserConstructor ? new ParserConstructor(this.root) 
: null;
+return parser;
+},
+configurable: true
+});
+
+// Extend with a ParserHelper instance
+Object.defineProperty(this, 'helper', {
+value: new ParserHelper(this.platform),
+enumerable: true,
+configurable: false,
+writable: false
+});
+}
+
+/**
+ * Gets a plugin handler (former 'handler') for this project's platform.
+ * Platform can provide it's own implementation for PluginHandler by
+ * exposing PlatformApi.PluginHandler constructor. If platform doesn't
+ * provides its own implementation, then embedded PluginHandler will be 
used.
+ * (Taken from platformConfig.json/platform/handler_file field)
+ *
+ * @return {PluginHandler} Instance of PluginHandler class that exposes
+ *  platform-related functionality for 
cordova.
+ */
+BasePlatformApi.prototype.getPluginHandler = function() {
+if (!this._pluginHandler) {
+var PluginHandler = BasePluginHandler;
+var PluginHandlerImpl;
+
+// Try find whether platform exposes its' API via js module
+var platformApiModule = path.join(this.root, 'cordova', 'Api.js');
+if (fs.existsSync(platformApiModule)) {
+PluginHandlerImpl = require(platformApiModule).PluginHandler;
+}
+
+if (!PluginHandlerImpl) {
+// If platform doesn't provide PluginHandler, use embedded one 
for current platform
+// The platform implementation, shipped with cordova-lib, 
isn't constructable so
+// we need to create a dummy class and copy implementation to 
its prototype.
+var LegacyPluginHandler = function 

[GitHub] cordova-lib pull request: Implementation for Unified platform API

2015-06-10 Thread omefire
Github user omefire commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/235#discussion_r32169661
  
--- Diff: cordova-lib/src/platforms/PlatformApi.js ---
@@ -0,0 +1,365 @@
+/**
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements.  See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership.  The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+License); you may not use this file except in compliance
+with the License.  You may obtain a copy of the License at
+
+http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+AS IS BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied.  See the License for the
+specific language governing permissions and limitations
+under the License.
+*/
+
+var Q = require('q'),
+fs = require('fs'),
+path = require('path'),
+shell = require('shelljs'),
+events = require('../events'),
+util = require('../cordova/util'),
+superspawn = require('../cordova/superspawn'),
+platforms = require('./platformsConfig.json'),
+ConfigParser = require('../configparser/ConfigParser');
+
+var BasePluginHandler = require('../plugman/platforms/PluginHandler');
+var ParserHelper = 
require('../cordova/metadata/parserhelper/ParserHelper');
+
+// Avoid loading the same platform projects more than once (identified by 
path)
+var cachedProjects = {};
+
+// A single class that exposes functionality from platform specific files 
from
--- End diff --

does this comment still apply ? should we remove 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org



[GitHub] cordova-lib pull request: Implementation for Unified platform API

2015-06-10 Thread nikhilkh
Github user nikhilkh commented on the pull request:

https://github.com/apache/cordova-lib/pull/235#issuecomment-110955660
  
It will help to generate API docs for the public API surface now. Reviewing 
the implementation guts and the API in code is not easy. In any case, the 
platform API might need to be documented for anyone looking to create a 
platform. Might as well do it now.

For that, using jsdoc or other tools to generate docs for the API from code 
would be super useful.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org



[GitHub] cordova-lib pull request: Implementation for Unified platform API

2015-06-10 Thread nikhilkh
Github user nikhilkh commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/235#discussion_r32181585
  
--- Diff: cordova-lib/src/platforms/PlatformApi.js ---
@@ -0,0 +1,365 @@
+/**
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements.  See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership.  The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+License); you may not use this file except in compliance
+with the License.  You may obtain a copy of the License at
+
+http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+AS IS BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied.  See the License for the
+specific language governing permissions and limitations
+under the License.
+*/
+
+var Q = require('q'),
+fs = require('fs'),
+path = require('path'),
+shell = require('shelljs'),
+events = require('../events'),
+util = require('../cordova/util'),
+superspawn = require('../cordova/superspawn'),
+platforms = require('./platformsConfig.json'),
+ConfigParser = require('../configparser/ConfigParser');
+
+var BasePluginHandler = require('../plugman/platforms/PluginHandler');
+var ParserHelper = 
require('../cordova/metadata/parserhelper/ParserHelper');
+
+// Avoid loading the same platform projects more than once (identified by 
path)
+var cachedProjects = {};
+
+// A single class that exposes functionality from platform specific files 
from
+// both places cordova/metadata and plugman/platforms. Hopefully, to be 
soon
+// replaced by real unified platform specific classes.
+function BasePlatformApi(platform, platformRootDir) {
+this.root = platformRootDir;
+this.platform = platform;
+
+// Parser property of Platform API left for backward compatibility
+// and smooth transition to ne API. It also does the job of requiring
+// and constructing legacy Parser instance, which required for 
platforms
+// that still stores their code in cordova-lib.
+var parser;
+Object.defineProperty(this, 'parser', {
+get: function () {
+if (parser)return parser;
+
+var ParserConstructor;
+try {
+ParserConstructor = 
require(platforms[this.platform].parser_file);
+} catch (e) { }
+
+// parser === null is the special case which means that we've 
tried
+// to get embedded platform parser and failed. In this case 
instead of
+// parser's methods will be called PlatformApi default 
implementations.
+parser = ParserConstructor ? new ParserConstructor(this.root) 
: null;
+return parser;
+},
+configurable: true
+});
+
+// Extend with a ParserHelper instance
+Object.defineProperty(this, 'helper', {
+value: new ParserHelper(this.platform),
+enumerable: true,
+configurable: false,
+writable: false
+});
+}
+
+/**
+ * Gets a plugin handler (former 'handler') for this project's platform.
+ * Platform can provide it's own implementation for PluginHandler by
+ * exposing PlatformApi.PluginHandler constructor. If platform doesn't
+ * provides its own implementation, then embedded PluginHandler will be 
used.
+ * (Taken from platformConfig.json/platform/handler_file field)
+ *
+ * @return {PluginHandler} Instance of PluginHandler class that exposes
+ *  platform-related functionality for 
cordova.
+ */
+BasePlatformApi.prototype.getPluginHandler = function() {
+if (!this._pluginHandler) {
+var PluginHandler = BasePluginHandler;
+var PluginHandlerImpl;
+
+// Try find whether platform exposes its' API via js module
+var platformApiModule = path.join(this.root, 'cordova', 'Api.js');
+if (fs.existsSync(platformApiModule)) {
+PluginHandlerImpl = require(platformApiModule).PluginHandler;
+}
+
+if (!PluginHandlerImpl) {
+// If platform doesn't provide PluginHandler, use embedded one 
for current platform
+// The platform implementation, shipped with cordova-lib, 
isn't constructable so
+// we need to create a dummy class and copy implementation to 
its prototype.
+var LegacyPluginHandler = function 

[GitHub] cordova-lib pull request: Implementation for Unified platform API

2015-06-10 Thread nikhilkh
Github user nikhilkh commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/235#discussion_r32181523
  
--- Diff: cordova-lib/src/platforms/PlatformApi.js ---
@@ -0,0 +1,365 @@
+/**
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements.  See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership.  The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+License); you may not use this file except in compliance
+with the License.  You may obtain a copy of the License at
+
+http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+AS IS BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied.  See the License for the
+specific language governing permissions and limitations
+under the License.
+*/
+
+var Q = require('q'),
+fs = require('fs'),
+path = require('path'),
+shell = require('shelljs'),
+events = require('../events'),
+util = require('../cordova/util'),
+superspawn = require('../cordova/superspawn'),
+platforms = require('./platformsConfig.json'),
+ConfigParser = require('../configparser/ConfigParser');
+
+var BasePluginHandler = require('../plugman/platforms/PluginHandler');
+var ParserHelper = 
require('../cordova/metadata/parserhelper/ParserHelper');
+
+// Avoid loading the same platform projects more than once (identified by 
path)
+var cachedProjects = {};
+
+// A single class that exposes functionality from platform specific files 
from
+// both places cordova/metadata and plugman/platforms. Hopefully, to be 
soon
+// replaced by real unified platform specific classes.
+function BasePlatformApi(platform, platformRootDir) {
+this.root = platformRootDir;
+this.platform = platform;
+
+// Parser property of Platform API left for backward compatibility
+// and smooth transition to ne API. It also does the job of requiring
+// and constructing legacy Parser instance, which required for 
platforms
+// that still stores their code in cordova-lib.
+var parser;
+Object.defineProperty(this, 'parser', {
+get: function () {
+if (parser)return parser;
+
+var ParserConstructor;
+try {
+ParserConstructor = 
require(platforms[this.platform].parser_file);
+} catch (e) { }
+
+// parser === null is the special case which means that we've 
tried
+// to get embedded platform parser and failed. In this case 
instead of
+// parser's methods will be called PlatformApi default 
implementations.
+parser = ParserConstructor ? new ParserConstructor(this.root) 
: null;
+return parser;
+},
+configurable: true
+});
+
+// Extend with a ParserHelper instance
+Object.defineProperty(this, 'helper', {
+value: new ParserHelper(this.platform),
+enumerable: true,
+configurable: false,
+writable: false
+});
+}
+
+/**
+ * Gets a plugin handler (former 'handler') for this project's platform.
+ * Platform can provide it's own implementation for PluginHandler by
+ * exposing PlatformApi.PluginHandler constructor. If platform doesn't
+ * provides its own implementation, then embedded PluginHandler will be 
used.
+ * (Taken from platformConfig.json/platform/handler_file field)
+ *
+ * @return {PluginHandler} Instance of PluginHandler class that exposes
+ *  platform-related functionality for 
cordova.
+ */
+BasePlatformApi.prototype.getPluginHandler = function() {
+if (!this._pluginHandler) {
+var PluginHandler = BasePluginHandler;
+var PluginHandlerImpl;
+
+// Try find whether platform exposes its' API via js module
+var platformApiModule = path.join(this.root, 'cordova', 'Api.js');
+if (fs.existsSync(platformApiModule)) {
+PluginHandlerImpl = require(platformApiModule).PluginHandler;
+}
+
+if (!PluginHandlerImpl) {
+// If platform doesn't provide PluginHandler, use embedded one 
for current platform
+// The platform implementation, shipped with cordova-lib, 
isn't constructable so
+// we need to create a dummy class and copy implementation to 
its prototype.
+var LegacyPluginHandler = function 

[GitHub] cordova-lib pull request: Implementation for Unified platform API

2015-06-08 Thread kamrik
Github user kamrik commented on the pull request:

https://github.com/apache/cordova-lib/pull/235#issuecomment-110155367
  
I don't have strong opinion here, but a bit more inclined towards 
dependency injection. For some modules like ConfigParser having different 
versions imported by different platforms can get very frustrating. Even when 
cordova is used without thc CLI, large parts of cordova-lib would still be 
needed, whether required()'d directly by the user or implicitly via 
cordova-android. So as an extreme option we can dependency-inject the entire 
cordova-lib (properly exposing the relevant classes). Somewhat more elegantly, 
we could construct a module inside cordova-lib with just the needed classes and 
inject it when instantiating cordova-android API. It might be useful to later 
split out this module as a separate npm package (or several packages), but for 
time being they could still stay part of the lib.

Superspawn is definitely a candidate for being a totally separate module. 
But modules like PluginInfo and ConfigParser are very unlikely to be used 
separately from each other. Maybe in the long run they should live in some new 
package named like cordova-core, or we could move the CLI specific stuff out 
of cordova-lib and into cordova-cli, then the lib would become that core.

That said, I'm fine with the approach you suggested.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org



[GitHub] cordova-lib pull request: Implementation for Unified platform API

2015-06-08 Thread vladimir-kotikov
Github user vladimir-kotikov commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/235#discussion_r31924092
  
--- Diff: cordova-lib/src/platforms/PlatformApi.js ---
@@ -0,0 +1,365 @@
+/**
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements.  See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership.  The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+License); you may not use this file except in compliance
+with the License.  You may obtain a copy of the License at
+
+http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+AS IS BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied.  See the License for the
+specific language governing permissions and limitations
+under the License.
+*/
+
+var Q = require('q'),
+fs = require('fs'),
+path = require('path'),
+shell = require('shelljs'),
+events = require('../events'),
+util = require('../cordova/util'),
+superspawn = require('../cordova/superspawn'),
+platforms = require('./platformsConfig.json'),
+ConfigParser = require('../configparser/ConfigParser');
+
+var BasePluginHandler = require('../plugman/platforms/PluginHandler');
+var ParserHelper = 
require('../cordova/metadata/parserhelper/ParserHelper');
+
+// Avoid loading the same platform projects more than once (identified by 
path)
+var cachedProjects = {};
+
+// A single class that exposes functionality from platform specific files 
from
+// both places cordova/metadata and plugman/platforms. Hopefully, to be 
soon
+// replaced by real unified platform specific classes.
+function BasePlatformApi(platform, platformRootDir) {
+this.root = platformRootDir;
+this.platform = platform;
+
+// Parser property of Platform API left for backward compatibility
+// and smooth transition to ne API. It also does the job of requiring
+// and constructing legacy Parser instance, which required for 
platforms
+// that still stores their code in cordova-lib.
+var parser;
+Object.defineProperty(this, 'parser', {
+get: function () {
+if (parser)return parser;
+
+var ParserConstructor;
+try {
+ParserConstructor = 
require(platforms[this.platform].parser_file);
+} catch (e) { }
+
+// parser === null is the special case which means that we've 
tried
+// to get embedded platform parser and failed. In this case 
instead of
+// parser's methods will be called PlatformApi default 
implementations.
+parser = ParserConstructor ? new ParserConstructor(this.root) 
: null;
+return parser;
+},
+configurable: true
+});
+
+// Extend with a ParserHelper instance
+Object.defineProperty(this, 'helper', {
+value: new ParserHelper(this.platform),
+enumerable: true,
+configurable: false,
+writable: false
+});
+}
+
+/**
+ * Gets a plugin handler (former 'handler') for this project's platform.
+ * Platform can provide it's own implementation for PluginHandler by
+ * exposing PlatformApi.PluginHandler constructor. If platform doesn't
+ * provides its own implementation, then embedded PluginHandler will be 
used.
+ * (Taken from platformConfig.json/platform/handler_file field)
+ *
+ * @return {PluginHandler} Instance of PluginHandler class that exposes
+ *  platform-related functionality for 
cordova.
+ */
+BasePlatformApi.prototype.getPluginHandler = function() {
+if (!this._pluginHandler) {
+var PluginHandler = BasePluginHandler;
+var PluginHandlerImpl;
+
+// Try find whether platform exposes its' API via js module
+var platformApiModule = path.join(this.root, 'cordova', 'Api.js');
+if (fs.existsSync(platformApiModule)) {
+PluginHandlerImpl = require(platformApiModule).PluginHandler;
+}
+
+if (!PluginHandlerImpl) {
+// If platform doesn't provide PluginHandler, use embedded one 
for current platform
+// The platform implementation, shipped with cordova-lib, 
isn't constructable so
+// we need to create a dummy class and copy implementation to 
its prototype.
+var LegacyPluginHandler = function 

[GitHub] cordova-lib pull request: Implementation for Unified platform API

2015-06-08 Thread vladimir-kotikov
Github user vladimir-kotikov commented on the pull request:

https://github.com/apache/cordova-lib/pull/235#issuecomment-110038030
  
@kamrik, thanks for feedback.

Dependency injection of cordova logic to platform was our very first idea, 
but in addition to limitations, you mentioned, it seems that it also will be 
impossible to use PlatformAPI without CLI in this case.

Another approach, we've been thinking about - factoring out common classes 
and utilities, like ConfigHelper, superspawn, Requirements-related stuff, etc., 
to separate module, which then can be used by platforms and cordova-lib. This 
approach is quite similar to @TimBarham's PR for cordova-serve: 
https://github.com/apache/cordova-lib/pull/238

What do you think about 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org



[GitHub] cordova-lib pull request: Implementation for Unified platform API

2015-06-05 Thread kamrik
Github user kamrik commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/235#discussion_r31840378
  
--- Diff: cordova-lib/src/platforms/PlatformApi.js ---
@@ -0,0 +1,365 @@
+/**
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements.  See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership.  The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+License); you may not use this file except in compliance
+with the License.  You may obtain a copy of the License at
+
+http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+AS IS BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied.  See the License for the
+specific language governing permissions and limitations
+under the License.
+*/
+
+var Q = require('q'),
+fs = require('fs'),
+path = require('path'),
+shell = require('shelljs'),
+events = require('../events'),
+util = require('../cordova/util'),
+superspawn = require('../cordova/superspawn'),
+platforms = require('./platformsConfig.json'),
+ConfigParser = require('../configparser/ConfigParser');
+
+var BasePluginHandler = require('../plugman/platforms/PluginHandler');
+var ParserHelper = 
require('../cordova/metadata/parserhelper/ParserHelper');
+
+// Avoid loading the same platform projects more than once (identified by 
path)
+var cachedProjects = {};
+
+// A single class that exposes functionality from platform specific files 
from
+// both places cordova/metadata and plugman/platforms. Hopefully, to be 
soon
+// replaced by real unified platform specific classes.
+function BasePlatformApi(platform, platformRootDir) {
+this.root = platformRootDir;
+this.platform = platform;
+
+// Parser property of Platform API left for backward compatibility
+// and smooth transition to ne API. It also does the job of requiring
+// and constructing legacy Parser instance, which required for 
platforms
+// that still stores their code in cordova-lib.
+var parser;
+Object.defineProperty(this, 'parser', {
+get: function () {
+if (parser)return parser;
+
+var ParserConstructor;
+try {
+ParserConstructor = 
require(platforms[this.platform].parser_file);
+} catch (e) { }
+
+// parser === null is the special case which means that we've 
tried
+// to get embedded platform parser and failed. In this case 
instead of
+// parser's methods will be called PlatformApi default 
implementations.
+parser = ParserConstructor ? new ParserConstructor(this.root) 
: null;
+return parser;
+},
+configurable: true
+});
+
+// Extend with a ParserHelper instance
+Object.defineProperty(this, 'helper', {
+value: new ParserHelper(this.platform),
+enumerable: true,
+configurable: false,
+writable: false
+});
+}
+
+/**
+ * Gets a plugin handler (former 'handler') for this project's platform.
+ * Platform can provide it's own implementation for PluginHandler by
+ * exposing PlatformApi.PluginHandler constructor. If platform doesn't
+ * provides its own implementation, then embedded PluginHandler will be 
used.
+ * (Taken from platformConfig.json/platform/handler_file field)
+ *
+ * @return {PluginHandler} Instance of PluginHandler class that exposes
+ *  platform-related functionality for 
cordova.
+ */
+BasePlatformApi.prototype.getPluginHandler = function() {
+if (!this._pluginHandler) {
+var PluginHandler = BasePluginHandler;
+var PluginHandlerImpl;
+
+// Try find whether platform exposes its' API via js module
+var platformApiModule = path.join(this.root, 'cordova', 'Api.js');
+if (fs.existsSync(platformApiModule)) {
+PluginHandlerImpl = require(platformApiModule).PluginHandler;
+}
+
+if (!PluginHandlerImpl) {
+// If platform doesn't provide PluginHandler, use embedded one 
for current platform
+// The platform implementation, shipped with cordova-lib, 
isn't constructable so
+// we need to create a dummy class and copy implementation to 
its prototype.
+var LegacyPluginHandler = function 

[GitHub] cordova-lib pull request: Implementation for Unified platform API

2015-06-05 Thread kamrik
Github user kamrik commented on the pull request:

https://github.com/apache/cordova-lib/pull/235#issuecomment-109401910
  
Looks great! The need to duplicate common code like ConfgiParser in 
cordova-android is a bit annoying and is bound to generate some versioning 
confusion. But probably it should be dealt with later.

One option could be to pass modules like ConfigParser to 
coraodva-android/Api.js from code that requires Api.js (usually cordova-lib). 
It's weird and unintuitive, but would solve the code duplication project.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org



[GitHub] cordova-lib pull request: Implementation for Unified platform API

2015-06-04 Thread vladimir-kotikov
GitHub user vladimir-kotikov opened a pull request:

https://github.com/apache/cordova-lib/pull/235

Implementation for Unified platform API 

An mplementation of Unified Platform API for cordova, that inspired by 
[this thread](http://markmail.org/thread/3dw4mis4qo5d4ecz) in cordova-dev 
mailing list and [this 
proposal](https://github.com/kamrik/cordova-discuss/blob/master/proposals/PlatformProject.md)

This PR is the updated version of 
https://github.com/apache/cordova-lib/pull/226

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/MSOpenTech/cordova-lib unified_platform_api

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/cordova-lib/pull/235.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 #235


commit 5a916c978c84b36f92e39f17dac8d48db7ef927d
Author: Vladimir Kotikov v-vlk...@microsoft.com
Date:   2015-06-04T09:57:18Z

Adds BasePlatformApi




---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org