lark-parser / Lark.js

Live port of Lark's standalone parser to Javascript
MIT License
71 stars 12 forks source link

Bug with `interactive.accepts()` #39

Open thekevinscott opened 7 months ago

thekevinscott commented 7 months ago

I believe there is a bug with the accepts() method when using interactive.

I set up a repo here demonstrating the issue.

This is the error:

Screenshot 2024-02-18 at 2 40 06 PM

I believe what is happening is that accepts is attempting to iterate with an of statement here, but it gets back an object from choices() which cannot be iterated over with of (it could be iterated over with in, which would return the object's keys).

thekevinscott commented 7 months ago

Just to provide a solution, rewriting the accepts() method to be:

  accepts() {
    let new_cursor;
    let accepts = new Set();
    const choices = this.choices();
    for (const key in choices) {
      const t = choices[key];

Seems to resolve the bug for me.

I see this method defined here and would be happy to push a PR modifying but I'm not familiar with how this library works, so lmk if you'd like help how best to help you.

erezsh commented 6 months ago

If you submit a PR with a validating test, I will merge it in.

If your offer of help extends beyond this PR, there are plenty of other tasks that need doing :)

thekevinscott commented 6 months ago

If you submit a PR with a validating test, I will merge it in.

Added here.

Tests pass, but I don't see any tests around InteractiveParser (so I don't think this change is covered by existing tests).

I can look into adding new tests for this, though if so I'll focus on this specific use case (since I'm not very familiar with the existing functionality set).

If your offer of help extends beyond this PR, there are plenty of other tasks that need doing :)

Maybe! Let's start here and see how it goes :D

erezsh commented 6 months ago

Sure, if you can, just add a test for this specific case. Thanks!

thekevinscott commented 6 months ago

Test added.