Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rename feature #1477

Merged
merged 25 commits into from
Jul 31, 2017
Merged

Rename feature #1477

merged 25 commits into from
Jul 31, 2017

Conversation

kecso
Copy link
Member

@kecso kecso commented Jul 27, 2017

The feature allows for renaming operations.
It introduces renaming functions on the Core API as well as enabling their use of the Meta editor.
It also introduces new server worker requests that can propagate these renamings.

Closes #1427 #1194 #1464 #1467 #1469

Copy link
Contributor

@pmeijer pmeijer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Finished with the first go on the core.js


function gmeMetaRename(logger, state, storage, saveRoot) {

function renameConcept(nodePath, type, oldName, newName, callback) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gather all client ServerWorkerRequests in a file like this..


function renameConcept(nodePath, type, oldName, newName, callback) {
var parameters = {
command: 'renameConcept',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/src/common/Constants.js SERVER_WORKER_REQUESTS

});
}

function renameAttributeDefinition(nodePath, meta, oldName, newName, callback) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regarding naming - see if there is a pattern in the current ones...
client API function - SERVER_WORKER_REQUEST - and if it's there core.user API name

@@ -2357,6 +2357,21 @@ define([
};

/**
* Returns the list of the META defined pointer names of the node introduced by the nide.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nide

@@ -2372,6 +2387,21 @@ define([
};

/**
* Returns the list of the META defined set names of the node introduced by its rules.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line 2360 consistent descriptions (also compare with getOwnValidChildrenTypes etc)

};

/**
* Renames the given attribute definition of the node. It also renames the default value of the definition!
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It also renames the default value of the definition, but does not rename any values stored at instances.

/**
* Moves the given target definition over to a new pointer or set. As actual values in case of
* relation definitions vary quite a bit from the meta-targets, this function does not deals with
* the actual pointer/set target/members.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As actual values in case of relation definitions vary quite a bit from the meta-targets, this function does not deals with the actual pointer/set target/members.
->
Note this does not alter the actual pointer target or set members.

};

/**
* Moves the given target definition over to a new aspect. As actual values in case of
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See above.

};

/**
* Returns if a node could be contained in the given container's aspect.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Returns if node is a valid aspect member of parent. Keep the same terminology

};

/**
* Returns the paths of Meta nodes that are possible targets of the given pointer/set introduced by the node.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...introduced by the node. -> ...where the node is the owner.

if (takenRelids) {
if (takenRelids[innerCore.getRelid(oldNode)]) {
node = innerCore.createChild(parent, takenRelids, relidLength);
if (typeof newRelid === 'string') {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't the same check as in createNode be applied here? (I know the newRelid isn't exposed at the top level core)


ASSERT(targetPath !== undefined);

if (pointerNames.indexOf(newName) !== -1) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we throw here?

console.log(self.getPath(node), oldName, newName);
self.setProperty(node, newName, self.getProperty(node, oldName));
self.deleteProperty(node, oldName);
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

console and what happens if the property already exists? Shouldn't we throw?

@@ -558,14 +601,38 @@ define([

this.setPointerMetaTarget = function (node, name, target, min, max) {
self.addMember(metaPointerNode(node, name), CONSTANTS.SET_ITEMS, target);
min = min || -1;
min = min === 0 ? 0 : min || -1;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add parenthesis to clearly show the order min = min === 0 ? 0 : (min || -1);

@@ -574,10 +641,10 @@ define([
};

this.setPointerMetaLimits = function (node, name, min, max) {
if (min) {
if (min || min === 0) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typeof min === 'number' ?


if (definedTarget) {
return {
sourceNode: node,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

owner*

};

this.getChildDefinitionInfo = function (node, child) {
return getFirstMatchingRuleInfo(node, undefined, child, self.getValidChildrenPaths, {});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be getOwnValidChildrenPaths

* actual definition was already changed among the meta definitions of the node.
* @param core
* @param node - any node in tree to be checked
* @param parameters
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doc for the parameters

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't the renames be batched?

return;
})
.catch(function (err) {
deferred.resolve(null);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should all errors be swallowed here?

@@ -1379,6 +1381,262 @@ function WorkerRequests(mainLogger, gmeConfig, webgmeUrl) {
.nodeify(finish);
}

function renameConcept(webgmeToken, parameters, callback) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

jsdoc

kecso added a commit that referenced this pull request Jul 29, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants