Incomplete parse is not working as expected

[igordejanovic]

Reported on the related issue #16.

This parse should succeed where the input string should be consumed incompletely:

g = Grammar.from_string('T: NL* L+ NL* | NL*; L: /.+/; NL: "\n";', re_flags=re.MULTILINE)
print(Parser(g, ws=False).parse('\nL1\nL2\n\n'))
# ParseError: Error at 3:0:"\nL1\n*L2\n\n" => Expected: STOP but found <L(L2)>

Relevant docs is here.

[amerlyq]

By the way, what about changing defaults to "always parse till the end with errors" and only by explicitly requesting allow_partial_input=True in parser() user will get continous parsing?
I feel like it's more intuitive due to incomplete parsing being not that frequent.
Or do you have your own point on using "continuous" as default?

grammar	allow_partial	behavior
S	False(default)	error
S	True	OK, partial
S EOF	False	error
S EOF	True	error

[igordejanovic]

I prefer this to be explicitly specified trough the grammar instead of introducing a new parser config flag. It's more a matter of preference but if it can be specified by the grammar I think it is always better that way. In my view using EOF in the grammar has a clean semantics.

[amerlyq]

I see. Actually it's matter of the perspective. For me "rule of least surprise" is "grammar matches whole input", which corresponds with the user goals of parsing in almost all cases. I think same is the case for others also, excluding ones raised on specialized literature exploring continuous steam parsing:) Actually, if you insist on adapting self sufficient grammar instead of parser option -- why not flip perspective out and use construct "S REST" with "S EOF" being default?) However you have problem here, as REST will consume all the rest of steam, preventing next parser in chain to kick in. Still, I don't see any problem with explicit option instead of encumbered grammar by additional EOF. Because decision if you want parse this concrete input completely or only as much as grammar can consume -- must be postponed until input actually becomes available. So same grammar could parse mixture of different grammars in data stream or single grammar in file, based on data source and business logic. In this case parser option is more logical and noninvasive, moreover you can't use EOF easily in this case at all. But, as I stated above, it's still matter of perspective.

…

On Mon, Oct 22, 2018, 20:00 Igor Dejanović ***@***.***> wrote: I prefer this to be explicitly specified trough the grammar instead of introducing a new parser config flag. It's more a matter of preference but if it can be specified by the grammar I think it is always better that way. In my view using EOF in the grammar has a clean semantics. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#64 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AFdvMHNgupARoJIxDpNToIoPfcmWGK3iks5unfmRgaJpZM4Xy48Q> .

[igordejanovic]

I understand your line of reasoning and it sound perfectly valid. The rule of least surprise greatly depends on your previous experiences I would say :)
Thanks for sharing your thinking and standpoint. When I get back to resolving this I'll reconsider different approaches again.

[amerlyq]

Heartily thanks for your time considering those suggestions. For now let's keep defaults as they are and try to fix regression at least when it bites ever again. Maybe my next predicament will be deeply entangled with stream parsing and I will find your variant more appealing, going back on my previous view:)

…

On Tue, Oct 23, 2018, 14:59 Igor Dejanović ***@***.***> wrote: I understand your line of reasoning and it sound perfectly valid. The rule of least surprise greatly depends on your previous experiences I would say :) Thanks for sharing your thinking and standpoint. When I get back to resolving this I'll reconsider different approaches again. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#64 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AFdvMGHh16NT2kmWs2cgxDwH5AwF9-lLks5unwSvgaJpZM4Xy48Q> .

[igordejanovic]

@amerlyq I've been rethinking this idea lately and I think that you are right. Several users have been sidetracked by this issue so it would definitely be better to make complete parse the default and have a parser option to allow incomplete parse. I'm planning to address this issue in the following days. Thanks for bringing this issue and discussing.

[igordejanovic]

Implementation is finished on the remove-eof branch. You can check out the tests and docs changes. Will test more in the following days and if all goes well this is going to be released in the next version.

[igordejanovic]

Everything seems fine so far. Merged to master.