Removed payload from callbacks since js can just use closure scope

nodegit · orderedlist · Feb 4, 2015 · Jan 30, 2015 · Jan 30, 2015 · Jan 30, 2015
commit 511699f51ef2a372f984927b0f3354d1e9cbd685
diff --git a/generate/combyne/manual/include/callbackWrapper.h b/generate/combyne/manual/include/callbackWrapper.h
diff --git a/generate/combyne/partials/async_function.cc b/generate/combyne/partials/async_function.cc
@@ -7,14 +7,6 @@ NAN_METHOD({{ cppClassName }}::{{ cppFunctionName }}) {
     return NanThrowError("Callback is required and must be a Function.");
   }
 
-  {%each args|argsInfo as arg %}
-    {%if arg.isCallbackFunction %}
-  CallbackWrapper* {{ arg.name }}_cbWrapper = (CallbackWrapper *)malloc(sizeof(CallbackWrapper));
-  {{ arg.name }}_cbWrapper->jsCallback = new NanCallback(args[{{ arg.jsArg }}].As<Function>());
-  NanAssignPersistent({{ arg.name }}_cbWrapper->payload, args[{{ arg.payload.jsArg }}]);
-    {%endif%}
-  {%endeach%}
-
   {{ cppFunctionName }}Baton* baton = new {{ cppFunctionName }}Baton;
 
   baton->error_code = GIT_OK;
@@ -26,12 +18,12 @@ NAN_METHOD({{ cppClassName }}::{{ cppFunctionName }}) {
   baton->{{ arg.name }} = ObjectWrap::Unwrap<{{ arg.cppClassName }}>(args.This())->GetValue();
       {%elsif arg.isCallbackFunction %}
   baton->{{ arg.name}} = {{ cppFunctionName }}_{{ arg.name }}_cppCallback;
-  baton->{{ arg.payload.name }} = {{ arg.name }}_cbWrapper;
+  baton->{{ arg.payload.name }} = new NanCallback(args[{{ arg.jsArg }}].As<Function>());
       {%elsif arg.payloadFor %}
-        {%-- payloads are handled inside of the callback condition --%}
+        {%-- payloads are ignored --%}
       {%elsif arg.name %}
   {%partial convertFromV8 arg%}
-        {%if not arg.isPayload %}
+        {%if not arg.payloadFor %}
   baton->{{ arg.name }} = from_{{ arg.name }};
           {%if arg | isOid %}
   baton->{{ arg.name }}NeedsFree = args[{{ arg.jsArg }}]->IsString();
@@ -135,11 +127,11 @@ void {{ cppClassName }}::{{ cppFunctionName }}Worker::HandleOKCallback() {
       baton->{{ arg.name}}NeedsFree = false;
       free((void*)baton->{{ arg.name }});
     }
+        {%elsif arg.isCallbackFunction %}
+    delete baton->{{ arg.payload.name }};
         {%else%}
     free((void*)baton->{{ arg.name }});
         {%endif%}
-      {%elsif arg.payloadFor%}
-    free(baton->{{ arg.name }});
       {%endif%}
     {%endeach%}
   }
@@ -160,10 +152,8 @@ void {{ cppClassName }}::{{ cppFunctionName }}Worker::HandleOKCallback() {
     baton->{{ arg.name}}NeedsFree = false;
     free((void *)baton->{{ arg.name }});
   }
-    {%elsif arg.isCallbackFunction%}
-  CallbackWrapper* cbWrapper = (CallbackWrapper *)baton->{{ arg.payload.name }};
-  NanDisposePersistent(cbWrapper->payload);
-  free(baton->{{ arg.payload.name }});
+    {%elsif arg.isCallbackFunction %}
+  delete (NanCallback *)baton->{{ arg.payload.name }};
     {%endif%}
   {%endeach%}
 

