Re: Pending contents in org documents (Re: Asynchronous blocks for everything (was Re: ...))

[Bruno Barbier]

Hi Ihor,

Thanks for the review.

I've pushed a new version, hoping to decrease the number of dislikes
;-)

Ihor Radchenko <yantar92@posteo.net> writes:

> Bruno Barbier <brubar.cs@gmail.com> writes:
>
>>> 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.
>>
>> I'm trying to limit the public API surface.  I don't think we should
>> leak that we are currently using a mix of overlays and text
>> properties.
>
> Let me rephrase my concern - I do not like that after reglock is no
> longer live (got success/failure signal), there is no way to clean up the
> visual hints associated with this particular reglock.
[....]

For the org-pending library, "live" means "locked".  Once the outcome
is known, it can't be "live" anymore (it's unlocked); as it's not
reusable, it's "dead".

As the region is not locked anymore, the lock properties/fields can't
be trusted anymore.

But see below about removing the visual outcome hints of a given
reglock.

>>> 2. Act on the outcome overlays - there is currently no way to remove
>>>    them using penreg object.
>>
>> I've added a funcion `org-pending-delete-outcome-marks' to manually
>> delete outcome marks that are in a given region.
>>
>> Else, everything is handled automatically. Once the outcome is known,
>> the reglock is dead (not live-p).  org-pending may leave outcome marks
>> about the outcomes (outcome marks are optional).  The outcome marks
>> automatically disappear if the user remove the section, or, if a new
>> lock is created for the same region.
>
> I do not like this.
> I'd like the Elisp program that creates the reglock to be able to
> clean up any visual hints associated with it. > A function doing it for a
> given region cannot do this AFAIU.

ok. I've added the function `org-pending-reglock-delete-outcome-marks,
that will delete the outcome visual hints for a given reglock, if
there are some.

I updated how the lock is described to the user
(org-pending-describe-reglock): I added a button "Forget" (if the lock
is dead, that removes the outcome marks), and I added a "Cancel"
button if the lock is still live.


>>>    Maybe :cancel signal? Canceled penreg
>>>    objects can then be garbage-collected from the manager.
>>
>> Cancel is handled by sending a failure message (see
>>  `org-pending-cancel').  It's customizable using the reglock field
>>  ~org-pending-reglock-user-cancel-function~, which can decide what to
>>  do (like kill a process) and which can send a better outcome.
>>  Standard 'cancel' leaves a failure outcome mark.
>
> Note that this function is not documented anywhere other than in reglock
> class documentation.

Thanks. I've improved the documentation of `org-pending' to mention
that one may want to customize the following fields of a reglock:
before-kill-function, user-cancel-function and
insert-details-function.  And, also, I added that one can attach
custom properties using the "properties" field.

> In general, I am confused about your overall design
> of the user interaction with the locks.
> The updated top commentary explains well how Elisp programs can send
> data to the locks, but it does not say anything about how Elisp programs
> can receive the data.

An elisp program, that uses org-pending, must update the locks using
`org-pending-send-update'.  That program does not receive any data
from the lock; it may customize Emacs behavior using the reglock
fields mentioned above: before-kill-function, user-cancel-function and
insert-details-function.

Hopefully, it's clearer now with the improved documentation of the
org-pending function.

Just let me know if you still think that the top commentary should
explain this.  Thanks.


> Also, I'd like to see more information in the top commentary about what
> are the "visual hints" displayed to the user and how to configure them.

If you think the current "visual hints" are good enough and could be
shipped as-is, in a first version (indirect buffers, etc.); I could
work on documenting them better.  What kind of configuration are you
thinking about ? just the faces ? or more advanced configurations ?


[...]

>>> Is there any reason why you hide the extra information behind :-alist
>>> filed? Why not directly adding extra fields with proper documentation?
>>
>> To hide them, indeed :)
>
>> The API for 'get-status and 'get-live-p are
>> `org-pending-reglock-status' and `org-pending-reglock-live-p' (they
>> are read-only).  The API for the new `useless-p' is
>> `org-pending-reglock-useless-p' (it's read-only too).
>
> We usually "hide" fields by declaring them private.
> Hiding them from the type docs is not a good idea because it defeats the
> purpose of type documentation in general.
>
>> The fields anchor-ovl, region-ovl, on-outcome, set-status and
>> creation-point are the dump of the closure context, so that
>> org-pending doesn't rely anymore on a closure to handle updates; I've
>> rewritten that recently.  Nobody is supposed to use or change those
>> values, except the update process.
>>
>> IMHO, dumping those as fields in the lock structure would be more
>> confusing and fragile than keeping those out of sight.  I could add
>> comments when they are created/used in the code to help understand how
>> they are used.
>
> I disagree. In particular, I dislike the fact that they are not
> documented anywhere and one has to read the internals of the code to
> understand their purpose.

Done.  I hope the minimal documentation is enough.

Thanks again for your reviews and your comments,


Bruno