stupidedi icon indicating copy to clipboard operation
stupidedi copied to clipboard

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

Open kputnam opened this issue 6 years ago • 4 comments

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.

kputnam avatar Apr 29 '18 22:04 kputnam

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

irobayna avatar Apr 30 '18 02:04 irobayna

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.

kputnam avatar Apr 30 '18 03:04 kputnam

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

irobayna avatar Apr 30 '18 20:04 irobayna

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 avatar May 08 '18 17:05 kputnam