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

Expose Utils.Ast_io. #268

Closed
arozovyk opened this issue Jul 7, 2021 · 6 comments
Closed

Expose Utils.Ast_io. #268

arozovyk opened this issue Jul 7, 2021 · 6 comments

Comments

@arozovyk
Copy link
Contributor

arozovyk commented Jul 7, 2021

Hi, i'm currently working on AST exploration features for VSCode extension: vscode-ocaml-platform, and i would like to use the Utils.Ast_io.read function in order to get the tranformed AST value from .pp.ml. (which i do locally this way: link).

Would it be possible to expose this module ?

@pitag-ha
Copy link
Member

Hi @arozovyk, the AST exploration feature for VSCode is super cool, thanks for working on that!

For me it's ok to expose Utils.Ast_io. But that module is currently a bit ppxlib-internals specific. I'm wondering if we should expose that whole module as is or if it would be enough to expose certain parts that are less specific. From what I understand, you have a binary and need to

  1. consume the magic number as in read_magic. But you already know, unless something is wrong, it will be the impl magic number from the current compiler.
  2. consume the input name as done here. You also don't seem to need the input name, at least not currently, but you need to consume what's in the channel before the AST.
  3. read the AST and convert it to the ppxlib-AST as done here

Is that what you need?

@arozovyk
Copy link
Contributor Author

Hi, @pitag-ha, thanks for replying!

Your comment made me remember that I wanted to add support for Inft and not only Impl as I showed here.

So basically i will need the whole ast field as you can see here.

And i feel like everything that i need (modulo stuff that you mentionned i.e. input_name) is done by from_channel
So exposing something like :

 let read_bin fn = 
        In_channel.with_file fn ~f:(from_channel ~input_kind:Necessarily_binary ) 

Plus Intf_or_impl module should do the job.

If that's not an option for some reason, maybe simplify the from_channel function, something along the lines of:

match read_magic ch with Error s ->  Error (*?*)
        | Ok s -> (
            match Find_version.from_magic s with
            | Intf (module Input_version : OCaml_version) ->
                let _ : string = input_value ch in
                let ast = input_value ch in
                let module Input_to_ppxlib = Convert (Input_version) (Js) in
                Intf_or_impl.Intf (Input_to_ppxlib.copy_signature ast) 
            | Impl (module Input_version : OCaml_version) ->
                let _ : string = input_value ch in
                let ast = input_value ch in
                let module Input_to_ppxlib = Convert (Input_version) (Js) in
                Intf_or_impl.Impl (Input_to_ppxlib.copy_structure ast)
            | Unknown -> Error (*?*)

can be done?

Please tell me what do you think.

@pitag-ha
Copy link
Member

So exposing something like :

let read_bin fn = 
      In_channel.with_file fn ~f:(from_channel ~input_kind:Necessarily_binary ) 

Yes, that sounds like a good idea to me.

Plus Intf_or_impl module should do the job.

Do you mean the whole Intf_and_impl module or Intf_and_impl.t?

@pitag-ha
Copy link
Member

Do you want to implement that and open a PR?

@smorimoto
Copy link

I think we can close this now.

@pitag-ha
Copy link
Member

I think we can close this now.

Indeed. Thanks!

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

3 participants