Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unhelpful error message "Stupidedi::Exceptions::ZipperError: root node has no siblings" #124

Open
kputnam opened this issue Apr 29, 2018 · 4 comments
Labels

Comments

@kputnam
Copy link
Owner

kputnam commented Apr 29, 2018

It looks like there's a bug in the error-handling code when an ISA segment hasn't been generated yet. The problem is in builder/generation.rb#L36. Here zipper.append fails when zipper is a RootCursor (i.e., the parse tree is empty). The example code in #123 will reproduce the error.

The correct behavior would be the same behavior that results when inserting an invalid segment somewhere else in the document (after an ISA has been generated). This would be a good issue for a contributor to work on, since it will take someone on a short tour of the internals without requiring much surgery on them. It's likely fixed with a few lines at the previously mentioned location.

@irobayna
Copy link
Collaborator

Don't we still need a segment on line 36 even for Stupidedi::Zipper::RootCursor so later on critique can be called?

FailureState.mksegment(segment_tok, state)

which is define as:

 def mksegment(segment_tok, parent)
        segment_val = Values::InvalidSegmentVal.new \
          "unexpected segment", segment_tok

        new(parent.separators,
            parent.segment_dict,
            parent.instructions,
            parent.zipper.append(segment_val),
            [])
 end

and exception coming from here:

      def append(node)
        raise Exceptions::ZipperError,
          "root node has no siblings"
      end

@kputnam
Copy link
Owner Author

kputnam commented Apr 30, 2018

Yes, good point! That line should remain, but perhaps when the zipper is empty then zipper.replace or something is called instead. I haven't thought out exactly how it should work, but I think the code path taken to add an ISA segment (the constructor for InterchangeState.push?) must change the zipper in a way that doesn't use #append.

@irobayna
Copy link
Collaborator

Adding this logic to Line 36 (generation.rb)

            if zipper.class == Stupidedi::Zipper::RootCursor
              active << zipper.replace(FailureState.mksegment(segment_tok, state))
            else 
              active << zipper.append(FailureState.mksegment(segment_tok, state))
            end

and

      def mksegment(segment_tok, parent)
        segment_val = Values::InvalidSegmentVal.new \
          "unexpected segment", segment_tok

        if parent.zipper.class == Stupidedi::Zipper::RootCursor
          new(parent.separators,
              parent.segment_dict,
              parent.instructions,
              parent.zipper.replace(segment_val),
              [])
        else  
          new(parent.separators,
              parent.segment_dict,
              parent.instructions,
              parent.zipper.append(segment_val),
              [])
        end
      end

will get beyond the initial issue. Good news is that we are executing critique bad news is that
now we get this other message: root node has no parent

https://github.com/irobayna/stupidedi/blob/000400c88801adbe46bb0019c7fe042806ea1785/lib/stupidedi/builder/builder_dsl.rb#L165

@kputnam
Copy link
Owner Author

kputnam commented May 8, 2018

Hmm, that seems more complicated than I first thought, but I'll take a closer look this week. Maybe it can't be done very cleanly.

@kputnam kputnam added the defect label Sep 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants