Closed oflatt closed 9 months ago
Good find! I fixed the infinite loop. This PR unfortunately includes a small s-expression parser.
Why do you need an sexp parser? seems like splitting on \n\n would work?
Why do you need an sexp parser? seems like splitting on \n\n would work?
That's a good point, I'll just make that change!
Otherwise I love the idea! Makes for super simple presentations.
I remember now- for the herbie demo, the code is too dense to add an extra newline in-between each command. It works a lot better to split by s-expression.
Some other heuristic could work, but I couldn't think of one. I want it to also preserve the comments and only show them with the annotated s-expression
I'm a little spooked by the sexp parser. Perhaps we could choose a different separator then? Something like \n\n\n
or \n\n;;
? Or even ;;;;
or something? The sexp parser will be brittle to parens in other places probably, right?
Yep, the sexp parser is very evil So three newlines makes my problem worse- the code has even more space in between Having a comment that separates every "slide" could work, kind of ugly though
Wait maybe i'm misunderstanding. You want to build in each top-level command individually? That seems like a bit much, you probably want to define rewrites in a big group, for example. Seems like \n\n
or even 3 should be fine, it will just reduce the number of slides you have.
Edit: yes, I think introducing each line works best for the herbie demo
Ok! Sounds good then!
I'll merge this as-is then? Could I get an approval @mwillsey?
What will this do for parens in comments?
On Wed, Nov 29, 2023 at 3:44 PM Oliver Flatt @.***> wrote:
I'll merge this as-is then? Could I get an approval @mwillsey https://github.com/mwillsey?
— Reply to this email directly, view it on GitHub https://github.com/egraphs-good/egglog/pull/302#issuecomment-1832874303, or unsubscribe https://github.com/notifications/unsubscribe-auth/AANTPTD6LAMMIWB6TMCEXJLYG7CEXAVCNFSM6AAAAAA74QWQQWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMZSHA3TIMZQGM . You are receiving this because you were mentioned.Message ID: @.***>
It will treat the comment as a slide I can make it skip comments though!
Ok as long as it doesn’t break! Sounds good to me!
This kind of terrible javascript adds a presentation mode to the egglog demo @saulshanabrook would be the best person to help me improve this