Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for_option in the protocols macros #331

Open
GuillaumeMilan opened this issue Oct 27, 2024 · 4 comments
Open

Support for_option in the protocols macros #331

GuillaumeMilan opened this issue Oct 27, 2024 · 4 comments

Comments

@GuillaumeMilan
Copy link
Contributor

Description of the feature

At the moment it is not possible to create operations you want to repeat in the protocols.
This could be handy for implementing set_cookies option to be able to set multiple cookie on the printing session.

Example:

defmodule MyApp.ProtocolExample do

  steps do
    for_options :set_cookies, ext:  :acc do
      call(:set_cookie, "Network.setCookie", &Map.fetch!(&1, :set_cookies_acc), %{httpOnly: true})

      await_response(:cookie_set, [])
    end
end

Implementation consideration

To be able to implement such a feature I am questioning myself on the following points:

  • Why steps are done through macros and not functions? While they are converted to function calls at the end
  • For the accumulator shall we save it inside the state? And if yes is there some standard to name keys (in the example I put it as :set_cookies_acc as the concatenation on set_cookies and acc as the extension.)

Also for the moment all the protocols system is not documented. Will it be part of a next iteration? (I am thinking it could be nice to make it part of a separated library)

@maltoe
Copy link
Collaborator

maltoe commented Oct 28, 2024

Hi @GuillaumeMilan

thank you for your request. I'm rather hesitant to put further complexity into ProtocolMacros, TBH, hence may I ask

This could be handy for implementing set_cookies option to be able to set multiple cookie on the printing session.

Is this an actual requirement of yours or are you just looking to contribute?


To your questions:

Why steps are done through macros and not functions? While they are converted to function calls at the end

There's probably no good reason for this. I wanted some syntax sugar and be able to write my "protocols" as almost linear code. But one could have easily implemented this in a functional way as way and come out with a similarly ok-ish looking interface.

%Protocol{}
|> Protocol.add_step(:call, &my_call_fun/2)
...

For the accumulator shall we save it inside the state? And if yes is there some standard to name keys (in the example I put it as :set_cookies_acc as the concatenation on set_cookies and acc as the extension.)

That would be the easiest, sure. I think I'd just re-use the set_cookies name as the iterator, because why not.

You have something like this in mind, right?

      defp do_build_steps([{:for_option, key} | rest], acc, opts) do
        for value <- List.wrap(Keyword.get(opts, key)) do
          do_build_steps_until_end(rest, acc, Keyword.put(opts, key, value))
        end
        |> List.flatten()
        |> ++(do_build_steps(skip_branch(rest, ...))
      end

@GuillaumeMilan
Copy link
Contributor Author

Is this an actual requirement of yours or are you just looking to contribute?

This is indeed a requirement I have in a project. But to do so I just created a custom protocol in charge of binding once a cookie. And then use the regular flow to bind the second one. My worry is as the protocol API is not documented it can be interpreted as a private API. And so might change in future version of the library.

I would also be happy to participate in the project but to be able to do so I will need to understand some of the choices made first.


There's probably no good reason for this. I wanted some syntax sugar and be able to write my "protocols" as almost linear code. But one could have easily implemented this in a functional way as way and come out with a similarly ok-ish looking interface.

Ok clear I'll try to think about it and come with a proposition that could keep the current protocols syntaxe but providing additional features

That would be the easiest, sure. I think I'd just re-use the set_cookies name as the iterator, because why not.

At first I was thinking the same, but then I though what if the user wants to use the complete opts argument along one. If we destructure inside the same name then it will not be possible. (:as can stay an option of for_option and only destructure in other place if requested)

You have something like this in mind, right?

Yes something similar

@maltoe
Copy link
Collaborator

maltoe commented Oct 28, 2024

My worry is as the protocol API is not documented it can be interpreted as a private API.

I don't intend to make it public as I don't consider it "essential" and it's not really polished either. That said, it's also unlikely that the protocol "virtual machine" is going to change anytime soon.

At first I was thinking the same, but then I though what if the user wants to use the complete opts argument along one. If we destructure inside the same name then it will not be possible. (:as can stay an option of for_option and only destructure in other place if requested)

OK, please do an :as option then (instead of ext and name interpolation), and let it default to the same key.

@GuillaumeMilan
Copy link
Contributor Author

Planning to work on this at the end of the week. Hope I can sort it out during the WE

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants