Closed D4ryus closed 5 years ago
Hallo, thanks for the code. I will need a few days to review it. I hope you dont mind if I rename idle and with-term to clarify intent. From the name of the functions it is not clear what they do. The name with-term suggests that it somehow relates to the terminal, while it is a helper function for slime. I'd like to reserve that name for future use when addressing actual terminals from the ncurse terminfo interface. I'd also move the code to a separate file to emphasize that it is related to slime specifically. Also since the amount of code in the commit is bigger then few lines, are you OK if it is published and distributed under the MIT licence as stated in the licence file?
Hey
I hope you dont mind if I rename idle and with-term to clarify intent.
I don't, i just couldn't come up with a better name.
From the name of the functions it is not clear what they do. The name with-term suggests that it somehow relates to the terminal, while it is a helper function for slime. I'd like to reserve that name for future use when addressing actual terminals from the ncurse terminfo interface. I'd also move the code to a separate file to emphasize that it is related to slime specifically.
The idea was to have a general way to execute code inside the ncurses thread, for example it allows doing async requests without blocking the terminal thread:
(bt:make-thread
(lambda ()
;; fetch some data async without blocking the terminal ui
(let ((data (some-http-request))
(with-term
;; check if window still exists
(when (stream-open-p window)
(show-data window data))))))
Which i think is a common use case. As you can see this requires a check if the window we want to write to is still alive once the request is done. Another approach would be to make with-term (or whatever it is called) require a window argument and add the queue as a slot to the window (or a derived class). This would also remove the global queue (*term-queue*
) and allows putting a call to idle
into event-case
of the window. Which would make it just work, without additional user setup.
Hence instead of having a global queue, have one for each window. Not sure whats the better solution here.
Also since the amount of code in the commit is bigger then few lines, are you OK if it is published and distributed under the MIT licence as stated in the licence file?
Yes.
If you want i would also open a pull request which describes how to use croatoan from slime once its clear how it is going to be implemented, but iam on holiday right now so it would take a few days to write up.
Also if there is anything else you want me to change (even small things), i dont mind adjusting my commit.
I've found time to play around with your thread queue, it works great. Thanks! So yes, eventually writing that up for slime users would be nice. But no hurry there, enjoy your holiday.
As I mentioned above, I'd like to move the contents of your commit to a separate file to keep slime-related code in one place. My time is somewhat scarce right now, so if you find a chance to do it before I do during the next week, could you please do it?
My time is somewhat scarce right now, so if you find a chance to do it before I do during the next week, could you please do it?
Yes, i can do it during the weekend. Any ideas on names to not conflict with upcoming changes?
I've moved all the queue related code to queue.lisp and added your author information.
I've renamed the two main functions/macros to "submit" and "process", to resonate with the usual job queue terminology.
If you have ideas how to make the queue processing part of the event loop in a practical way, please make further commits. Thanks.
WITH-TERM uses a thread-safe FIFO queue to queue up requests which should be evaluated inside the terminal thread. For this to work one has to call IDLE from inside the terminal thread to pop requests from the FIFO and evaluate them. When a condition is signaled while the body of WITH-TERM is evaluated, it is handled by IDLE and put inside a WITH-TERM-ERROR which also contains the failed form. The condition is then signaled again from within a restart which allows skipping the failed form and continue evaluating requests.