mods to add Camel and snake_case for proto #13
Conversation
src/index.js
Outdated
| @@ -185,65 +186,81 @@ function toProto(doc) { | |||
| // params | |||
| coll = doc.alps.descriptor.filter(semantic); | |||
| coll.forEach(function(msg) { | |||
| rtn += `message ${msg.id}Params {\n`; | |||
| val = makeSnakeCase(msg.id); | |||
There was a problem hiding this comment.
makePascalCase() should be applied here
There was a problem hiding this comment.
OK. will make this change.-- #thanks
There was a problem hiding this comment.
pretty sure these changes resullt in an invalid proto file:
syntax = "proto3";
package ToDo_API;
// *******************************************************************
// generated by "unified" from todo-alps.yaml
// date: Thu Dec 31 2020 19:28:00 GMT-0500 (Eastern Standard Time)
// http://github.com/mamund/2020-11-unified
// *******************************************************************
message Id_params {
string Id = 1;
}
message Body_params {
string Body = 1;
}
message TodoItem {
string id = 1;
string body = 2;
}
message TodoItem_response {
repeated TodoItem TodoItem_collection = 1;
}
message TodoItem_empty {}
service TodoApiService {
rpc TodoList(todo_item_empty) returns (todo_item_response) {};
rpc TodoAdd(todo_item) returns (todo_item_response) {};
rpc TodoRemove(id_params) returns (todo_item_response) {};
}
wdyt?
src/index.js
Outdated
| rtn += '}\n'; | ||
| }); | ||
| rtn += '\n'; | ||
|
|
||
| // objects | ||
| coll = doc.alps.descriptor.filter(groups); | ||
| coll.forEach(function(msg) { | ||
| rtn += `message ${msg.id} {\n`; | ||
| val = makeSnakeCase(msg.id); |
There was a problem hiding this comment.
Pascal case should be applied here as well.
There was a problem hiding this comment.
OK. will make this change.-- #thanks
There was a problem hiding this comment.
see above -- i think the snake and pascal casing routines no longer match
There was a problem hiding this comment.
Thanks for addressing this issue. The changes so far fix the syntax errors caused by identifiers with en-dashes.
I left comments suggesting that Pascal case be applied anytime the converter is emit a proto Message type. Otherwise this looks good.
The .proto generated with this input still won't compile because the RPC method signatures contain references to undefined message types; but that should probably be addressed in separate issues through some combination of refining the mappings between ALPS and ProtocolBuffer concepts and/or tweaking the ALPS input for this particular case.
|
the ALPS file you are using as your example is not valid semantically. it contains several non-reference-able href values. when you fix those, we can see if the latest changes have brokekn any proto syntax elements. you can also test this w/ the todo-alps.yaml example in the repo. that should generate valid proto files. |
|
made additional mods to this branch to force the code to generate a valid proto file that passes validation using this protogen online site: https://protogen.marcgravell.com/ right now, it's all using makePascalCase() (not ideal, but it works). please check it out and let's see what needs to be changed to makeSnakeCase in order to get the proper style and still validate/generate valid code. check out the todo-alps.yaml and todo-alps.proto files and let me know what you think. |
| @@ -185,65 +186,82 @@ function toProto(doc) { | |||
| // params | |||
| coll = doc.alps.descriptor.filter(semantic); | |||
| coll.forEach(function(msg) { | |||
| rtn += `message ${msg.id}Params {\n`; | |||
| val = makePascalCase(msg.id); | |||
| rtn += `message ${val}_params {\n`; | |||
There was a problem hiding this comment.
-- updated --
Also capture a snake-case formatted value to be used in the next hunk.
val1 = makeSnakeCase(msg.id);
| var c = 0; | ||
| c++; | ||
| rtn += ` string ${msg.id} = ${c};\n`; | ||
| rtn += ` string ${val} = ${c};\n`; |
There was a problem hiding this comment.
-- update --
This should be snake-case.
use val1 (whatever variable you used to store the snake-case ID) here.
rtn += ` string ${val1} = ${c};\n`;
| var c = 0; | ||
| msg.descriptor.forEach(function(prop) { | ||
| c++; | ||
| rtn += ` string ${prop.href} = ${c};\n`; | ||
| val2 = makePascalCase(prop.href.slice(1)); |
| rtn += `message ${msg.id}Response {\n`; | ||
| rtn += ` repeated ${msg.id} ${msg.id}Collection = 1;\n` | ||
| rtn += `message ${val1}_response {\n`; | ||
| rtn += ` repeated ${val1} ${val1}_collection = 1;\n` |
There was a problem hiding this comment.
change to
rtn += ` repeated ${val1} ${val1SnakeCase}_collection = 1;\n`-- updated --
I did some testing. My earlier suggestion introduces a bug in the name of the collection field.
When processing group descriptors, grab a snake-case version of the descriptorID to be used as the name of the collection field.
|
this PR has timed out. i'll take arnother run at this issue sometime soon. |
working to support better proto3 code styles.