Make context available in transformation callbacks #202
Conversation
64e298c to
2659578
Compare
|
The ocaml-ci fails aren't related to this PR. |
There was a problem hiding this comment.
It looks great! Thanks for working on this, I think it's a solid improvement to ppxlib's API.
I have a few remark and suggestions.
Also something I found is that the cram test is a bit overly complex. I wonder if we couldn't simply create a single standalone driver with a single transformation that simply inserts a floating attribute with the values of tool_name, input_name and the file_path component of the code_pathor even just dump those values on the standard output. It should obviously register impl and intf transformations. We could then invoke it on the different files you generate in your cram test! I think that would simplify the setup.
What do you think?
|
Can you rebase on top of master so we get rid of most of the CI failures? |
2659578 to
bf1a8c7
Compare
Before, via the `file_path` component of `code_path`, the user could already access the `file path` through the expansion context. This commit adds the possibility to also access the slighty different concept `input name`. Signed-off-by: Sonja Heinze <sonjaleaheinze@gmail.com>
That would indeed make it simpler. It would have a couple of downsides / difficulties:
But maybe the simplicity makes up for that. In any case, I do think that in that case I would only care about input_name and file_path though and not take the tool_name into account since it wouldn't actually add any test, would it? In fact, I think I would do that anyways. What do you think? Edit (after our conversation): Sounds all very good! Thanks! I've done that with the only difference that I've still needed two different drivers (the standard one and one for the |
ca9d23b to
8b58c15
Compare
|
@NathanReb, in the documentation of |
The later versions (V2 or V3) of the Extension and Deriving modules allow to access an expansion context when writing such rewriters. By adding a V2 to Driver, this commit also adds that possibility to whole file transformations registered through the `~impl`, `intf`, `~preprocess_impl` `~preprocess_intf`, `lint_impl` or `lint_intf` arguments of `Driver.register_transformation`. The expansion context chosen is the Base expansion context in order to be compatible with `Ast_traverse.map_with_expansion_context`. There are other kinds of transformations that still don't offer that possibility such as other context free rules than extensions. Signed-off-by: Sonja Heinze <sonjaleaheinze@gmail.com>
8b58c15 to
d45cabd
Compare
|
No hassle at all, thanks for your help! |
CHANGES: - Driver (important for bucklescript): handling binary AST's, accept any supported version as input; preserve that version (ocaml-ppx/ppxlib#205, @pitag-ha) - `-as-ppx`: take into account the `-loc-filename` argument (ocaml-ppx/ppxlib#197, @pitag-ha) - Add input name to expansion context (ocaml-ppx/ppxlib#202, @pitag-ha) - Add Driver.V2: give access to expansion context in whole file transformation callbacks of `register_transformation` (ocaml-ppx/ppxlib#202, @pitag-ha) - Driver: take `-cookie` argument into account, also when the input is a binary AST (@pitag-ha, ocaml-ppx/ppxlib#209) - `run_as_ppx_rewriter`: take into account the arguments `-loc-filename`, `apply` and `dont-apply` (ocaml-ppx/ppxlib#205, @pitag-ha) - Location.Error: add functions `raise` and `update_loc` (ocaml-ppx/ppxlib#205, @pitag-ha)
CHANGES: - Fix ppxlib.traverse declaration and make it a deriver and not a rewriter (ocaml-ppx/ppxlib#213, @NathanReb) - Driver (important for bucklescript): handling binary AST's, accept any supported version as input; preserve that version (ocaml-ppx/ppxlib#205, @pitag-ha) - `-as-ppx`: take into account the `-loc-filename` argument (ocaml-ppx/ppxlib#197, @pitag-ha) - Add input name to expansion context (ocaml-ppx/ppxlib#202, @pitag-ha) - Add Driver.V2: give access to expansion context in whole file transformation callbacks of `register_transformation` (ocaml-ppx/ppxlib#202, @pitag-ha) - Driver: take `-cookie` argument into account, also when the input is a binary AST (@pitag-ha, ocaml-ppx/ppxlib#209) - `run_as_ppx_rewriter`: take into account the arguments `-loc-filename`, `apply` and `dont-apply` (ocaml-ppx/ppxlib#205, @pitag-ha) - Location.Error: add functions `raise` and `update_loc` (ocaml-ppx/ppxlib#205, @pitag-ha)
The later versions (V2 or V3) of the Extension and Deriving modules allow to access an expansion context when writing such rewriters. By adding a V2 to Driver, this commit also adds that possibility to whole file transformations registered through the
~impl,intf,~preprocess_impl,~preprocess_intf,lint_implorlint-intfarguments ofDriver.register_transformation. The expansion context chosen is the Base expansion context in order to be compatible withAst_traverse.map_with_expansion_context.There are other kinds of transformations that still don't offer that possibility such as other context free rules than extensions (see issue #169).
The PR also adds input name to the expansion context.