diff --git a/generate/combyne/partials/callback_helpers.cc b/generate/combyne/partials/callback_helpers.cc
@@ -39,13 +39,14 @@ void {{ cppClassName }}::{{ cppFunctionName }}_{{ cbFunction.name }}_asyncAfter(
 
   {{ cppFunctionName }}_{{ cbFunction.name|titleCase }}Baton* baton = static_cast<{{ cppFunctionName }}_{{ cbFunction.name|titleCase }}Baton*>(req->data);
 
-  CallbackWrapper* cbWrapper = (CallbackWrapper *)baton->payload;
+  NanCallback* callback = (NanCallback *)baton->payload;
 
   Local<Value> argv[{{ cbFunction.args|jsArgsCount }}] = {
     {% each cbFunction.args|argsInfo as arg %}
       {% if arg.name == "payload" %}
         {%-- payload is always the last arg --%}
-        NanNew(cbWrapper->payload)
+        // payload is null because we can use closure scope in javascript
+        NanUndefined()
       {% elsif arg.isJsArg %}
         {% if arg.isEnum %}
           NanNew((int)baton->{{ arg.name }}),
@@ -62,7 +63,7 @@ void {{ cppClassName }}::{{ cppFunctionName }}_{{ cbFunction.name }}_asyncAfter(
   };
 
   TryCatch tryCatch;
-  Handle<Value> result = cbWrapper->jsCallback->Call({{ cbFunction.args|jsArgsCount }}, argv);
+  Handle<Value> result = callback->Call({{ cbFunction.args|jsArgsCount }}, argv);
 
   if (result->IsObject() && result->ToObject()->Has(NanNew("then"))) {
     Handle<Value> thenProp = result->ToObject()->Get(NanNew("then"));

diff --git a/generate/combyne/partials/guard_arguments.cc b/generate/combyne/partials/guard_arguments.cc
@@ -2,20 +2,22 @@
 {%each args|argsInfo as arg%}
   {%if arg.isJsArg%}
     {%if not arg.isOptional%}
-      {%if arg | isOid %}
+      {%if not arg.payloadFor %}
+        {%if arg | isOid %}
   if (args.Length() == {{arg.jsArg}}
     || (!args[{{arg.jsArg}}]->IsObject() && !args[{{arg.jsArg}}]->IsString())) {
     return NanThrowError("{{arg.jsClassName}} {{arg.name}} is required.");
   }
-      {%elsif arg.isCallbackFunction %}
+        {%elsif arg.isCallbackFunction %}
   if (args.Length() == {{arg.jsArg}} || !args[{{arg.jsArg}}]->IsFunction()) {
     return NanThrowError("{{arg.jsClassName}} {{arg.name}} is required.");
   }
-      {%else%}
+        {%else%}
   if (args.Length() == {{arg.jsArg}} || !args[{{arg.jsArg}}]->Is{{arg.cppClassName|cppToV8}}()) {
     return NanThrowError("{{arg.jsClassName}} {{arg.name}} is required.");
   }
 
+        {%endif%}
       {%endif%}
     {%endif%}
   {%endif%}

diff --git a/generate/combyne/partials/sync_function.cc b/generate/combyne/partials/sync_function.cc
@@ -26,9 +26,7 @@ NAN_METHOD({{ cppClassName }}::{{ cppFunctionName }}) {
 
 {%each args|argsInfo as arg %}
   {%if arg.isCallbackFunction %}
-CallbackWrapper* {{ arg.name }}_cbWrapper = (CallbackWrapper *)malloc(sizeof(CallbackWrapper));
-{{ arg.name }}_cbWrapper->jsCallback = new NanCallback(args[{{ arg.jsArg }}].As<Function>());
-NanAssignPersistent({{ arg.name }}_cbWrapper->payload, args[{{ arg.payload.jsArg }}]);
+NanCallback* {{ arg.name }}_callback = new NanCallback(args[{{ arg.jsArg }}].As<Function>());
   {%endif%}
 {%endeach%}
 {%if .|hasReturns %}
@@ -43,7 +41,7 @@ ObjectWrap::Unwrap<{{ arg.cppClassName }}>(args.This())->GetValue()
 {{ arg.name }}
     {%elsif arg.isCallbackFunction %}
 {{ cppFunctionName }}_{{ arg.name }}_cppCallback,
-{{ arg.name }}_cbWrapper
+{{ arg.name }}_callback
     {%elsif arg.payloadFor %}
 {%-- payloads are handled inside of the callback condition --%}
     {%else%}
@@ -55,7 +53,7 @@ from_{{ arg.name }}
 
 {%each args|argsInfo as arg %}
   {%if arg.isCallbackFunction %}
-  free({{ arg.name }}_cbWrapper);
+delete {{ arg.name }}_callback;
   {%endif%}
 {%endeach%}
 

diff --git a/generate/combyne/templates/class_content.cc b/generate/combyne/templates/class_content.cc
@@ -11,7 +11,6 @@ extern "C" {
   {% endeach %}
 }
 
-#include "../include/callbackWrapper.h"
 #include "../include/functions/copy.h"
 #include "../include/macros.h"
 #include "../include/{{ filename }}.h"

diff --git a/generate/combyne/templates/class_header.h b/generate/combyne/templates/class_header.h
@@ -15,8 +15,6 @@ extern "C" {
 #include "{{ dependency }}"
 {%endeach%}
 
-#include "../include/callbackWrapper.h"
-
 {%if needsForwardDeclaration %}
 // Forward declaration.
 struct {{ cType }} {