fictiveworks / CalyxSharp

Generative text processing for C# and Unity applications
Other
0 stars 0 forks source link

25 NullRefException in CollectAtoms if Expansion(Exp e, string) called with e != Exp.Atom #31

Closed bentorkington closed 1 year ago

bentorkington commented 1 year ago

This removes the possibility of creating an Expression object with an invalid state that would crash CollectAtoms()

This refactor shook up a lot of the tests, as they were inspecting the internals directly. These tests have been replaced by object equality tests where appropriate which seem a little cleaner.

Non-atom Expression types include a convenience initializer which allows the expression and the atom it contains be initialized in one call.

bentorkington commented 1 year ago

I'd like to expand on where I'm going with this, just a bit tired now. We can go over it with a chat or I'll get to annotating the commits tomorrow.

bentorkington commented 1 year ago

I'm not happy with how this turned out, and want to have a second go at it

maetl commented 1 year ago

I'm not happy with how this turned out, and want to have a second go at it

I’ve also been thinking about this a bit from the perspective of the result tree structure being more user-centric. Embedding an interactive render of the tree in the interactive examples has been quite eye opening in terms of reinforcing the need for it to be a bit better organised.

image
bentorkington commented 1 year ago

Just before putting in this PR I'd been having a play with Calyx(JS) on a side project, and made a Pug template to break the expression down to help me understand it:

mixin parseExpression(exp)
  if (Symbol.keyFor(exp[0]) === 'concat')
    span.calyx-concat.calyx
      each x in exp[1]
        +parseExpression(x)
  else if (Symbol.keyFor(exp[0]) === 'atom')
    span.calyx-atom.calyx= exp[1]
  else if (Symbol.keyFor(exp[0]) === 'choice')
    span.calyx-choice.calyx
      +parseExpression(exp[1])
  else 
    span.calyx-other.calyx= Symbol.keyFor(exp[0])
      +parseExpression(exp[1])

html 
  head 
    style 
      | .calyx { margin: 2; padding: 2; border: 1px solid black;}
      //- | .calyx-choice::before { content: 'choice' }
      //- | .calyx-concat::before { content: 'concat' }
      //- | .calyx-other::before { content: 'other' }
      | .calyx-atom { color: red; backround: yellow; }
  body 
    span.calyx-start
      +parseExpression(x)

I helped me understand the resulting expression tree better than using the debugger