Skip to content
This repository has been archived by the owner on Jul 26, 2018. It is now read-only.

different error approach, should be backwards compatible #3

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

onnlucky
Copy link

Hi,

Hope you like this, I've used this style of error reporting with much success in jmeta. (see the calc.g sample)

There are still minor issues with it, but if you don't use it it shouldn't be in the way... however, I've reworked quite some stuff in the guts of greg. I think I'll need to tidy that up at one point.

(typing 'aaa' in the calculator should report: expected end of line, but reports: (null))

And likely this needs api to fetch the message, the offending line, and such.

tl;dr; good start but work in progress ...

keep state and error state differently
add current line number to state (only works in files using \n as newline)
update calc sample, document it better

after you no longer allow backtracking, place @"..." signs reporting the error
see annotated sample/calc.g how this works.
@nddrylliog
Copy link
Member

That's an awesome idea :) rock would benefit a lot from using this error type instead.

Just running out of time to handle everything (argl), have you tried to regenerate https://github.com/nddrylliog/nagaqueen with it?

@onnlucky
Copy link
Author

On Mon, Jun 20, 2011 at 11:26 PM, nddrylliog
reply@reply.github.com
wrote:

That's an awesome idea :) rock would benefit a lot from using this error type instead.

Just running out of time to handle everything (argl), have you tried to regenerate nddrylliog/nagaqueen with it?

no, sorry

Actually I think I'll work on this more later this month, but likely
will break the current greg compatibility. I think it will be worth
it.

I will remove the buffer that windows over the input, lots of times
you have the complete text in memory already. When you do not you can
do your own buffering (aided by greg maybe).

I will remove the global non scoped text completely, I think it can
actually cause corruption under certain backtracking. In any case, not
doing it globally means only strdup of text we actually captured
successfully. And "consuming" the yytext means greg won't have to free
it (write null into the yytext variable). Should save a lot of malloc
traffic. (Especially without the window means only a '>' will allocate
memory.)

I will add a position (line, char) statement, that just like yytext
will be passed into rules, using the last captured position in your
rule.

Since this will break greg, I get the opportunity to introduce a real
api, instead of a bunch of need to know places you can poke at the
_GREG struct ...

not sure when I'll work on this, basically whenever I need something
from what I just described above ...

regards,
-Onne

@nddrylliog
Copy link
Member

All those ideas look good. I suggest you create another branch to work on
this, so it doesn't break for everyone right now and I can still pull your
changes to the main repo. Eventually I'll adjust nagaqueen to be compatible
with your new version :)

In this whole discussion you seem to subtly imply that greg's architecture
suck - what can I say? It's kinda true. It was a long time ago that I worked
on this, I had very little knowledge about parsers and parser generators at
the time, and piumarta/_why's code was pretty opaque to me too. I just did
what I could at the time to make it work and with better error reporting
than any greg fork I had seen so far.

So yes, architecture changes and cleanups are good, I agree with your ideas,
just don't hit me on the head too hard because I was young and foolish and
constrained by the decisions of others :)

Cheers,
Amos

On Tue, Jun 21, 2011 at 9:54 AM, onnlucky <
reply@reply.github.com>wrote:

On Mon, Jun 20, 2011 at 11:26 PM, nddrylliog
reply@reply.github.com
wrote:

That's an awesome idea :) rock would benefit a lot from using this error
type instead.

Just running out of time to handle everything (argl), have you tried to
regenerate nddrylliog/nagaqueen with it?

no, sorry

Actually I think I'll work on this more later this month, but likely
will break the current greg compatibility. I think it will be worth
it.

I will remove the buffer that windows over the input, lots of times
you have the complete text in memory already. When you do not you can
do your own buffering (aided by greg maybe).

I will remove the global non scoped text completely, I think it can
actually cause corruption under certain backtracking. In any case, not
doing it globally means only strdup of text we actually captured
successfully. And "consuming" the yytext means greg won't have to free
it (write null into the yytext variable). Should save a lot of malloc
traffic. (Especially without the window means only a '>' will allocate
memory.)

I will add a position (line, char) statement, that just like yytext
will be passed into rules, using the last captured position in your
rule.

Since this will break greg, I get the opportunity to introduce a real
api, instead of a bunch of need to know places you can poke at the
_GREG struct ...

not sure when I'll work on this, basically whenever I need something
from what I just described above ...

regards,
-Onne

Reply to this email directly or view it on GitHub:
#3 (comment)

Amos Wenger
Community Development Engineer
official.fm - "The Do It Yourself Music Club"
I'm @nddrylliog https://github.com/nddrylliog on GitHub

@onnlucky
Copy link
Author

oh no trouble, I just assumed all the cruft was from ian :) especially the whole input window/buffer/alloc

it is good you went this far, I used leg before, but abandoned that because I didn't have the heart to make it thread safe and remove all the leaks ... so thank you and _why

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants