aj-foster / open-api-generator

Open API code generator for Elixir
MIT License
97 stars 13 forks source link

Convert function argument from camel case into underscore case #24

Closed wingyplus closed 1 year ago

wingyplus commented 1 year ago

Grab some demo from LINE SDK for Elixir:

diff --git a/lib/line/messaging_api/messaging_api.ex b/lib/line/messaging_api/messaging_api.ex
index d1ad297..d9c8afa 100644
--- a/lib/line/messaging_api/messaging_api.ex
+++ b/lib/line/messaging_api/messaging_api.ex
@@ -141,11 +141,11 @@ defmodule LINE.MessagingAPI.MessagingApi do

   """
   @spec delete_rich_menu(String.t(), keyword) :: :ok | :error
-  def delete_rich_menu(richMenuId, opts \\ []) do
+  def delete_rich_menu(rich_menu_id, opts \\ []) do
     client = opts[:client] || @default_client

     client.request(%{
-      args: [richMenuId: richMenuId],
+      args: [richMenuId: rich_menu_id],
       call: {LINE.MessagingAPI.MessagingApi, :delete_rich_menu},
       url: "/v2/bot/richmenu/#{richMenuId}",
       method: :delete,

The pending issue is string interpolation in URL path params that use regular expressions to settle this. I quite doubt how to convert that value (probably it's already midnight in my time zone).

I would like to open this to discuss and collect the feedback before moving forward. :)

wingyplus commented 1 year ago

Or we may use Regex.scan to scan path param, convert it to underscore case, replace with proper varname and convert {} into string interpolation?

aj-foster commented 1 year ago

Hi @wingyplus thanks for opening this. I agree it would be nice to underscore these, and it would be great to do this consistently (including the keys of the args list, for example). I wonder if it might be easier to make this change in the Operation module when we first process the path params. That might be a good time to change the URL as well. What do you think?

wingyplus commented 1 year ago

Hi @wingyplus thanks for opening this. I agree it would be nice to underscore these, and it would be great to do this consistently (including the keys of the args list, for example). I wonder if it might be easier to make this change in the Operation module when we first process the path params. That might be a good time to change the URL as well. What do you think?

I see, so we need to modify https://github.com/aj-foster/open-api-generator/blob/main/lib/open_api/generator/operation.ex#L135 and to add underscore case to the tuple and then replacing path at https://github.com/aj-foster/open-api-generator/blob/main/lib/open_api/generator/operation.ex#L30?

aj-foster commented 1 year ago

That looks right to me. So the path_params function could return a tuple with the parameters and an updated path for the caller.

(By the way, I just merged #23 so that you hopefully won't have merge conflicts if you make these changes.)

wingyplus commented 1 year ago

@aj-foster Just update the patch. Please kindly review. 🙏

aj-foster commented 1 year ago

Hi @wingyplus I just pushed a quick change that makes the keys of the args keyword list also underscored. As a side effect of this, we're able to "hide" the fact that we modified the param name. Does this work for your use case?

I also realized that query params in should also be normalized in this way, but I can make that a separate PR.

wingyplus commented 1 year ago

Hi @wingyplus I just pushed a quick change that makes the keys of the args keyword list also underscored. As a side effect of this, we're able to "hide" the fact that we modified the param name. Does this work for your use case?

I also realized that query params in should also be normalized in this way, but I can make that a separate PR.

Agreed.

wingyplus commented 1 year ago

The solution is look nicer now. :) I will come back testing it in a few hours. 🙇‍♂️

wingyplus commented 1 year ago

@aj-foster Sorry for the late response. Tested and its work. :)