From 01efe2ec942780edc33463360dbd7c10e44b5492 Mon Sep 17 00:00:00 2001 From: Patrik Meijer Date: Thu, 9 Nov 2017 11:23:49 -0600 Subject: [PATCH 1/4] WIP Document plugin cbs better and fix links in Core --- src/common/core/core.js | 37 ++++++++++++++++++++----------------- src/plugin/PluginBase.js | 31 +++++++++++++++++++++---------- 2 files changed, 41 insertions(+), 27 deletions(-) diff --git a/src/common/core/core.js b/src/common/core/core.js index 092a4e348..bebe9e8e4 100644 --- a/src/common/core/core.js +++ b/src/common/core/core.js @@ -464,7 +464,7 @@ define([ * @param {Error|CoreIllegalArgumentError|CoreAssertError|null} callback.error - the result of the execution * @param {module:Core~Node} callback.node - the resulting root node * - * @return {External~Promise} If no callback is given, the result will be provided in + * @return {external:Promise} If no callback is given, the result will be provided in * a promiselike manner. * * @throws {CoreIllegalArgumentError} If some of the parameters don't match the input criteria. @@ -493,7 +493,7 @@ define([ * @param {Error|CoreIllegalArgumentError|CoreAssertError|null} callback.error - the result of the execution * @param {module:Core~Node} callback.node - the resulting child * - * @return {External~Promise} If no callback is given, the result will be provided in a promise. + * @return {external:Promise} If no callback is given, the result will be provided in a promise. * * @throws {CoreIllegalArgumentError} If some of the parameters don't match the input criteria. */ @@ -520,7 +520,7 @@ define([ * @param {Error|CoreIllegalArgumentError|CoreAssertError|null} callback.error - the result of the execution * @param {module:Core~Node} callback.node - the resulting node * - * @return {External~Promise} If no callback is given, the result will be provided in a promise. + * @return {external:Promise} If no callback is given, the result will be provided in a promise. * * @throws {CoreIllegalArgumentError} If some of the parameters don't match the input criteria. */ @@ -546,7 +546,7 @@ define([ * @param {Error|CoreIllegalArgumentError|CoreAssertError|null} callback.error - the result of the execution * @param {module:Core~Node[]} callback.children - the resulting children * - * @return {External~Promise} If no callback is given, the result will be provided in a promise. + * @return {external:Promise} If no callback is given, the result will be provided in a promise. * * @throws {CoreIllegalArgumentError} If some of the parameters don't match the input criteria. */ @@ -572,7 +572,7 @@ define([ * @param {Error|CoreIllegalArgumentError|CoreAssertError|null} callback.error - the result of the execution * @param {module:Core~Node[]} callback.node - the resulting children * - * @return {External~Promise} If no callback is given, the result will be provided in a promise. + * @return {external:Promise} If no callback is given, the result will be provided in a promise. * * @throws {CoreIllegalArgumentError} If some of the parameters don't match the input criteria. */ @@ -600,7 +600,7 @@ define([ * @param {Error|CoreIllegalArgumentError|CoreAssertError|null} callback.error - the result of the execution * @param {module:Core~Node} callback.node - the resulting target * - * @return {External~Promise} If no callback is given, the result will be provided in a promise. + * @return {external:Promise} If no callback is given, the result will be provided in a promise. * * @throws {CoreIllegalArgumentError} If some of the parameters don't match the input criteria. */ @@ -627,7 +627,7 @@ define([ * @param {Error|CoreIllegalArgumentError|CoreAssertError|null} callback.error - the result of the execution * @param {module:Core~Node[]} callback.node - the resulting sources * - * @return {External~Promise} If no callback is given, the result will be provided in a promise. + * @return {external:Promise} If no callback is given, the result will be provided in a promise. * * @throws {CoreIllegalArgumentError} If some of the parameters don't match the input criteria. */ @@ -652,7 +652,7 @@ define([ * @param {Error|CoreIllegalArgumentError|CoreAssertError|null} callback.error - the result of the execution * @param {module:Core~Node[]} callback.node - the resulting sources * - * @return {External~Promise} If no callback is given, the result will be provided in a promise. + * @return {external:Promise} If no callback is given, the result will be provided in a promise. * * @throws {CoreIllegalArgumentError} If some of the parameters don't match the input criteria. */ @@ -677,7 +677,7 @@ define([ * @param {Error|CoreIllegalArgumentError|CoreAssertError|null} callback.error - the result of the execution * @param {module:Core~Node[]} callback.node - the resulting sources * - * @return {External~Promise} If no callback is given, the result will be provided in a promise. + * @return {external:Promise} If no callback is given, the result will be provided in a promise. * * @throws {CoreIllegalArgumentError} If some of the parameters don't match the input criteria. */ @@ -702,7 +702,7 @@ define([ * @param {Error|CoreIllegalArgumentError|CoreAssertError|null} callback.error - the result of the execution. * @param {module:Core~Node[]} callback.nodes - the resulting nodes. * - * @return {External~Promise} If no callback is given, the result will be provided in a promise. + * @return {external:Promise} If no callback is given, the result will be provided in a promise. * * @throws {CoreIllegalArgumentError} If some of the parameters don't match the input criteria. */ @@ -2205,6 +2205,7 @@ define([ * @param {Error|CoreIllegalArgumentError|CoreIllegalOperationError|CoreAssertError|null} callback.error - the * result of the execution. * + * @return {external:Promise} If no callback is given, the result will be provided in a promise. * @throws {CoreIllegalArgumentError} If some of the parameters don't match the input criteria. */ this.setGuid = function (node, guid, callback) { @@ -3074,7 +3075,7 @@ define([ * @param {object} callback.treeDiff - the difference between the two containment hierarchies in * a special JSON object * - * @return {External~Promise} - if the callback is not defined, the result is provided in a promise + * @return {external:Promise} - if the callback is not defined, the result is provided in a promise * like manner. * * @throws {CoreIllegalArgumentError} If some of the parameters don't match the input criteria. @@ -3099,6 +3100,8 @@ define([ * @param {function} [callback] * @param {Error|CoreIllegalArgumentError|CoreAssertError|null} callback.error - the result of the execution. * + * @return {external:Promise} If no callback is given, the result will be provided in a promise. + * * @throws {CoreIllegalArgumentError} If some of the parameters don't match the input criteria. */ this.applyTreeDiff = function (node, patch, callback) { @@ -3473,7 +3476,7 @@ define([ * @param {Error|CoreIllegalArgumentError|CoreIllegalOperationError|CoreAssertError|null} callback.error - the * result of the execution. * - * @return {External~Promise} If no callback is given, the result is provided in a promise like manner. + * @return {external:Promise} If no callback is given, the result is provided in a promise like manner. * * @throws {CoreIllegalArgumentError} If some of the parameters don't match the input criteria. */ @@ -3517,7 +3520,7 @@ define([ * @param {Error|CoreIllegalArgumentError|CoreIllegalOperationError|CoreAssertError|null} callback.error - the * status of the execution. * - * @return {External~Promise} If no callback is given, the result is presented in a promise like manner. + * @return {external:Promise} If no callback is given, the result is presented in a promise like manner. * * @throws {CoreIllegalArgumentError} If some of the parameters don't match the input criteria. */ @@ -3797,7 +3800,7 @@ define([ * @param {function} [callback] * @param {Error|CoreIllegalArgumentError|CoreAssertError|null} callback.error - the status of the execution. * - * @return {External~Promise} If no callback is given, the end of traverse is marked in a promise like + * @return {external:Promise} If no callback is given, the end of traverse is marked in a promise like * manner. * * @throws {CoreIllegalArgumentError} If some of the parameters don't match the input criteria. @@ -3898,7 +3901,7 @@ define([ * @param {Error|CoreIllegalArgumentError|CoreAssertError|null} callback.error - the status of the execution. * @param {module:Core~Node[]} callback.nodes - the found instances of the node. * - * @return {External~Promise} If no callback is given, the result will be provided in a promise + * @return {external:Promise} If no callback is given, the result will be provided in a promise * like manner. * * @throws {CoreIllegalArgumentError} If some of the parameters don't match the input criteria. @@ -3921,7 +3924,7 @@ define([ * @param {Error|CoreIllegalArgumentError|CoreAssertError|null} callback.error - the status of the execution. * @param {module:Core~Node[]} callback.nodes - the found members of the set of the node. * - * @return {External~Promise} If no callback is given, the result will be provided in a promise + * @return {external:Promise} If no callback is given, the result will be provided in a promise * like manner. * * @throws {CoreIllegalArgumentError} If some of the parameters don't match the input criteria. @@ -3945,7 +3948,7 @@ define([ * @param {Error|CoreIllegalArgumentError|CoreAssertError|null} callback.error - the status of the execution. * @param {module:Core~Node[]} callback.nodes - the found own members of the set of the node. * - * @return {External~Promise} If no callback is given, the result will be provided in a promise + * @return {external:Promise} If no callback is given, the result will be provided in a promise * like manner. * * @throws {CoreIllegalArgumentError} If some of the parameters don't match the input criteria. diff --git a/src/plugin/PluginBase.js b/src/plugin/PluginBase.js index 65301e9f9..1e5b7d2a5 100644 --- a/src/plugin/PluginBase.js +++ b/src/plugin/PluginBase.js @@ -143,7 +143,9 @@ define([ *
- Do NOT put any user interaction logic UI, etc. inside this function. *
- callback always have to be called even if error happened. * - * @param {function(string|Error, PluginResult)} callback - the result callback + * @param {function} callback - the result callback + * @param {null|Error} callback.err - status of the call + * @param {PluginResult} callback.result - plugin result */ PluginBase.prototype.main = function (/*callback*/) { throw new Error('implement this function in the derived class'); @@ -381,6 +383,7 @@ define([ * @param {boolean} [message.toBranch=false] - If true, and the plugin is running on the server on a branch - * will broadcast to all sockets in the branch room. * @param {function(Error)} [callback] - optional callback invoked when message has been emitted from server. + * @param {null|Error} callback.err - status of the call */ PluginBase.prototype.sendNotification = function (message, callback) { var self = this, @@ -428,7 +431,10 @@ define([ * To report the commits in the PluginResult make sure to invoke this.addCommitToResult with the given status. * * @param {string|null} message - commit message - * @param {function(Error, module:Storage~commitResult)} callback + * @param {function} [callback] - the result callback + * @param {null|Error} callback.err - status of the call + * @param {module:Storage~commitResult} callback.commitResult - status of the commit made + * @return {external:Promise} If no callback is given, the result will be provided in a promise */ PluginBase.prototype.save = function (message, callback) { var self = this, @@ -532,9 +538,10 @@ define([ * N.B. Use this with caution, for instance manually referenced nodes in a plugin will still be part of the * previous commit. Additionally if the namespaces have changed between commits - the this.META might end up * being empty. - * - * @param {function(Error, boolean)} [callback] - Resolved with true if branch had moved forward. - * @returns {Promise} + * @param {function} [callback] - the result callback + * @param {null|Error} callback.err - status of the call + * @param {boolean} callback.didUpdate - true if there was a change and it updated the state to it + * @return {external:Promise} If no callback is given, the result will be provided in a promise */ PluginBase.prototype.fastForward = function (callback) { var self = this, @@ -622,11 +629,13 @@ define([ }; /** - * Loads all the nodes starting from node and returns a map from paths to nodes. + * Loads all the nodes in the subtree starting from node and returns a map from paths to nodes. * @param {module:Core~Node} [node=self.rootNode] - Optional node to preload nodes from, * by default all will be loaded. - * @param {function} [callback] - if defined no promise will be returned - * @return {external:Promise} - If successful will resolve with object where keys are paths and values nodes. + * @param {function} [callback] - the result callback + * @param {null|Error} callback.err - status of the call + * @param {object} callback.nodeMap - keys are paths and values are nodes + * @return {external:Promise} If no callback is given, the result will be provided in a promise */ PluginBase.prototype.loadNodeMap = function (node, callback) { var self = this; @@ -660,8 +669,10 @@ define([ * @param {object} [context.pluginConfig] - Specific configuration parameters that should be used for the invocation. * If not provided will first check if the currentConfig of this plugin contains this plugin as dependency within * the array this._currentConfig._dependencies. Finally it will fall back to the default config of the plugin. - * @param {function(Error, InterPluginResult)} [callback] - * @returns {*} + * @param {function} [callback] - the result callback + * @param {null|Error} callback.err - status of the call + * @param {InterPluginResult} callback.result - result from the invoked plugin + * @return {external:Promise} If no callback is given, the result will be provided in a promise */ PluginBase.prototype.invokePlugin = function (pluginId, context, callback) { var self = this, From bb5aaa28fac9887afa8b920f36eaf5a2ca5671a4 Mon Sep 17 00:00:00 2001 From: Patrik Meijer Date: Thu, 9 Nov 2017 16:12:05 -0600 Subject: [PATCH 2/4] WIP Add InterPluginResult to docs --- jsdoc_conf.json | 1 + 1 file changed, 1 insertion(+) diff --git a/jsdoc_conf.json b/jsdoc_conf.json index b2c791a75..323f44e21 100644 --- a/jsdoc_conf.json +++ b/jsdoc_conf.json @@ -17,6 +17,7 @@ "src/plugin/PluginMessage.js", "src/plugin/PluginNodeDescription.js", "src/plugin/PluginResult.js", + "src/plugin/InterPluginResult.js", "src/common/executor", "src/common/core/core.js", "src/server/middleware/auth/gmeauth.js", From d9e8cdd81cefd161b376d899229bbcc7788e1942 Mon Sep 17 00:00:00 2001 From: Patrik Meijer Date: Tue, 14 Nov 2017 09:41:15 -0600 Subject: [PATCH 3/4] WIP Fix misleading comments in code --- src/common/core/librarycore.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/common/core/librarycore.js b/src/common/core/librarycore.js index eed5e2df6..9150290b8 100644 --- a/src/common/core/librarycore.js +++ b/src/common/core/librarycore.js @@ -361,11 +361,10 @@ define([ } function checkClosure(allMetaNodes, closureInformation) { - //here we only check for exact GUID matches - //TODO we might be able to map even with no exact GUID match based on library information var keys = Object.keys(allMetaNodes), occurrences, i, j, errorTxt; + // First check against direct GUID matches.. closureInformation.destinationBases = {}; for (i = 0; i < keys.length; i += 1) { closureInformation.destinationBases[self.getGuid(allMetaNodes[keys[i]])] = keys[i]; @@ -375,6 +374,7 @@ define([ for (i = 0; i < keys.length; i += 1) { if (!closureInformation.destinationBases[keys[i]]) { + // ... if no match try to find a unique match based on library GUIDs in the current project. occurrences = gatherOccurancesOfType(keys[i], closureInformation, allMetaNodes); if (occurrences.length === 0) { throw new CoreIllegalOperationError('Cannot find necessary base [' + From 39dfd05d47d183b34c33272fe2660facc64816c5 Mon Sep 17 00:00:00 2001 From: Patrik Meijer Date: Tue, 14 Nov 2017 10:06:44 -0600 Subject: [PATCH 4/4] WIP Minor improvement in doc --- src/common/core/core.js | 8 ++++---- src/common/core/librarycore.js | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/common/core/core.js b/src/common/core/core.js index bebe9e8e4..18b3f9c72 100644 --- a/src/common/core/core.js +++ b/src/common/core/core.js @@ -3677,11 +3677,11 @@ define([ }; /** - * Returns the origin GUID of any library node. - * + * Returns the origin GUID of any library node. (If name is not provided the returned GUID will be the same + * across all projects where the library node is contained - regardless of library hierarchy.) * @param {module:Core~Node} node - the node in question. - * @param {undefined | string} [name] - name of the library where we want to deduct the GUID from. If not given, - * than the GUID is computed from the direct library root of the node + * @param {undefined | string} [name] - name of the library where we want to compute the GUID from. If not given, + * then the GUID is computed from the direct library root of the node. * * @return {module:Core~GUID | Error} - Returns the origin GUID of the node or * error if the query cannot be fulfilled. diff --git a/src/common/core/librarycore.js b/src/common/core/librarycore.js index 9150290b8..6e4ec8fb8 100644 --- a/src/common/core/librarycore.js +++ b/src/common/core/librarycore.js @@ -374,7 +374,7 @@ define([ for (i = 0; i < keys.length; i += 1) { if (!closureInformation.destinationBases[keys[i]]) { - // ... if no match try to find a unique match based on library GUIDs in the current project. + // ... if no match try to find a unique match based on library GUIDs. occurrences = gatherOccurancesOfType(keys[i], closureInformation, allMetaNodes); if (occurrences.length === 0) { throw new CoreIllegalOperationError('Cannot find necessary base [' +