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

Add docs for overriding parser pipeline #31

Closed
ian-luca opened this issue Sep 6, 2018 · 4 comments
Closed

Add docs for overriding parser pipeline #31

ian-luca opened this issue Sep 6, 2018 · 4 comments
Assignees
Milestone

Comments

@ian-luca
Copy link

ian-luca commented Sep 6, 2018

I tried to remove Parsers from the Pipeline.

First I tried to use the RendererSettingsBuilder SetParserPipeline method, but the ParserPipeline class is internal - no luck there.

And there is no way to bypass the ParserPipeline class :(

But why even provide a SetParserPipeline method then?

@ian-luca ian-luca changed the title Not able to override ParserPipeline Not able to customise ParserPipeline Sep 6, 2018
@Romanx
Copy link
Contributor

Romanx commented Sep 8, 2018

Hi there,

Sorry the documentation around this is rough at the moment but to make sure of the semantics we use the builder pattern for creating of parser pipelines. If you wanted to remove a parser (depending on which one you're removing) you'd do:

var builder = new ParserPipelineBuilder();
builder.InlineParsers.RemoveAll(p => p is InterpolationTagParser);
var pipeline = builder.Build();

var stubble = new StubbleBuilder()
    .Configure(conf => {
        conf.SetParserPipeline(pipeline);
    })
    .Build();

The code above removes the interpolation tag parser from the pipeline. I'm looking at ways to improve the usability of this so any suggestions would be gratefully accepted.

I'll tweak this issue to request better docs for overriding the parser pipeline. Hopefully this makes sense.

@Romanx Romanx self-assigned this Sep 8, 2018
@Romanx Romanx changed the title Not able to customise ParserPipeline Add docs for overriding parser pipeline Sep 8, 2018
@ian-luca
Copy link
Author

I think it would be easier if one could instantiate the ParserPipeline with the Parsers just like the Builder does.

Sure, someone might have to refactor their code if the ParserPipeline implementation changes but it doesn’t seem that complex to me.

@Romanx
Copy link
Contributor

Romanx commented Sep 12, 2018

It would be simpler in this way I agree however the most common scenario for users is currently optimized since no parser pipeline has to be configured and we can share the pipeline across all parsers using a shared pipeline since it's stateless.

I wanted to allow more complex scenarios such as adding or removing parsers to be aided by helper methods so that advanced users who were performing these actions would fall into the "pit of success".

This unfortunately fell by the wayside for version 1.0 but with some better docs and some helper methods I think this method would be clearer since you should only be doing it once when you configure your renderer and then reusing the renderer instance in the ideal scenario.

Hopefully that clarifies some sense of my thinking.

Romanx added a commit that referenced this issue Sep 30, 2018
Also adds helper methods to the pipeline builder
Romanx added a commit that referenced this issue Sep 30, 2018
Also adds helper methods to the pipeline builder
Romanx added a commit that referenced this issue Oct 8, 2018
Also adds helper methods to the pipeline builder
@Romanx Romanx added this to the 1.1 milestone Oct 8, 2018
@Romanx
Copy link
Contributor

Romanx commented Oct 14, 2018

This was just updated and some helpers added in version 1.1. Thanks for your contribution 👍

@Romanx Romanx closed this as completed Oct 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants