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

neotoma:create_transform declared in wrong order #15

Open
csurface opened this issue Jul 20, 2012 · 10 comments
Open

neotoma:create_transform declared in wrong order #15

csurface opened this issue Jul 20, 2012 · 10 comments

Comments

@csurface
Copy link

I noticed that the transform_module option wasn't working for me. After debugging, I found that the create_transform function is declared in the wrong order. The catch-all clause "create_transform(,,[])" is declared first, which is causing the transform_module option to be ignored. The fix is to move this clause to the end of the declaration.

My use case does not call neotoma from an escript. Instead I am calling directly to neotoma:file/2. Running erlang 15B on Mac OSX Snow Leopard.

@seancribbs
Copy link
Owner

@csurface I realize this issue is really stale, do you still use the transform_module option? I'm considering removing it.

@dmitriid
Copy link

I believe transform_module is useful if you want to have a clean and readable grammar description that's not littered with the actual parsing logic

@csurface
Copy link
Author

I'm not using neotoma currently, but I can share my usecase if that helps. I experimented with neotoma to create a SIP request parser. The entrypoint is:

erl -pa ebin -pa deps/neotoma/ebin -noshell -eval 'neotoma:file("priv/${MOD}.peg", [{output, "src/"}, {transform_module, sip_transform}])' -s erlang halt

My sip_transform module declares transform function clauses for SIP header and intermediate value types. For example:

transform('Request', Node, _) ->
{request, proplists:get_value(request_line, Node), proplists:get_value(headers, Node), proplists:get_value(body, Node)};

transform('Response', Node, _) ->
{response, proplists:get_value(status_line, Node), proplists:get_value(headers, Node), proplists:get_value(body, Node)};

At the time, I felt that using transform_module was the most convenient way to solve the problem.

thanks,
Charlie

@dmitriid
Copy link

My (hopefully upcoming) case would be to use definitions from https://github.com/for-GET/core-pegjs to create Erlang parsers for a small project. This means having no code in the grammar, and all logic in an external module.

On the other hand I haven't looked at the generated AST too closely, so it might be enough to just work with the AST that Neotoma produces :)

@csurface
Copy link
Author

In my case, it was cleaner to have the generated parsing code separate from the "rendering" code in the transform module. For some of the more complex SIP constructs, I had to play with several variations of erlang term shape. It was easier to do that separate from the actual parsing. I also needed to create helper functions which, IIRC, would have been blown away after each re-generation of the parsing file.

@seancribbs
Copy link
Owner

Ok, second question. What if the transform module/function target was specified via a grammar directive, rather than via a compile option?

@dmitriid
Copy link

I'm undecided on this :)

However, I think it opens a possibility to implement "append/include other peg files" in a syntactically consistent manner.

That is, if we use, for example a hack employed here, we could have something like

%% @implemetation module.erl
%% @append another_grammar.peg

@seancribbs
Copy link
Owner

As a counter-example, yecc requires you to construct terms from matching rules. One might also argue that having too much complicated logic in your transform is a code smell, and that an AST should be emitted that is later traversed and manipulated.

That said, I'm hopeful some sort of directive could solve most cases where people are using transform_module currently.

@csurface
Copy link
Author

Having a directive seems like a reasonable option. I'd be willing to test options if and when you decide to make any changes.

@seancribbs
Copy link
Owner

I'm currently teasing apart the meta-grammar syntax from the semantic analysis and code-generation. Most of the semantic analysis is done; once I have a bunch of tests for it I'll be reworking code-generation. You can see the WIP on the 2.0-refactor branch.

This refactor should allow me to address both #9 and #14 as well.

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