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

Allow multiply output types and add new output type "ast". #303

Closed
wants to merge 2 commits into from

Conversation

Mingun
Copy link
Contributor

@Mingun Mingun commented Nov 8, 2014

Now options.output may be an array of supported output types. Output types is:

  • parser
  • source
  • ast (new)

If output types are an array, buildParser returns object whose keys are array elements, and values -- the appropriate results of generation.

@dmajda
Copy link
Contributor

dmajda commented Jun 12, 2015

I have two questions:

  1. Why would you like to add the ast output type?
  2. Why would you like to produce multiple outputs?

@Mingun
Copy link
Contributor Author

Mingun commented Jun 13, 2015

I have the command prompt utility for parsing some input just giving input and grammar. When debugging possibility of import of grammar I needed to see both AST, and the source code of the generated parser -- therefore I added opportunity for one pass to generate all necessary information.

Except that AST can it is useful in the analysis of grammar, for example when making IDE.

@atavakoli
Copy link
Contributor

If you want multiple things returned from the parse, you can construct and return an object in the starting rule action with all of the things you need (including the AST you constructed) as that object's properties.

@Mingun
Copy link
Contributor Author

Mingun commented Jun 14, 2015

If you want multiple things returned from the parse, you can construct and return an object in the starting rule action with all of the things you need (including the AST you constructed) as that object's properties.

For this purpose the grammar shall be in a special way prepared that is impossible in case of foreign grammars. Besides, unless any reasons for obvious feature are required? If it not evolution, what then?

@dmajda
Copy link
Contributor

dmajda commented Jun 17, 2015

@Mingun The use case you describe is valid, but it’s developer-centric (as opposed to user-centric) and not common enough to warrant complicating the public API.

The API extensions you propose also don’t feel right to me for two reasons:

  1. They expose what a developer-level API (the AST format) in user-level function (PEG.buildParser). I think it’s better to have it somewhat hidden and accessible only via plugins.
  2. They replace a simple input → output workflow with something more complicated without a significant gain.

It’s fairly easy to accomplish your goals without the extensions:

  • You can teach PEG.js to produce the AST as an output by writing a simple plugin which would replace code generation compiler passes with something close to a no-op.
  • Similarly, you can write a plugin to produce multiple outputs when desired. Or simply run PEG.buildParser multiple times.

Given all the above I’m closing the PR.

@dmajda dmajda closed this Jun 17, 2015
@Mingun
Copy link
Contributor Author

Mingun commented Jun 17, 2015

They expose what a developer-level API (the AST format) in user-level function (PEG.buildParser). I think it’s better to have it somewhat hidden and accessible only via plugins.

Hmm... but users of PEGjs is developers, so, developer-level === user-level, are not it?

It’s fairly easy to accomplish your goals without the extensions:

Yes, it is possible, but users which the developers using PEGjs for the analysis of grammar will find it less convenient than if this function was realized directly in library.

@dmajda
Copy link
Contributor

dmajda commented Jun 17, 2015

Hmm... but users of PEGjs is developers, so, developer-level === user-level, are not it?

By user, I mean someone who wants to use PEG.js to write a grammar, generate a parser, and mostly forget about it. By developer, I mean someone who wants to use it in more advanced ways or to extend it.

Needs of these two groups are very different. And there are much more users than developers.

It’s fairly easy to accomplish your goals without the extensions:

Yes, it is possible, but users which the developers using PEGjs for the analysis of grammar will find it less convenient than if this function was realized directly in library.

Yes, but this has to be balanced with simplicity for users.

@Mingun
Copy link
Contributor Author

Mingun commented Jun 17, 2015

By user, I mean someone who wants to use PEG.js to write a grammar, generate a parser, and mostly forget about it.

In that case enough cli utility will be it, they at all won't invoke PEG.buildParser, or perhaps even won't know at all that it is a js-script.

Yes, but this has to be balanced with simplicity for users.

How extension of option will affect complexity for users? You say that it is necessary for them only generate parser, and it doesn't require supply any options and doesn't even call PEG.buildParser because they will be use cli utility.

By developer, I mean someone who wants to use it in more advanced ways or to extend it.

I think, 'more advanced way' will often demand access to AST, and for perfomance -- opportunity to receive as much as possible information for one call. For example, I have grammars, building parser on which takes 30 seconds and more. So, repeated calls to PEG.buildParser for different kinds of output thus are unacceptable, and creation of a special plug-in only complicates use, but doesn't simplify it.

I agree that uncommon things, like support of a new language for actions or a new type of a output, have to be implemented in plug-ins, but useful simple things which are already implemented, but have no public simple API, must be provided by the library, I think.

@dmajda
Copy link
Contributor

dmajda commented Jul 3, 2015

By user, I mean someone who wants to use PEG.js to write a grammar, generate a parser, and mostly forget about it.

In that case enough cli utility will be it, they at all won't invoke PEG.buildParser, or perhaps even won't know at all that it is a js-script.

Judging by incoming e-mails, issues, etc. many users use the JavaScript API.

Yes, but this has to be balanced with simplicity for users.

How extension of option will affect complexity for users? You say that it is necessary for them only generate parser, and it doesn't require supply any options and doesn't even call PEG.buildParser because they will be use cli utility.

It would complicate the documentation (which users pretty much have to read) and make the API surface bigger (think of a content of a tooltip offered by some IDE for PEG.buildParser arguments — assuming the IDE has e.g. a solid TypeScript PEG.js API definition available).

Also, I try to make the CLI functionally equivalent to the JavaScript API so at least the ast output type would have to be supported there (multiple outputs probably wouldn’t make sense).

For example, I have grammars, building parser on which takes 30 seconds and more. So, repeated calls to PEG.buildParser for different kinds of output thus are unacceptable, and creation of a special plug-in only complicates use, but doesn't simplify it.

Performance problems of this nature isn’t a reason to change the API but to optimize the generator performance.

I agree that uncommon things, like support of a new language for actions or a new type of a output, have to be implemented in plug-ins, but useful simple things which are already implemented, but have no public simple API, must be provided by the library, I think.

And I disagree :-)

(This is my last comment on this topic.)

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

Successfully merging this pull request may close these issues.

None yet

3 participants