Hi Ihor,
Ihor Radchenko <yanta...@posteo.net> writes:
> Bruno Barbier <brubar...@gmail.com> writes:
>
>> I added a function 'org-pending-unlock-NOW!' which unlock the region
>> immediately. The uppercase "NOW!" emphasizes that it's not the
>> "safe" way to unlock a region.
>
> I expect to see this function called by some kind of button in the
> details buffer, so that users can actually call it.
This function should not be used, not even in code, except to work
around bugs. I would prefer not to provide a button for it.
> Also, a few more small comments on the commentary section:
>
>> ;; The library makes locks visible to the user using text properties
>> ;; and/or overlays. It diplays and updates the status while the
> * displays
Thanks.
>> ;; If the user kills a buffer, or, kills Emacs, some locks may have to
>> ;; be destroyed. The library will ask the user to confirm if an
>> ;; operation requires to destroy some locks. See the field
>> ;; `before-destroy-function' of REGLOCK object, if you need to do
>> ;; something before a lock is destroyed.
>
> We should provide a user option to suppress the query. Something like
> what `confirm-kill-processes' does. Maybe even support something akin
> `process-query-on-exit-flag', but for reglocks.
IMHO, it's the package that uses org-pending that should provide such a
feature: I may not care about loosing data by ignoring on-going code
block executions, but, I'll probably care if Emacs aborts some money
transfers.
>
>> ... feel free to look at the docstrings of the
>> ;; cl-defstruct `org-pending-reglock' for the full documentation.
>
> Maybe better refer to M-x cl-describe-type org-pending-reglock. It will
> look nicer.
Thanks. I didn't know how to display that documentation actually :)
As this library is for developpers, I've used the syntax:
(cl-describe-type 'org-pending-reglock)
so that the documentation is only one click away.
>> (cl-defun org-pending (region
>> ...
>> On receiving the outcome (a :success or :failure message, sent with
>> `org-pending-send-update'), remove the region protection. Call
>> ON-OUTCOME with the reglock and the outcome, from the position from
>> where the REGLOCK was created. If ON-OUTCOME returns a region (a
>> pair (start position . end position)), use it to report the
>> success/failure using visual hints on that region. If ON-OUTCOME
>> returns nothing, don't display outcome marks.
>
> Please describe what the default value of ON-OUTCOME (when ON-OUTCOME is
> not explicitly provided) does right in the docstring.
Done.
>> You may set/update the following fields of your reglock to customize its
>> behavior:
>> - Emacs may have to destroy your locks; see the field
>> `before-destroy-function' if you wish to do something before your
>> lock is destroyed.
>> - The user may ask Emacs to cancel your lock; see the field
>> `user-cancel-function' to override the default cancel function.
>> - The user may request a description of the lock; see the the field
>> `insert-details-function' to add custom information when your
>> lock is displayed to the user.
>>
>> You may add/update your own properties to your reglock using the field
>> `properties', which is an association list.
>
> Here, we may also refer to cl-describe-type for more details about the fields.
Done.
>> ( user-cancel-function nil
>> :documentation
>> "Function called when the user wish to cancel this REGLOCK,
>> with the REGLOCK as argument. This function must return immediately; it
>> may, asynchronously, stop some processing and release resources; and,
>> once this is done, it should send the outcome to the REGLOCK (using
>> `org-pending-send-update', so that the region is unlocked and the
>> REGLOCK destroyed). The default value is
>> `org-pending--user-cancel-default'" )
>
> How come the default value is `org-pending--user-cancel-default' when it
> is nil?
The only way to build a lock is to use the function 'org-pending'; and
that function was setting it to that value. I've moved the setting
to the defstruct definition.
>> (cl-defstruct (org-pending-reglock
>> ...
>
> It would be nice to define slot types for each slot.
I added that to my todo list: I'll have to learn how to write types in
elisp (or common lisp?) to be able to do that (... the very bottom of my
list :)
)
>> insert-details-function nil
>> When non-nil, function called to insert custom details at the end of
>> ‘org-pending-describe-reglock’. The function is called with a REGLOCK,
>> a START position and an END position, it must insert details at
>> point. Assuming B is a (virtual) buffer containing all detailed human
>> readable information, insert at point details from START to END. Handle
>> cases where START, END are nil or out of bounds without raising an
>> error. The function may use text properties, overlays, etc. See
>> ‘org-pending-describe-reglock’
>
> From this description, my understanding is that the existing text
> between START and END must be replaced? Or is it something else?
No. That description says:
"it must insert details at point."
> It is
> also not clear what the custom function is supposed to do when START/END
> are nil.
>
> I looked up the docstring of `org-pending-describe-reglock', but it does
> not contain anything useful wrt setting insert-details-function slot.
Yeah ... that feature was smelling more and more like an overdesign :)
I'm dropping that feature altogether: no more start/end; problem solved
;)
For the record, the idea was to allow to insert a small view from a
10GB log; i.e. to insert *at point* in the description buffer, the
text that begins at START in that log file and ends at END in that log
file; the current default implementation would just display the first
few lines. That API would allow:
1. to configure how much is displayed,
2. to display the tail if needed,
2. and even to implement paging.
As I removed the feature, the org-pending user is now responsible to
insert at point only a reasonable amount of text.
I've pushed the changes to my repository.
Thanks again for the review.
Bruno
