Generalize chain?

[baskerville]

The current behavior is to abort a chain of actions on the first failure.

Would it be possible to introduce the following variants of chain:

• and_then: only executes if the last executed action succeeded.
• or_else: only executes if the last executed action failed.

[shermp]

Intriguing idea. I wrote the original chain code, with the goal of being as simple and non-invasive to the current code base (at the time) as possible.

Currently, it's basically just a dumb list of function pointers, I'd have to have a bit of a think as to how it could be implemented (and parsed!)

[shermp]

Because I can't help myself, I'm prototyping this to see how it could work.

[baskerville]

I think you'd need an extra attribute in nm_menu_action_t to express whether you're dealing with and_then or or_else, (chain is probably a synonym for and_then). And then you just need to keep track, within the aforementioned loop, of the return status of the last executed action.

[shermp]

I'm actually thinking of using the terms on_success/on_failure instead. Seems to be a more explicit terminology for the desired effect. Thoughts?

[baskerville]

I'm actually thinking of using the terms on_success/on_failure instead.

I'm fine with it.

[shermp]

Got a prototype semi-working. It's going to need a bit more time in the oven before I push it.

EDIT: Hopefully, morning brain might be a bit more with it than midnight brain...

[pgaskin]

I don't know what @shermp has in mind yet, but I'd probably implement this myself by adding an on_success and on_failure bool fields nm_action_t which is only on_success by default. Then, I'd make variants of chain with chain_failure and chain_always. This has the advantage of pushing the complexity to the config parsing rather than the menu itself, while still having the best backwards-compatibility. I'm open to other ideas, though.

Another idea I'm considering is to add another option to nm_action_result_t which specifies whether to continue or abort, but I probably won't go with this. It seems simple at a first glance, but it ties a lot of unnecessary things together, which will make things harder to debug and maintain in the future.

[shermp]

Ok, I've pushed a prototype to shermp/conditional_actions. It's not extensively tested, but I haven't managed to break it FWIW. More testing can occur if @geek1011 thinks my approach is ok.

Here's the quick doc I wrote for the feature:

on_success:<action>:<arg>
on_failure:<action>:<arg>
     Adds a conditional action that is triggered when the preceeding chain or menu_item fails or succeeds.
     At most, one of each is allowed, and only one will be executed. If the on_success/on_failure succeeds,
     the next action in the chain will be triggered as normal.

The approach I took was a "jump ahead" approach. The jump distances are set during config parsing, and during the loop in menu.cc, the pointer is advanced as necessary to skip unwanted actions.

The approach means that one or both options may be set in the config, and the order of the two doesn't matter.

[pgaskin]

Thanks! I'll have a closer look at this tomorrow or the day after.

One thought: That point about the order is interesting. The advantage of your approach is the clarity, but it comes at the cost of flexibility with action chaining. If anyone else has opinions on this tradeoff, I'd like to hear them.

[shermp]

I hadn't really thought of conditional chains. My first impression of that idea is "it could get messy".

Note, with my approach, each action can have a condition attached to it if you like. Makes it very clear what condition belongs with which action.

eg:

menu_item ....
on_success ...
on_failure ...
chain ...
on_failure ...
chain ...
chain ...
on_failure ...
on_success ...

[pgaskin]

OTOH, I was thinking more along the lines of shell script && and ||, so you'd have chain for always chaining, chain_success for if the previous action (whether in a chain or not) succeeded, and chain_failure for failure. Then, the menu would loop over each item and set a bool for if it failed or succeeded. It would skip over items which don't match the current set of bools.

So, you could do something like:

menu_item : ... : cmd_output : /bin/whatever
chain_failure : cmd_output : /bin/whatever_fallback
chain_failure : cmd_output : /bin/whatever_fallback2
chain_success : dbg_msg : Something worked
chain_failure: dbg_msg : Nothing worked
chain : dbg_msg : All finished
chain : do_something_else

[pgaskin]

Does anybody else have an opinion on this?

[baskerville]

The behavior of && and || is probably what I had in mind.

[shermp]

As this is @baskerville 's request, I'm happy with whatever you guys are happy with. Whether that's my approach, or a different one is fine by me.

[pgaskin]

Ok, I'll probably go with my approach then. Thanks for working on this, though, @shermp.

[pgaskin]

@baskerville, can you test this: https://github.com/geek1011/NickelMenu/suites/672525752/artifacts/6077878?

Here's what I tested this with myself:

menu_item:main:Test:cmd_output:500:mkdir /tmp/geek1011 && echo mkdir
chain:dbg_msg:Success
chain_failure:cmd_output:500:rm -rf /tmp/geek1011 && echo rm
chain_always:dbg_toast:Done

[baskerville]

It seems to work fine, thanks.

[pgaskin]

No problem. @shermp or @NiLuJe, can you try and break it, and maybe also come up with an example?