AshlinHarris / Spinners.jl

Command line spinners in Julia with Unicode support
MIT License
13 stars 1 forks source link

Spinner won't terminate if user function has an error #44

Closed AshlinHarris closed 1 year ago

AshlinHarris commented 1 year ago
@spinner not_defined
ERROR: UndefVarError: not_defined not defined
Stacktrace:
 [1] top-level scope
   @ ~/.julia/packages/Spinners/jQ2bX/src/Spinners.jl:219
AshlinHarris commented 1 year ago

Running the user function inside a try-finally-catch block will change the scope, and I can't think of any way to export every change it might make. So, it might not be possible to address this within Spinners.jl.

Instead, it might be necessary for users to ensure that the input does not fail. After all, the @spinner macro is not intended to be run in the REPL, other than for testing. Its use case is inside of other packages, where the user function is the developer's responsibility.

AshlinHarris commented 1 year ago

One alternative is to make sure that the spinner can be interrupted by the user so that the terminal doesn't need to be closed.

AshlinHarris commented 1 year ago

Or, the spinner process could catch exceptions from the parent process.

AshlinHarris commented 1 year ago

Also, a maximum run time for the spinner could be assigned.

AshlinHarris commented 1 year ago

I can't find any way in Julia to kill the parent process from the child process using a PID, and I'm not sure how the child process can poll for changes in a system-agnostic way.

AshlinHarris commented 1 year ago

I'd like for the parent process to pass the value of id = getpid() to the spinner process when it is created. Then, in the spinner process, while true would be replaced with something analogous to checking the value of kill -0 $id. Anything like this in Julia seems to require a ::Base.Process, rather than an integer. There doesn't seem to be any inverse function to getpid() in Julia.

AshlinHarris commented 1 year ago

When the process catches an interrupt signal, the cursor is redrawn. There is still #62 to resolve.