Bruno Barbier <brubar...@gmail.com> writes: > I've pushed an update that should address most of your comments.
Thanks! > I've found a better name: I'm now calling it a "lock". So I renamed > "PENREG" into "REGLOCK" as "region lock". The structure is now > `org-pending-reglock'. I slightly dislike short names and would prefer region-lock, but not a big deal - it is just my personal style preference. > I've renamed org-pending-ti-send-update to org-pending-send-update > (now that the task control is gone, the prefix becomes useless). I have a further request on interaction with penreg objects. I feel that it is not ideal that overlays associated with penreg objects cannot be fully controlled by the callers. In particular, I'd like to see some way to 1. Create penreg object without locking the region, so that scheduled-at time is not set immediately and status overlay is not displayed. Then, `org-pending-send-update' could send :schedule signal to perform actual lock. 2. Act on the outcome overlays - there is currently no way to remove them using penreg object. Maybe :cancel signal? Canceled penreg objects can then be garbage-collected from the manager. Also, the top-level commentary is getting incomplete and out-of-sync at this point. May you work towards more detailed top-level description of the library? This will make your ideas more explicit and make life easier for me during the further review (now, I have to guess what you meant by some parts of the code). >> HANLDER will be another object we may expose via something like >> (org-pending-handler (&key on-success-function on-cancel-function on-await >> on-insert-logs) ...) >> Then, PENREG will call appropriate handler function as needed. > > As the task-control is now gone: > - get/await is gone, > - cancel is now a hook/function of REGLOCK, > - insert-details is now a hook/function too of REGLOCK. >> ... >> Yes. Lexical context is implicit and harder to debug, while storing >> necessary data explicitly in the struct slots is more robust as we are >> very clear which context is intended to be captured. > > Done. Is there any reason why you hide the extra information behind :-alist filed? Why not directly adding extra fields with proper documentation? -- Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at <https://orgmode.org/>. Support Org development at <https://liberapay.com/org-mode>, or support my work at <https://liberapay.com/yantar92>
