Skip to content

Commit 0458b53

Browse files
johnhaley81johnhaley81
committed
Merge pull request nodegit#416 from nodegit/fix-callbacks
Fix callbacks with just return value and single payload
2 parents 663f693 + dece787 commit 0458b53

File tree

14 files changed

+159
-60
lines changed

14 files changed

+159
-60
lines changed

‎generate/input/callbacks.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -455,7 +455,7 @@
455455
],
456456
"return": {
457457
"type": "int",
458-
"noResults": 1,
458+
"noResults": 0,
459459
"success": 0,
460460
"error": -1
461461
}
@@ -495,7 +495,7 @@
495495
],
496496
"return": {
497497
"type": "int",
498-
"noResults": 1,
498+
"noResults": 0,
499499
"success": 0,
500500
"error": -1
501501
}

‎generate/scripts/helpers.js‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,7 @@ var Helpers = {
121121
processPayload: function(field, allFields) {
122122
if (field.name === "payload") {
123123
field.payloadFor = "*";
124+
field.globalPayload = true;
124125
field.isOptional = true;
125126
}
126127
else {
@@ -302,6 +303,11 @@ var Helpers = {
302303
var argOverrides = fnOverrides.args || {};
303304
fnDef.args.forEach(function(arg) {
304305
Helpers.decorateArg(arg, fnDef.args, typeDef, fnDef, argOverrides[arg.name] || {}, enums);
306+
307+
// if a function has any callbacks then it MUST be async
308+
if (arg.isCallbackFunction) {
309+
fnDef.isAsync = true;
310+
}
305311
});
306312

307313
if (fnDef.return) {

‎generate/templates/filters/returns_info.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,12 @@ module.exports = function(fn, argReturnsOnly, isAsync) {
1414
return_info.parsedClassName = (return_info.cppClassName || '').toLowerCase() + "_t";
1515
return_info.returnNameOrName = return_info.returnName || return_info.name;
1616
return_info.jsOrCppClassName = return_info.jsClassName || return_info.cppClassName;
17+
return_info.isOutParam = true;
1718

1819
result.push(return_info);
1920
});
2021

2122
if (!result.length
22-
&& !fn.isCallbackFunction
2323
&& !argReturnsOnly
2424
&& fn.return
2525
&& !fn.return.isErrorCode

‎generate/templates/manual/src/str_array_converter.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ git_strarray *StrArrayConverter::ConvertArray(Array *val) {
3030
git_strarray *result;
3131

3232
for(int i = 0; i < count; i++) {
33-
NanUtf8String entry(val->CloneElementAt(i));
33+
NanUtf8String entry(val->Get(i));
3434
strings[i] = *entry;
3535
}
3636

‎generate/templates/partials/async_function.cc‎

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,21 +12,37 @@ NAN_METHOD({{ cppClassName }}::{{ cppFunctionName }}) {
1212
baton->error_code = GIT_OK;
1313
baton->error = NULL;
1414

15+
{%each args|argsInfo as arg %}
16+
{%if arg.globalPayload %}
17+
{{ cppFunctionName }}_globalPayload* globalPayload = new {{ cppFunctionName }}_globalPayload;
18+
{%endif%}
19+
{%endeach%}
20+
1521
{%each args|argsInfo as arg %}
1622
{%if not arg.isReturn %}
1723
{%if arg.isSelf %}
1824
baton->{{ arg.name }} = ObjectWrap::Unwrap<{{ arg.cppClassName }}>(args.This())->GetValue();
1925
{%elsif arg.isCallbackFunction %}
2026
if (!args[{{ arg.jsArg }}]->IsFunction()) {
2127
baton->{{ arg.name }} = NULL;
28+
{%if arg.payload.globalPayload %}
29+
globalPayload->{{ arg.name }} = NULL;
30+
{%else%}
2231
baton->{{ arg.payload.name }} = NULL;
32+
{%endif%}
2333
}
2434
else {
2535
baton->{{ arg.name}} = {{ cppFunctionName }}_{{ arg.name }}_cppCallback;
36+
{%if arg.payload.globalPayload %}
37+
globalPayload->{{ arg.name }} = new NanCallback(args[{{ arg.jsArg }}].As<Function>());
38+
{%else%}
2639
baton->{{ arg.payload.name }} = new NanCallback(args[{{ arg.jsArg }}].As<Function>());
40+
{%endif%}
2741
}
2842
{%elsif arg.payloadFor %}
29-
{%-- payloads are ignored --%}
43+
{%if arg.globalPayload %}
44+
baton->{{ arg.name }} = globalPayload;
45+
{%endif%}
3046
{%elsif arg.name %}
3147
{%partial convertFromV8 arg%}
3248
{%if not arg.payloadFor %}
@@ -134,7 +150,11 @@ void {{ cppClassName }}::{{ cppFunctionName }}Worker::HandleOKCallback() {
134150
free((void*)baton->{{ arg.name }});
135151
}
136152
{%elsif arg.isCallbackFunction %}
153+
{%if not arg.payload.globalPayload %}
137154
delete baton->{{ arg.payload.name }};
155+
{%endif%}
156+
{%elsif arg.globalPayload %}
157+
delete ({{ cppFunctionName}}_globalPayload*)baton->{{ arg.name }};
138158
{%else%}
139159
free((void*)baton->{{ arg.name }});
140160
{%endif%}
@@ -159,7 +179,11 @@ void {{ cppClassName }}::{{ cppFunctionName }}Worker::HandleOKCallback() {
159179
free((void *)baton->{{ arg.name }});
160180
}
161181
{%elsif arg.isCallbackFunction %}
162-
delete (NanCallback *)baton->{{ arg.payload.name }};
182+
{%if not arg.payload.globalPayload %}
183+
delete baton->{{ arg.payload.name }};
184+
{%endif%}
185+
{%elsif arg.globalPayload %}
186+
delete ({{ cppFunctionName}}_globalPayload*)baton->{{ arg.name }};
163187
{%endif%}
164188
{%endeach%}
165189

‎generate/templates/partials/callback_helpers.cc‎

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,10 @@
2222
this_thread::sleep_for(chrono::milliseconds(1));
2323
}
2424

25-
{% each cbFunction|returnsInfo true false as _return %}
25+
{% each cbFunction|returnsInfo false true as _return %}
26+
{% if _return.isOutParam %}
2627
*{{ _return.name }} = *baton->{{ _return.name }};
28+
{% endif %}
2729
{% endeach %}
2830

2931
return baton->result;
@@ -42,7 +44,11 @@ void {{ cppClassName }}::{{ cppFunctionName }}_{{ cbFunction.name }}_asyncAfter(
4244

4345
{% each cbFunction.args|argsInfo as arg %}
4446
{% if arg | isPayload %}
47+
{% if cbFunction.payload.globalPayload %}
48+
NanCallback* callback = (({{ cppFunctionName }}_globalPayload*)baton->{{ arg.name }})->{{ cbFunction.name }};
49+
{% else %}
4550
NanCallback* callback = (NanCallback *)baton->{{ arg.name }};
51+
{% endif %}
4652
{% endif %}
4753
{% endeach %}
4854

@@ -84,18 +90,25 @@ void {{ cppClassName }}::{{ cppFunctionName }}_{{ cbFunction.name }}_asyncAfter(
8490
}
8591
}
8692

87-
{{ cbFunction.return.type }} resultStatus;
88-
89-
{% each cbFunction|returnsInfo true false as _return %}
93+
{% each cbFunction|returnsInfo false true as _return %}
9094
if (result.IsEmpty() || result->IsNativeError()) {
9195
baton->result = {{ cbFunction.return.error }};
9296
}
9397
else if (!result->IsNull() && !result->IsUndefined()) {
98+
{% if _return.isOutParam %}
9499
{{ _return.cppClassName }}* wrapper = ObjectWrap::Unwrap<{{ _return.cppClassName }}>(result->ToObject());
95100
wrapper->selfFreeing = false;
96101

97102
baton->{{ _return.name }} = wrapper->GetRefValue();
98103
baton->result = {{ cbFunction.return.success }};
104+
{% else %}
105+
if (result->IsNumber()) {
106+
baton->result = (int)result->ToNumber()->Value();
107+
}
108+
else {
109+
baton->result = {{ cbFunction.return.noResults }};
110+
}
111+
{% endif %}
99112
}
100113
else {
101114
baton->result = {{ cbFunction.return.noResults }};
@@ -124,18 +137,26 @@ void {{ cppClassName }}::{{ cppFunctionName }}_{{ cbFunction.name }}_asyncPromis
124137
if (isFulfilled->Value()) {
125138
NanCallback* resultFn = new NanCallback(promise->Get(NanNew("value")).As<Function>());
126139
Handle<v8::Value> result = resultFn->Call(0, argv);
127-
{{ cbFunction.return.type }} resultStatus;
128140

129-
{% each cbFunction|returnsInfo true false as _return %}
141+
{% each cbFunction|returnsInfo false true as _return %}
130142
if (result.IsEmpty() || result->IsNativeError()) {
131143
baton->result = {{ cbFunction.return.error }};
132144
}
133145
else if (!result->IsNull() && !result->IsUndefined()) {
146+
{% if _return.isOutParam %}
134147
{{ _return.cppClassName }}* wrapper = ObjectWrap::Unwrap<{{ _return.cppClassName }}>(result->ToObject());
135148
wrapper->selfFreeing = false;
136149

137150
baton->{{ _return.name }} = wrapper->GetRefValue();
138151
baton->result = {{ cbFunction.return.success }};
152+
{% else %}
153+
if (result->IsNumber()) {
154+
baton->result = (int)result->ToNumber()->Value();
155+
}
156+
else {
157+
baton->result = {{ cbFunction.return.noResults }};
158+
}
159+
{% endif %}
139160
}
140161
else {
141162
baton->result = {{ cbFunction.return.noResults }};

‎generate/templates/partials/field_accessors.cc‎

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,10 @@
4242
wrapper->raw->{{ field.name }} = {% if not field.cType | isPointer %}*{% endif %}ObjectWrap::Unwrap<{{ field.cppClassName }}>(value->ToObject())->GetValue();
4343

4444
{% elsif field.isCallbackFunction %}
45+
if (wrapper->{{ field.name }} != NULL) {
46+
delete wrapper->{{ field.name }};
47+
}
48+
4549
if (value->IsFunction()) {
4650
if (!wrapper->raw->{{ field.name }}) {
4751
wrapper->raw->{{ field.name }} = ({{ field.cType }}){{ field.name }}_cppCallback;
@@ -94,8 +98,10 @@
9498
this_thread::sleep_for(chrono::milliseconds(1));
9599
}
96100

97-
{% each field|returnsInfo true false as _return %}
101+
{% each field|returnsInfo false true as _return %}
102+
{% if _return.isOutParam %}
98103
*{{ _return.name }} = *baton->{{ _return.name }};
104+
{% endif %}
99105
{% endeach %}
100106

101107
return baton->result;
@@ -173,18 +179,25 @@
173179
}
174180
}
175181

176-
{{ field.return.type }} resultStatus;
177-
178-
{% each field|returnsInfo true false as _return %}
182+
{% each field|returnsInfo false true as _return %}
179183
if (result.IsEmpty() || result->IsNativeError()) {
180184
baton->result = {{ field.return.error }};
181185
}
182186
else if (!result->IsNull() && !result->IsUndefined()) {
187+
{% if _return.isOutParam %}
183188
{{ _return.cppClassName }}* wrapper = ObjectWrap::Unwrap<{{ _return.cppClassName }}>(result->ToObject());
184189
wrapper->selfFreeing = false;
185190

186191
baton->{{ _return.name }} = wrapper->GetRefValue();
187192
baton->result = {{ field.return.success }};
193+
{% else %}
194+
if (result->IsNumber()) {
195+
baton->result = (int)result->ToNumber()->Value();
196+
}
197+
else {
198+
baton->result = {{ field.return.noResults }};
199+
}
200+
{% endif %}
188201
}
189202
else {
190203
baton->result = {{ field.return.noResults }};
@@ -213,18 +226,26 @@
213226
if (isFulfilled->Value()) {
214227
NanCallback* resultFn = new NanCallback(promise->Get(NanNew("value")).As<Function>());
215228
Handle<v8::Value> result = resultFn->Call(0, argv);
216-
{{ field.return.type }} resultStatus;
217229

218-
{% each field|returnsInfo true false as _return %}
230+
{% each field|returnsInfo false true as _return %}
219231
if (result.IsEmpty() || result->IsNativeError()) {
220232
baton->result = {{ field.return.error }};
221233
}
222234
else if (!result->IsNull() && !result->IsUndefined()) {
235+
{% if _return.isOutParam %}
223236
{{ _return.cppClassName }}* wrapper = ObjectWrap::Unwrap<{{ _return.cppClassName }}>(result->ToObject());
224237
wrapper->selfFreeing = false;
225238

226239
baton->{{ _return.name }} = wrapper->GetRefValue();
227240
baton->result = {{ field.return.success }};
241+
{% else %}
242+
if (result->IsNumber()) {
243+
baton->result = (int)result->ToNumber()->Value();
244+
}
245+
else{
246+
baton->result = {{ field.return.noResults }};
247+
}
248+
{% endif %}
228249
}
229250
else {
230251
baton->result = {{ field.return.noResults }};

‎generate/templates/partials/sync_function.cc‎

Lines changed: 2 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -15,27 +15,20 @@ NAN_METHOD({{ cppClassName }}::{{ cppFunctionName }}) {
1515
{%each args|argsInfo as arg %}
1616
{%if not arg.isSelf %}
1717
{%if not arg.isReturn %}
18-
{%if not arg.isCallbackFunction %}
19-
{%if not arg.payloadFor %}
20-
{%partial convertFromV8 arg %}
21-
{%if arg.saveArg %}
18+
{%partial convertFromV8 arg %}
19+
{%if arg.saveArg %}
2220
Handle<Object> {{ arg.name }}(args[{{ arg.jsArg }}]->ToObject());
2321
{{ cppClassName }} *thisObj = ObjectWrap::Unwrap<{{ cppClassName }}>(args.This());
2422

2523
NanDisposePersistent(thisObj->{{ cppFunctionName }}_{{ arg.name }});
2624

2725
NanAssignPersistent(thisObj->{{ cppFunctionName }}_{{ arg.name }}, {{ arg.name }});
28-
{%endif%}
29-
{%endif%}
3026
{%endif%}
3127
{%endif%}
3228
{%endif%}
3329
{%endeach%}
3430

3531
{%each args|argsInfo as arg %}
36-
{%if arg.isCallbackFunction %}
37-
NanCallback* {{ arg.name }}_callback = new NanCallback(args[{{ arg.jsArg }}].As<Function>());
38-
{%endif%}
3932
{%endeach%}
4033
{%if .|hasReturns %}
4134
{{ return.cType }} result = {%endif%}{{ cFunctionName }}(
@@ -47,24 +40,13 @@ NanCallback* {{ arg.name }}_callback = new NanCallback(args[{{ arg.jsArg }}].As<
4740
ObjectWrap::Unwrap<{{ arg.cppClassName }}>(args.This())->GetValue()
4841
{%elsif arg.isReturn %}
4942
{{ arg.name }}
50-
{%elsif arg.isCallbackFunction %}
51-
{{ cppFunctionName }}_{{ arg.name }}_cppCallback,
52-
{{ arg.name }}_callback
53-
{%elsif arg.payloadFor %}
54-
{%-- payloads are handled inside of the callback condition --%}
5543
{%else%}
5644
from_{{ arg.name }}
5745
{%endif%}
5846
{%if not arg.lastArg %},{%endif%}
5947
{%endeach%}
6048
);
6149

62-
{%each args|argsInfo as arg %}
63-
{%if arg.isCallbackFunction %}
64-
delete {{ arg.name }}_callback;
65-
{%endif%}
66-
{%endeach%}
67-
6850
{%if return.isErrorCode %}
6951
if (result != GIT_OK) {
7052
{%each args|argsInfo as arg %}
@@ -120,5 +102,3 @@ delete {{ arg.name }}_callback;
120102
{%endif%}
121103
{%endif%}
122104
}
123-
124-
{%partial callbackHelpers .%}

‎generate/templates/templates/class_header.h‎

Lines changed: 33 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -135,17 +135,43 @@ class {{ cppClassName }} : public ObjectWrap {
135135
};
136136
{%endif%}
137137

138-
{%each function.args as arg %}
139-
{%if arg.payloadFor %}
140-
141-
Persistent<Value> {{ function.cppFunctionName }}_{{ arg.name }};
142-
{%endif%}
143-
{%endeach%}
144-
145138
static NAN_METHOD({{ function.cppFunctionName }});
146139
{%endif%}
147140
{%endeach%}
148141

142+
{%each functions as function%}
143+
{%each function.args as arg %}
144+
{% if arg.globalPayload %}
145+
146+
struct {{ function.cppFunctionName }}_globalPayload {
147+
{%each function.args as arg %}
148+
{%if arg.isCallbackFunction %}
149+
NanCallback * {{ arg.name }};
150+
{%endif%}
151+
{%endeach%}
152+
153+
{{ function.cppFunctionName }}_globalPayload() {
154+
{%each function.args as arg %}
155+
{%if arg.isCallbackFunction %}
156+
{{ arg.name }} = NULL;
157+
{%endif%}
158+
{%endeach%}
159+
}
160+
161+
~{{ function.cppFunctionName }}_globalPayload() {
162+
{%each function.args as arg %}
163+
{%if arg.isCallbackFunction %}
164+
if ({{ arg.name }} != NULL) {
165+
delete {{ arg.name }};
166+
}
167+
{%endif%}
168+
{%endeach%}
169+
}
170+
};
171+
{%endif%}
172+
{%endeach%}
173+
{%endeach%}
174+
149175
{%if cType%}
150176
{{ cType }} *raw;
151177
{%endif%}

0 commit comments

Comments
 (0)