diff --git a/generate/input/callbacks.json b/generate/input/callbacks.json index 75c4b26e5..46dbb4654 100644 --- a/generate/input/callbacks.json +++ b/generate/input/callbacks.json @@ -455,7 +455,7 @@ ], "return": { "type": "int", - "noResults": 1, + "noResults": 0, "success": 0, "error": -1 } @@ -495,7 +495,7 @@ ], "return": { "type": "int", - "noResults": 1, + "noResults": 0, "success": 0, "error": -1 } diff --git a/generate/scripts/helpers.js b/generate/scripts/helpers.js index da7be6cf1..49cf3be44 100644 --- a/generate/scripts/helpers.js +++ b/generate/scripts/helpers.js @@ -121,6 +121,7 @@ var Helpers = { processPayload: function(field, allFields) { if (field.name === "payload") { field.payloadFor = "*"; + field.globalPayload = true; field.isOptional = true; } else { @@ -302,6 +303,11 @@ var Helpers = { var argOverrides = fnOverrides.args || {}; fnDef.args.forEach(function(arg) { Helpers.decorateArg(arg, fnDef.args, typeDef, fnDef, argOverrides[arg.name] || {}, enums); + + // if a function has any callbacks then it MUST be async + if (arg.isCallbackFunction) { + fnDef.isAsync = true; + } }); if (fnDef.return) { diff --git a/generate/templates/filters/returns_info.js b/generate/templates/filters/returns_info.js index c1a315aee..ff36e0c51 100644 --- a/generate/templates/filters/returns_info.js +++ b/generate/templates/filters/returns_info.js @@ -14,12 +14,12 @@ module.exports = function(fn, argReturnsOnly, isAsync) { return_info.parsedClassName = (return_info.cppClassName || '').toLowerCase() + "_t"; return_info.returnNameOrName = return_info.returnName || return_info.name; return_info.jsOrCppClassName = return_info.jsClassName || return_info.cppClassName; + return_info.isOutParam = true; result.push(return_info); }); if (!result.length - && !fn.isCallbackFunction && !argReturnsOnly && fn.return && !fn.return.isErrorCode diff --git a/generate/templates/manual/src/str_array_converter.cc b/generate/templates/manual/src/str_array_converter.cc index 131989c64..1f85a8bda 100644 --- a/generate/templates/manual/src/str_array_converter.cc +++ b/generate/templates/manual/src/str_array_converter.cc @@ -30,7 +30,7 @@ git_strarray *StrArrayConverter::ConvertArray(Array *val) { git_strarray *result; for(int i = 0; i < count; i++) { - NanUtf8String entry(val->CloneElementAt(i)); + NanUtf8String entry(val->Get(i)); strings[i] = *entry; } diff --git a/generate/templates/partials/async_function.cc b/generate/templates/partials/async_function.cc index e65d8ff08..2ed5089c3 100644 --- a/generate/templates/partials/async_function.cc +++ b/generate/templates/partials/async_function.cc @@ -12,6 +12,12 @@ NAN_METHOD({{ cppClassName }}::{{ cppFunctionName }}) { baton->error_code = GIT_OK; baton->error = NULL; + {%each args|argsInfo as arg %} + {%if arg.globalPayload %} + {{ cppFunctionName }}_globalPayload* globalPayload = new {{ cppFunctionName }}_globalPayload; + {%endif%} + {%endeach%} + {%each args|argsInfo as arg %} {%if not arg.isReturn %} {%if arg.isSelf %} @@ -19,14 +25,24 @@ NAN_METHOD({{ cppClassName }}::{{ cppFunctionName }}) { {%elsif arg.isCallbackFunction %} if (!args[{{ arg.jsArg }}]->IsFunction()) { baton->{{ arg.name }} = NULL; + {%if arg.payload.globalPayload %} + globalPayload->{{ arg.name }} = NULL; + {%else%} baton->{{ arg.payload.name }} = NULL; + {%endif%} } else { baton->{{ arg.name}} = {{ cppFunctionName }}_{{ arg.name }}_cppCallback; + {%if arg.payload.globalPayload %} + globalPayload->{{ arg.name }} = new NanCallback(args[{{ arg.jsArg }}].As()); + {%else%} baton->{{ arg.payload.name }} = new NanCallback(args[{{ arg.jsArg }}].As()); + {%endif%} } {%elsif arg.payloadFor %} - {%-- payloads are ignored --%} + {%if arg.globalPayload %} + baton->{{ arg.name }} = globalPayload; + {%endif%} {%elsif arg.name %} {%partial convertFromV8 arg%} {%if not arg.payloadFor %} @@ -134,7 +150,11 @@ void {{ cppClassName }}::{{ cppFunctionName }}Worker::HandleOKCallback() { free((void*)baton->{{ arg.name }}); } {%elsif arg.isCallbackFunction %} + {%if not arg.payload.globalPayload %} delete baton->{{ arg.payload.name }}; + {%endif%} + {%elsif arg.globalPayload %} + delete ({{ cppFunctionName}}_globalPayload*)baton->{{ arg.name }}; {%else%} free((void*)baton->{{ arg.name }}); {%endif%} @@ -159,7 +179,11 @@ void {{ cppClassName }}::{{ cppFunctionName }}Worker::HandleOKCallback() { free((void *)baton->{{ arg.name }}); } {%elsif arg.isCallbackFunction %} - delete (NanCallback *)baton->{{ arg.payload.name }}; + {%if not arg.payload.globalPayload %} + delete baton->{{ arg.payload.name }}; + {%endif%} + {%elsif arg.globalPayload %} + delete ({{ cppFunctionName}}_globalPayload*)baton->{{ arg.name }}; {%endif%} {%endeach%} diff --git a/generate/templates/partials/callback_helpers.cc b/generate/templates/partials/callback_helpers.cc index 8cbfc50c0..e8209c484 100644 --- a/generate/templates/partials/callback_helpers.cc +++ b/generate/templates/partials/callback_helpers.cc @@ -22,8 +22,10 @@ this_thread::sleep_for(chrono::milliseconds(1)); } - {% each cbFunction|returnsInfo true false as _return %} + {% each cbFunction|returnsInfo false true as _return %} + {% if _return.isOutParam %} *{{ _return.name }} = *baton->{{ _return.name }}; + {% endif %} {% endeach %} return baton->result; @@ -42,7 +44,11 @@ void {{ cppClassName }}::{{ cppFunctionName }}_{{ cbFunction.name }}_asyncAfter( {% each cbFunction.args|argsInfo as arg %} {% if arg | isPayload %} + {% if cbFunction.payload.globalPayload %} + NanCallback* callback = (({{ cppFunctionName }}_globalPayload*)baton->{{ arg.name }})->{{ cbFunction.name }}; + {% else %} NanCallback* callback = (NanCallback *)baton->{{ arg.name }}; + {% endif %} {% endif %} {% endeach %} @@ -84,18 +90,25 @@ void {{ cppClassName }}::{{ cppFunctionName }}_{{ cbFunction.name }}_asyncAfter( } } - {{ cbFunction.return.type }} resultStatus; - - {% each cbFunction|returnsInfo true false as _return %} + {% each cbFunction|returnsInfo false true as _return %} if (result.IsEmpty() || result->IsNativeError()) { baton->result = {{ cbFunction.return.error }}; } else if (!result->IsNull() && !result->IsUndefined()) { + {% if _return.isOutParam %} {{ _return.cppClassName }}* wrapper = ObjectWrap::Unwrap<{{ _return.cppClassName }}>(result->ToObject()); wrapper->selfFreeing = false; baton->{{ _return.name }} = wrapper->GetRefValue(); baton->result = {{ cbFunction.return.success }}; + {% else %} + if (result->IsNumber()) { + baton->result = (int)result->ToNumber()->Value(); + } + else { + baton->result = {{ cbFunction.return.noResults }}; + } + {% endif %} } else { baton->result = {{ cbFunction.return.noResults }}; @@ -124,18 +137,26 @@ void {{ cppClassName }}::{{ cppFunctionName }}_{{ cbFunction.name }}_asyncPromis if (isFulfilled->Value()) { NanCallback* resultFn = new NanCallback(promise->Get(NanNew("value")).As()); Handle result = resultFn->Call(0, argv); - {{ cbFunction.return.type }} resultStatus; - {% each cbFunction|returnsInfo true false as _return %} + {% each cbFunction|returnsInfo false true as _return %} if (result.IsEmpty() || result->IsNativeError()) { baton->result = {{ cbFunction.return.error }}; } else if (!result->IsNull() && !result->IsUndefined()) { + {% if _return.isOutParam %} {{ _return.cppClassName }}* wrapper = ObjectWrap::Unwrap<{{ _return.cppClassName }}>(result->ToObject()); wrapper->selfFreeing = false; baton->{{ _return.name }} = wrapper->GetRefValue(); baton->result = {{ cbFunction.return.success }}; + {% else %} + if (result->IsNumber()) { + baton->result = (int)result->ToNumber()->Value(); + } + else { + baton->result = {{ cbFunction.return.noResults }}; + } + {% endif %} } else { baton->result = {{ cbFunction.return.noResults }}; diff --git a/generate/templates/partials/field_accessors.cc b/generate/templates/partials/field_accessors.cc index cd3a1e4a0..dcd83937e 100644 --- a/generate/templates/partials/field_accessors.cc +++ b/generate/templates/partials/field_accessors.cc @@ -42,6 +42,10 @@ wrapper->raw->{{ field.name }} = {% if not field.cType | isPointer %}*{% endif %}ObjectWrap::Unwrap<{{ field.cppClassName }}>(value->ToObject())->GetValue(); {% elsif field.isCallbackFunction %} + if (wrapper->{{ field.name }} != NULL) { + delete wrapper->{{ field.name }}; + } + if (value->IsFunction()) { if (!wrapper->raw->{{ field.name }}) { wrapper->raw->{{ field.name }} = ({{ field.cType }}){{ field.name }}_cppCallback; @@ -94,8 +98,10 @@ this_thread::sleep_for(chrono::milliseconds(1)); } - {% each field|returnsInfo true false as _return %} + {% each field|returnsInfo false true as _return %} + {% if _return.isOutParam %} *{{ _return.name }} = *baton->{{ _return.name }}; + {% endif %} {% endeach %} return baton->result; @@ -173,18 +179,25 @@ } } - {{ field.return.type }} resultStatus; - - {% each field|returnsInfo true false as _return %} + {% each field|returnsInfo false true as _return %} if (result.IsEmpty() || result->IsNativeError()) { baton->result = {{ field.return.error }}; } else if (!result->IsNull() && !result->IsUndefined()) { + {% if _return.isOutParam %} {{ _return.cppClassName }}* wrapper = ObjectWrap::Unwrap<{{ _return.cppClassName }}>(result->ToObject()); wrapper->selfFreeing = false; baton->{{ _return.name }} = wrapper->GetRefValue(); baton->result = {{ field.return.success }}; + {% else %} + if (result->IsNumber()) { + baton->result = (int)result->ToNumber()->Value(); + } + else { + baton->result = {{ field.return.noResults }}; + } + {% endif %} } else { baton->result = {{ field.return.noResults }}; @@ -213,18 +226,26 @@ if (isFulfilled->Value()) { NanCallback* resultFn = new NanCallback(promise->Get(NanNew("value")).As()); Handle result = resultFn->Call(0, argv); - {{ field.return.type }} resultStatus; - {% each field|returnsInfo true false as _return %} + {% each field|returnsInfo false true as _return %} if (result.IsEmpty() || result->IsNativeError()) { baton->result = {{ field.return.error }}; } else if (!result->IsNull() && !result->IsUndefined()) { + {% if _return.isOutParam %} {{ _return.cppClassName }}* wrapper = ObjectWrap::Unwrap<{{ _return.cppClassName }}>(result->ToObject()); wrapper->selfFreeing = false; baton->{{ _return.name }} = wrapper->GetRefValue(); baton->result = {{ field.return.success }}; + {% else %} + if (result->IsNumber()) { + baton->result = (int)result->ToNumber()->Value(); + } + else{ + baton->result = {{ field.return.noResults }}; + } + {% endif %} } else { baton->result = {{ field.return.noResults }}; diff --git a/generate/templates/partials/sync_function.cc b/generate/templates/partials/sync_function.cc index eee87f4cc..8a8aab89e 100644 --- a/generate/templates/partials/sync_function.cc +++ b/generate/templates/partials/sync_function.cc @@ -15,27 +15,20 @@ NAN_METHOD({{ cppClassName }}::{{ cppFunctionName }}) { {%each args|argsInfo as arg %} {%if not arg.isSelf %} {%if not arg.isReturn %} - {%if not arg.isCallbackFunction %} - {%if not arg.payloadFor %} - {%partial convertFromV8 arg %} - {%if arg.saveArg %} + {%partial convertFromV8 arg %} + {%if arg.saveArg %} Handle {{ arg.name }}(args[{{ arg.jsArg }}]->ToObject()); {{ cppClassName }} *thisObj = ObjectWrap::Unwrap<{{ cppClassName }}>(args.This()); NanDisposePersistent(thisObj->{{ cppFunctionName }}_{{ arg.name }}); NanAssignPersistent(thisObj->{{ cppFunctionName }}_{{ arg.name }}, {{ arg.name }}); - {%endif%} - {%endif%} {%endif%} {%endif%} {%endif%} {%endeach%} {%each args|argsInfo as arg %} - {%if arg.isCallbackFunction %} -NanCallback* {{ arg.name }}_callback = new NanCallback(args[{{ arg.jsArg }}].As()); - {%endif%} {%endeach%} {%if .|hasReturns %} {{ return.cType }} result = {%endif%}{{ cFunctionName }}( @@ -47,11 +40,6 @@ NanCallback* {{ arg.name }}_callback = new NanCallback(args[{{ arg.jsArg }}].As< ObjectWrap::Unwrap<{{ arg.cppClassName }}>(args.This())->GetValue() {%elsif arg.isReturn %} {{ arg.name }} - {%elsif arg.isCallbackFunction %} -{{ cppFunctionName }}_{{ arg.name }}_cppCallback, -{{ arg.name }}_callback - {%elsif arg.payloadFor %} -{%-- payloads are handled inside of the callback condition --%} {%else%} from_{{ arg.name }} {%endif%} @@ -59,12 +47,6 @@ from_{{ arg.name }} {%endeach%} ); -{%each args|argsInfo as arg %} - {%if arg.isCallbackFunction %} -delete {{ arg.name }}_callback; - {%endif%} -{%endeach%} - {%if return.isErrorCode %} if (result != GIT_OK) { {%each args|argsInfo as arg %} @@ -120,5 +102,3 @@ delete {{ arg.name }}_callback; {%endif%} {%endif%} } - -{%partial callbackHelpers .%} diff --git a/generate/templates/templates/class_header.h b/generate/templates/templates/class_header.h index cb6abfde4..b633ce200 100644 --- a/generate/templates/templates/class_header.h +++ b/generate/templates/templates/class_header.h @@ -135,17 +135,43 @@ class {{ cppClassName }} : public ObjectWrap { }; {%endif%} - {%each function.args as arg %} - {%if arg.payloadFor %} - - Persistent {{ function.cppFunctionName }}_{{ arg.name }}; - {%endif%} - {%endeach%} - static NAN_METHOD({{ function.cppFunctionName }}); {%endif%} {%endeach%} + {%each functions as function%} + {%each function.args as arg %} + {% if arg.globalPayload %} + + struct {{ function.cppFunctionName }}_globalPayload { + {%each function.args as arg %} + {%if arg.isCallbackFunction %} + NanCallback * {{ arg.name }}; + {%endif%} + {%endeach%} + + {{ function.cppFunctionName }}_globalPayload() { + {%each function.args as arg %} + {%if arg.isCallbackFunction %} + {{ arg.name }} = NULL; + {%endif%} + {%endeach%} + } + + ~{{ function.cppFunctionName }}_globalPayload() { + {%each function.args as arg %} + {%if arg.isCallbackFunction %} + if ({{ arg.name }} != NULL) { + delete {{ arg.name }}; + } + {%endif%} + {%endeach%} + } + }; + {%endif%} + {%endeach%} + {%endeach%} + {%if cType%} {{ cType }} *raw; {%endif%} diff --git a/generate/templates/templates/struct_content.cc b/generate/templates/templates/struct_content.cc index 2f67fbdde..40a343ae5 100644 --- a/generate/templates/templates/struct_content.cc +++ b/generate/templates/templates/struct_content.cc @@ -41,8 +41,19 @@ using namespace std; } {{ cppClassName }}::~{{ cppClassName }}() { - // This is going to cause memory leaks. We'll have to solve that later - // TODO: Clean up memory better + {% each fields|fieldsInfo as field %} + {% if not field.ignore %} + {% if not field.isEnum %} + {% if field.isCallbackFunction %} + if (this->{{ field.name }} != NULL) { + delete this->{{ field.name }}; + this->raw->{{ fields|payloadFor field.name }} = NULL; + } + {% endif %} + {% endif %} + {% endif %} + {% endeach %} + if (this->selfFreeing) { free(this->raw); } @@ -65,7 +76,7 @@ void {{ cppClassName }}::ConstructFields() { // the current instance this->raw->{{ field.name }} = NULL; this->raw->{{ fields|payloadFor field.name }} = (void *)this; - this->{{ field.name }} = new NanCallback(); + this->{{ field.name }} = NULL; {% elsif field.payloadFor %} Local {{ field.name }} = NanUndefined(); diff --git a/lib/index.js b/lib/index.js index 686f36778..e4b9431d8 100644 --- a/lib/index.js +++ b/lib/index.js @@ -19,17 +19,17 @@ Index.prototype.entries = function() { var addAll = Index.prototype.addAll; Index.prototype.addAll = function(pathspec, flags, matchedCallback) { - return addAll.call(this, pathspec, flags, matchedCallback, null); + return addAll.call(this, pathspec || "*", flags, matchedCallback, null); }; var removeAll = Index.prototype.removeAll; Index.prototype.removeAll = function(pathspec, matchedCallback) { - return removeAll.call(this, pathspec, matchedCallback, null); + return removeAll.call(this, pathspec || "*", matchedCallback, null); }; var updateAll = Index.prototype.updateAll; Index.prototype.updateAll = function(pathspec, matchedCallback) { - return updateAll.call(this, pathspec, matchedCallback, null); + return updateAll.call(this, pathspec || "*", matchedCallback, null); }; module.exports = Index; diff --git a/package.json b/package.json index 1757a8731..eb5ea2d6a 100644 --- a/package.json +++ b/package.json @@ -62,7 +62,7 @@ "which-native-nodish": "^1.0.3" }, "devDependencies": { - "combyne": "^0.6.4", + "combyne": "^0.6.5", "istanbul": "^0.3.5", "js-beautify": "^1.5.4", "jshint": "^2.6.0", diff --git a/test/tests/clone.js b/test/tests/clone.js index bbe6dfa4e..6f485672e 100644 --- a/test/tests/clone.js +++ b/test/tests/clone.js @@ -19,7 +19,7 @@ describe("Clone", function() { var sshPrivateKey = local("../id_rsa"); // Set a reasonable timeout here now that our repository has grown. - this.timeout(15000); + this.timeout(30000); beforeEach(function() { return NodeGit.Promise.all([ diff --git a/test/tests/index.js b/test/tests/index.js index babf4cbb7..4521a2d72 100644 --- a/test/tests/index.js +++ b/test/tests/index.js @@ -5,6 +5,10 @@ var Promise = require("nodegit-promise"); var promisify = require("promisify-node"); var fse = promisify(require("fs-extra")); +var writeFile = promisify(function(filename, data, callback) { + return require("fs").writeFile(filename, data, {}, callback); +}); + describe("Index", function() { var Repository = require(local("../../lib/repository")); @@ -43,7 +47,9 @@ describe("Index", function() { var fileNames = Object.keys(fileContent); return Promise.all(fileNames.map(function(fileName) { - fse.writeFile(path.join(repo.workdir(), fileName), fileContent[fileName]); + return writeFile( + path.join(repo.workdir(), fileName), + fileContent[fileName]); })) .then(function() { return index.addAll(); @@ -57,7 +63,7 @@ describe("Index", function() { }) .then(function() { return Promise.all(fileNames.map(function(fileName) { - fse.remove(path.join(repo.workdir(), fileName)); + return fse.remove(path.join(repo.workdir(), fileName)); })); }) .then(function() { @@ -76,7 +82,9 @@ describe("Index", function() { var fileNames = Object.keys(fileContent); return Promise.all(fileNames.map(function(fileName) { - fse.writeFile(path.join(repo.workdir(), fileName), fileContent[fileName]); + return writeFile( + path.join(repo.workdir(), fileName), + fileContent[fileName]); })) .then(function() { return index.addAll(); @@ -99,7 +107,7 @@ describe("Index", function() { }) .then(function() { return Promise.all(fileNames.map(function(fileName) { - fse.remove(path.join(repo.workdir(), fileName)); + return fse.remove(path.join(repo.workdir(), fileName)); })); }) .then(function() { @@ -117,7 +125,9 @@ describe("Index", function() { var fileNames = Object.keys(fileContent); return Promise.all(fileNames.map(function(fileName) { - fse.writeFile(path.join(repo.workdir(), fileName), fileContent[fileName]); + return writeFile( + path.join(repo.workdir(), fileName), + fileContent[fileName]); })) .then(function() { return index.addAll();