Chevrotain / chevrotain

Parser Building Toolkit for JavaScript
https://chevrotain.io
Apache License 2.0
2.44k stars 200 forks source link

Allow to overwrite if token can be deleted in recovery mode #1754

Closed medihack closed 2 years ago

medihack commented 2 years ago

Fixes #1753

medihack commented 2 years ago

During playing around with the already existing tests it seems that can perform single token deletion recovery does not really test the single token deletion recovery. When you simply return false from canRecoverWithSingleTokenDeletion then the test still passes (yes, I did a rebuild). It seems more that it tests some re-sync or so. Maybe I can come up with a better one. I will also fix the typo in the filename if you are ok with that.

bd82 commented 2 years ago

I will also fix the typo in the filename if you are ok with that.

Typo fixes are always welcome, just be wary that combining source code changes and file renaming won't confuse the version history in git.

it seems that can perform single token deletion recovery does not really test the single token deletion recovery

This is a very old test (5+ years?) it is possible that some changes along the way broke some of the test's assumptions

medihack commented 2 years ago

Alright, I will fix the typo in the filename and add the return type of canTokenTypeBeInsertedInRecovery in separate PRs. Is this PR ok then? I must confess that the tests are a bit "constructed", but it does test it ;-)

bd82 commented 2 years ago

in separate PRs. Is this PR ok then?

Sure.

bd82 commented 2 years ago

I must confess that the tests are a bit "constructed", but it does test it ;-)

It is clear enough for me. 👍

Once you remove the console.log we can merge this PR

medihack commented 2 years ago

Super and done :-)