Implement to call function which take dictionaries type as parameter #185
Conversation
|
i am fine. because these days i am focusing on chromium. :) |
|
|
||
| import IDLDefinition from './idl_definition'; | ||
| import IDLDictionary from './idl_dictionary'; | ||
|
|
There was a problem hiding this comment.
Please remove unnecessary blank.
generator/parser/dictionary_types.ts
Outdated
|
|
||
|
|
||
| export default class DictionaryTypes { | ||
| dictionaries: IDLDictionary[]; |
There was a problem hiding this comment.
i added readonly keyword.
generator/parser/idl_dictionary.ts
Outdated
| } | ||
|
|
||
| export default class IDLDictionary extends IDLDefinition { | ||
| members: DictionaryMember[]; |
There was a problem hiding this comment.
i added readonly keyword.
template/interface_cpp.njk
Outdated
| @@ -20,6 +20,9 @@ if (!EnumValidator::isValildEnum({{argument.name}}, enum_value_set)) { | |||
| .ThrowAsJavaScriptException(); | |||
| return{% if not is_constructor %} Napi::Value(){% endif %}; | |||
| } | |||
| {% elif argument.dictionary %} | |||
| {{argument.type | pascalcase}} {{argument.name}}; | |||
| //FIXME(hwanseung): Make NatvieTypeTraits for dictionary types. | |||
There was a problem hiding this comment.
Should be // FIXME(hwanseung).
template/interface_cpp.njk
Outdated
| @@ -20,6 +20,9 @@ if (!EnumValidator::isValildEnum({{argument.name}}, enum_value_set)) { | |||
| .ThrowAsJavaScriptException(); | |||
| return{% if not is_constructor %} Napi::Value(){% endif %}; | |||
| } | |||
| {% elif argument.dictionary %} | |||
| {{argument.type | pascalcase}} {{argument.name}}; | |||
| //FIXME(hwanseung): Make NatvieTypeTraits for dictionary types. | |||
There was a problem hiding this comment.
BTW, why do we need NativeTypeTraits for dictionary?
You already implemented the dictionary to be generated as C++ code(native code).
There was a problem hiding this comment.
yes i made some code to be generated as C++ code.
but present, javascript object will not converted to c++ object.
actually that code is not exist yet. it should be implement.
There was a problem hiding this comment.
Please check this comment as well :)
There was a problem hiding this comment.
i left comment. but it was pended. sorry.
There was a problem hiding this comment.
The following code might make some trouble like unused variable error as per compiler.
{{argument.type | pascalcase}} {{argument.name}};
The NativeTypeTraits should be implemented for primitive types. I'm not sure if we should make all custom types. Moreover, IDL dictionary type's name matches with native dictionary type's name perfectly in current implementation. So, I think the generated code should include the converter codes or generate converter codes here without adding a new NativeTypeTraits.
There was a problem hiding this comment.
i didn't know about which NativeTypeTraits is only for primitive types.
i just want to comment out in order to make some converting codes to native dictionary type.
so i will change comment.
and {{argument.type | pascalcase}} {{argument.name}}; is used to pass parameter like below.
TestDict dicValue;
// FIXME(hwanseung): Convert to native dictionary object from javascript object.
impl_->
VoidMethodTestDictionaryArg(dicValue);
template/dictionary_types.njk
Outdated
| {% endfor %} | ||
| private: | ||
| {% for member in members %} | ||
| bool has_{{member.name}}_ = false; |
There was a problem hiding this comment.
Always false?
It should be checked in runtime, so, it should not be generated I think.
There was a problem hiding this comment.
no, when javascript object will be converted to c++ object, it should be set true if that javascript object's attribute has value. actually that code is not exist yet. it should be implement.
There was a problem hiding this comment.
Yeah, I know but do we have to have the member variable to check whether it exists or not?
Is it just cache?
There was a problem hiding this comment.
um.. if there were boolean value in dictionary. and if it was passed as false value, it will be too hard to know that value came from javascript or it was just initial value.
so i think we have to know it was set or not.
There was a problem hiding this comment.
um.. if there were boolean value in dictionary. and if it was passed as false value, it will be
too hard to know that value came from javascript or it was just initial value.
so i think we have to know it was set or not.
The initial value you mentioned is a default value in WebIDL?
There was a problem hiding this comment.
No. it means initial value of native's members. but i am not sure it will be used or not.
so i will remove it now.
Signed-off-by: Hwanseung Lee <hs1217.lee@samsung.com>
Signed-off-by: Hwanseung Lee <hs1217.lee@samsung.com>
This patch is first step to implement dictionaries feature.
it is possible only to call function which take dictionaries type
as parameter in this patch. so we should implement below things
to enhance dictionaries feature.
ISSUE=#169