search

Origen-SDK / o2

MIT License
4 stars 0 forks source link

Added result to processor output #95

Closed ginty closed 4 years ago

ginty commented 4 years ago

Hi @coreyeng, another AST processor update...

I've got a STIL parser now working and have moved onto processing it. When writing a real-life processor It quickly became clear that they will be able to generate errors and currently the only way to deal with those is to panic.

This update makes all processor's on_node methods return a Result<Return> instead of just a Return code.

I toyed with adding an error code to the existing Return enum, however that would mean that ? couldn't be used within the on_node implementations to bubble errors which would be inconvenient and unexpected/unusual from a coding perspective. I had a look to see if there was a result trait that could be added to Return, but I couldn't find anything like that.

The net effect of this on processors doesn't seem too bad, and basically boils down to returning something like Ok(Return::ProcessChildren) instead of Return::ProcessChildren.

coreyeng commented 4 years ago

Looks good! I had encountered the same but I figured you just wanted it to panic. I'm in favor of this though.

Spec fail looked odd too. Didn't install python 3.8. But it passed in the push. I just restarted the PR 3.8. We'll see what happens the second time but might need to add a 'redo' in the yml to account for random failures out of our control.

coreyeng commented 4 years ago

Yeah, it passed second time through... I was watching it. Really curious as to what it'd do. lol.

ginty commented 4 years ago

Cool, thanks @coreyeng