Re: Pending contents in org documents

[Bruno Barbier]

Hi Ihor,

Ihor Radchenko <yantar92@posteo.net> writes:

> Bruno Barbier <brubar.cs@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