pgaskin / NickelMenu

The easiest way to launch scripts, change settings, and run actions on Kobo e-readers.
https://pgaskin.net/NickelMenu
MIT License
591 stars 32 forks source link

Introduce a 'cmd_wait' action #9

Closed shermp closed 4 years ago

shermp commented 4 years ago

I was thinking it might be useful to have a cmd_wait (or whatever it needs to be called) action. I'm thinking such an action would behave like cmd_output, except not output a message to the user to dismiss.

Use cases might be waiting for a process to come up that was launched via cmd_spawn, or introducing a delay between chain items etc.

The quck'n'dirty way to do this would be to copy the cmd_output function, rename it, remove the "output" portion, and call it a day.

The proper way to do it would be to factor out the common code to a separate function, but then I run into the same headscratcher as @NiLuJe found, namely, how to work with the whole macro return stuff...

pgaskin commented 4 years ago

I'd probably do this by adding a quiet option to cmd_output, as I'm trying to limit the number of top-level actions.

For the macro return stuff, it's actually pretty simple (basically, think Go error handling, which is what it was inspired by since I'm not a fan of C's multitude of error-codes, and we don't have enough need for efficiency to need to avoid putting mallocs everywhere, plus we handle almost all errors anyways and will generate a message for it somewhere, so it might as well be in the original function). Basically, you either return the actual value or an error. For an error, the actual return value doesn't matter (it's only defined at the start for convenience, but it doesn't have to be), and you need to set *err_out to a malloc'd string if it is a valid pointer. On success, you return whatever value, but you set *err_out to null if it's valid. In other words, I can easily add another macro which passes an error up as-is or wraps it with another message.

NiLuJe commented 4 years ago

@geek1011: You've piqued my interest with that final comment ^^.

What do you mean, exactly by "error" and "message"? My main issue was mostly around how to deal with the message part of the equation. Ideally, I'd want "message" to be a fmt, ... varargs list in order not to have to format the string first, allocate it somewhere, and pass a pointer to that... (Although, formatting the string first has the advantage of keeping FILENAME accurate).

Otherwise, assuming you meant "an int" for "error", passing that up is essentially what I ended up doing to workaround that, so, yeah, that works, too ;).

pgaskin commented 4 years ago

For the message, I was thinking we could just free/reformat it for each level, as we don't really care about the absolute best performance.

Also, on a side note, I just discovered the __attribute__((cleanup)), which is quite similar to Go's defer statement. Every major compiler supports it except for MSVC. This would probably be useful for freeing checked errors.

shermp commented 4 years ago

I'd probably do this by adding a quiet option to cmd_output, as I'm trying to limit the number of top-level actions.

I'm fine with that. The only reason for suggesting the different action name was I wasn't sure if you were willing to add a new option to cmd_option

NiLuJe commented 4 years ago

For the message, I was thinking we could just free/reformat it for each level, as we don't really care about the absolute best performance.

True, that would work ;).

Also, on a side note, I just discovered the __attribute__((cleanup)), which is quite similar to Go's defer statement. Every major compiler supports it except for MSVC. This would probably be useful for freeing checked errors.

Yeah, I've read a few things about it. IIRC, systemd has extensive helpers to make use of it across the codebase. Haven't actually looked at it, though, but the concept certainly looks fun/interesting ;).

pgaskin commented 4 years ago

I'm fine with that.

OK, I'll probably do it that way for v0.1.0 (or maybe v0.0.2).

pgaskin commented 4 years ago

@shermp, @baskerville, can you try this: https://github.com/geek1011/NickelMenu/suites/677080392/artifacts/6194955?

I've done some preliminary testing with the following entries:

menu_item:main:cmd1:cmd_output:5000:sleep 2; uname -a
chain:dbg_msg:done
menu_item:main:cmd2:cmd_output:5000 : quiet : sleep 2; uname -a
chain:dbg_msg:done

P.S. One use I can think of for this option, once I merge #11, is to make the example telnet/ftp actions into a toggle (by checking the result of pkill as a quiet cmd_output).

shermp commented 4 years ago

Seemed to work fine adding a delay between launching KU and the web browser. Sorry I haven't been active much over the past couple of days. Work takes priority over play I'm afraid 😢

baskerville commented 4 years ago

Seems to work fine.