Condition signaling syntax simplification suggestion

Technologicat commented 5 years ago

From the example, currently the way to signal a condition is to bind the instance of the restarts context manager to some name such as call, and then use call(signal, ...).

To my reading of the code, it seems that this is the only way the signal function is intended to be invoked. This prompts the following questions:

Is there a need to have the top-level signal function exposed to the user?
Is there any other use case for the restarts instance except calling signal through it? If I understand correctly, its only purpose is to set up the internal exception handler to catch the InvokeRestart?

If not, the signaling syntax could be simplified, by making the __call__ operator act as signal. This would simplify the usage:

with restarts(use_value,
              reparse) as signal:
    return signal(MalformedLogEntryError(text))

Would we lose anything important if we did that?

(By the way, this seems a nice library, very pythonic and lispy at the same time. Elegant double inversion of what can be thought of as inner-level vs. outer-level code, essentially by callbacks within callbacks...

Debating the finer points of syntax is arguably not the Common Lisp way, but it is the Python way, to reduce friction in the user experience.

I'm interested in this, because lately, I've been borrowing selected parts of Lisp into Python. I was recently considering if it's reasonably possible to build the conditions system, and noticed that you got that one first. :) )

svetlyak40wt commented 5 years ago

Sorry, I have no time to dive into this right now. But if you feel you can make this library better, pull-requests are appreciated!

Technologicat commented 5 years ago

Ok. Thanks for the info, and the quick reply!

For the record - there was an error in what I suggested above: for maximum flexibility, we should be able to signal from anywhere, not only lexically from inside a with restarts, since at any time we may actually be inside the dynamic extent of some other with restarts.

So, the simple solution is to refactor the InvokeRestart exception catcher into signal itself, since (IIUC) the aim is to make signal return whatever the invoked restart returns. Then signal can remain public, and this part of the usage example becomes just:

with restarts(use_value,
              reparse):
    return signal(MalformedLogEntryError(text))

Let's keep the issue open for a couple more days, in case I find the time to try making this simplification.

Technologicat commented 5 years ago

Primarily to confirm my understanding, I made a minimal implementation of conditions myself. 150 lines in total, of which 51 non-blank SLOC. Interestingly the size is similar to your implementation.

I need more unit tests, but the idea seems to work as I intended. Now to port it to python-cl-conditions...

svetlyak40wt / python-cl-conditions

Condition signaling syntax simplification suggestion #4