On Jun 26, 2013, at 2:17 PM, Xikilze <notifications@github.com> wrote:

> In src/main/java/org/codehaus/jparsec/LocatableParser2.java:
> 
> > +	  private final Parser<T> parser;
> > +	  private LocatableHandler<U> handler;
> > +
> > +	  public LocatableParser2(Parser<T> parser, LocatableHandler<U> handler) {
> > +	    this.parser = parser;
> > +	    this.handler = handler;
> > +	  }
> > +
> > +	  @SuppressWarnings("unchecked")
> > +	  @Override boolean apply(ParseContext ctxt) {
> > +	    int begin = ctxt.getIndex();
> > +	    if (!parser.apply(ctxt)) {
> > +	      return false;
> > +	    }
> > +
> > +	    handler.handle((U) ctxt.result, ctxt.source.subSequence(begin, ctxt.getIndex()).toString(), begin, ctxt.getIndex());
> For the AST I use for my parsers, I always have a root object with a reference to a location object. This is what I did for the JUnit test of my proposition. It perfectly fits my needs, but of course there can be other use cases.
> 
> 

The JUnit test calls locate() on both DECL and IDENTIFIER.  I have some worry about such side-effect based design causing confusions with grammar rules like peek(). But I'm not sure how realistic it is.


> The AST object I use implements a Visitor pattern then I can iterate through the tree whithout having to deal with an external Map. This allows for instance to find a node by a cursor position.
> 
> I'm not sure the source substring image is very useful. Otherwise it can be useful to have a reference to the parsed file name. I've already implemented this with the module, I will put it in github shortly.
> 
> I believe the interceptor method could be a better solution, but it's more difficult to implement than what is done now.
> 
> 

It's been a while since I last looked at the code so I could be wrong.

It doesn't feel that difficult to me. Just add an overload parse(String, interceptor) method to Parser, and stuff a interceptor instance in ParseContext object.

Then in Parser.run(ParseContext) method, invoke the interceptor if it's set. There could be more elegant solution but the naive one seems not hard to do.

We'd certainly need to have a Location class and a Context object for the interceptor method. They can start simple.


> —
> Reply to this email directly or view it on GitHub.
>