Origen-SDK / o2

MIT License
4 stars 0 forks source link

Pattern headers #107

Closed coreyeng closed 4 years ago

coreyeng commented 4 years ago

Hello! This PR adds the beginnings of the pattern headers we had in O1 to O2 pattern generation. Below is an example:

image

There's still some things to work out: for example, the command is actually just the path of the pattern in the ugly 'windows-style' (in my case), but this should still be a good start (FWIW, the producer and job need some work, but, other than the command, they aren't really pertinent to this PR).

The pattern header itself I think should be fairly uncontroversial. The actual header is added here, the application can inject comments here, and the pattern itself can inject comments through the produce_pattern call.

Instead of just pushing straight strings around, I added various text nodes which can be used to build up text blocks and abstract some of the more involved attributes, like a timestamp, the executable path, or the targets. The text blocks can then be flattened by a processor, which will handle some formatting, attribute translations, and leave just a bunch of text nodes in its place which can then just be written straight by the renderer (or whatever else). So far, the only place this is used is in the pattern header, but I think this may have some more use cases in the future. I could see the release notes or email dialog making use of this.

I added a few processor return types to avoid requiring multiple passes in the flattener. Both the UnwrapWithProcessedChildren and InlineWithProcessedChildren are similar to their previous counterparts except that they eat the parent node and process the children in a single step.

If this looks good, I think the next big step would be to add something like the examples from O1 for regression tests for patterns, and eventually program, components. I could add some tests here, but I think they'd just get replaced by the 'examples' later on so I'd rather just jump to that.

Thanks!

coreyeng commented 4 years ago

As I was typing this up, I think that the pattern_header function should actually make use of decorators. I haven't messed this those much, either in creating or using them, but this might be better than a named function. Same could be said for startup and shutdown too now that I think about it.

ginty commented 4 years ago

This LGTM @coreyeng, I like the approach of putting the info in the AST to allow different formatting and/or partial consuming downstream.

Agree re. the approach of bringing in the O1 pattern and program examples, being able to replicate their output in O2 is a big milestone we should be aiming for.

I do plan to work on the prog gen once I get time btw, so I'll leave the pattern side to you.

Not sure I get the comment re. decorators? I haven't used them much either, but my mental model of them at least is that you use them to decorate functions rather than replace them, and they're probably a bit like (a less powerful version of) Rust's macros.

coreyeng commented 4 years ago

I think with decorators you can get more out of introspection, which would allow us to do things like dynamic ordering, querying, etc. We've had problems in O1 where startup methods get run and no one knows where it is (I've had to go so far as to dynamically rename them meta-programmatically so I can call them in the order I want - otherwise its a dependent on the Gemfile/gemspec ordering, which can be hard to maintain as dependencies then have their own ordering).

I'll play with them a bit when I get the chance and see what comes of it. My first thought on them is they'll give us a better way to handle callback functions and allow end user more freedom and debug capabilities, should they need it.