Hi Ihor,

Ihor Radchenko <yanta...@posteo.net> writes:

> Bruno Barbier <brubar...@gmail.com> writes:
>
> I have one suggestion though. You now do
>
>     Use the function ON-OUTCOME to update the region with the outcome; if it
>     is nil, set it to the function `org-pending-on-outcome-replace'.
>
> However, `org-pending' is defined via `cl-defun', so you can instead
> just put the default value for :on-outcome key and mention that it is
> the default in the docstring. Then, you do not need to do any additional
> checks in the function body; `cl-defun' will take care about assigning
> the default value.

Done.


> I tried to run your example and have several observations:
>
> 1. On failure, it is not obvious that failure happened:
>    - The failure overlay disappear very quickly, and is not visible at
>       all if I happen to look elsewhere in the buffer. Maybe we can
>       simply keep it and remove the overlay on click

The current method was to 'sit-for' 0.2 seconds; the new default is to
do nothing (the overlay disappears immediately), but it's now
configurable, see below.


>    - After failure, the "!" fringe indicator is visible, but it is not
>      obvious at all that user can click to get details
>      I first tried to click on the fringe itself to no avail. Then, I
>      randomly clicked on the text and got the description buffer; but
>      that was unexpected - the text I clicked did not have any
>      indication of its "clickability" - neither some kind of underline
>      face, nor an overlay or a mouse hint.

I couldn't find a way to answer a click on a fringe.  Is there a way?

About how much to decorate, it depends on the user, I guess.  For
example, when org-pending is used for org babel, it should be obvious
that one has to click on "#+RESULTS:".

The current decoration is not the best for discoverability, indeed.
But decorating the whole outcome region would be too much IMHO, and,
it might interfer with other buffer fontifications.

I've refactored the code and added two variables so that it's now
configurable, see 'org-pending-outcome-pre-display-function' and
'org-pending-outcome-post-display-function'.

I'll be happy to commit a patch that provide better defaults.

> 2. I tried to do M-x undo while the reglock was active, and got an
>    error. I'd expect that undo would work, skipping the region.

I'm not sure exactly what you did, nor which error you got.  I've
noticed that the hacks (to handle indirect buffers) were flagging the
buffer as "modified" (using text properties).  I've fixed that.

Is the problem solved now ?

>
> 3. I tried M-x undo _after_ reglock was unlocked, and I got "TO REPLACE"
>    word highlighted. I did not expect it to be highlighted.

I couldn't get that behavior, but the undo wasn't correct either.

org-pending is now directly instrumenting the buffer-undo-list, and
manually adding an undo-boundary.

Do you still see your problem ?


> 4. If I try to cancel the reglock, it does get canceled, but *Region
>    Lock* buffer is not updated - Live? still displays "yes Cancel".

It's by design.

The function `org-pending-describe-reglock' works like
`describe-variable' and other similar functions.  You need to revert
the buffer (g) to update it.

The reglock is live in its buffer.


>>> What is the difference between "canceling" and "killing" the reglock?
>>> Do they need to be separate?
>>
>> If you cut out, from the example, the part where they differ, they do
>> look the same indeed :)
>>
>> I'm apparently failing to explain and document this correctly, as it
>> looks like a recurring topic, sorry.
>>
>> Yes, they need to be separate as they are two different operations.
>>
>>  - cancel: The *user* may request a *cancel*; it's a polite way to
>>    tell org-pending that the user doesn't care anymore about the
>>    outcome.  A valid implementation is to ignore the user request.
>>    The default implementation is to unlock the region (sending a
>>    cancel :failure using 'org-pending-send-update'): it unlocks the
>>    region, ignoring why it was locked..
>>
>>  - kill: *Emacs* may have to *kill* some locks, because Emacs is
>>    killed, or the lock buffer is killed.  org-pending will intercept
>>    the operations of this kind, ask the user to confirm the
>>    destruction, and, if confirmed, it will give a chance to the lock
>>    to do some cleanup by calling the 'before-kill-function'.
>
> Does it mean that clicking "cancel" does not guarantee that the region
> will not be updated by the running process/timer?

Yes, org-pending does not enforce that; and it should not, else it
would forbid valid use cases of org-pending. A given user of
org-pending may decide to garantuee that though (using a suitable
function for :on-outcome).


> In my eyes, there is no difference between user request and "kill". If
> users asks things to stop, no modifications should be allowed to
> the region.

There is no relation between "kill" and "cancel".

For "kill", *Emacs* is killing the owner of the lock; there is nothing
to update.  This is synchronous, immediate and definitive.

For "cancel", the *user* is sending an asynchronous message to
org-pending that it would be nice to release this particular lock
sooner, if possible; that message implies that the user doesn't care
about the outcome, but, if that outcome is available, then, just don't
waste it: insert it in the document.

Should I rename "kill" and "cancel" to something better ?



>> I modified the example to rely on the reglock when possible (as
>> opposed to values kept from the creation time).
>
> I tried to simplify your example as the following:
>
[...]
>

Nice! Thanks.

I simplified the pcase.  I switched back to the "my-" prefix.  I'm not
sure why you're using quoted lambdas, as if we were in 2011 ;) I guess
it's so that this example works when copy/pasted to scratch for
evaluation in a non lexical-binding buffer.

I've left the 'warn' messages, but, I'm not sure we should. In my
case, they are just killing my window configuration, even stealing the
window where the lock itself was.



> In the above, it is not fully clear for me what BEG and END arguments in
> `org-pending-reglock-insert-details-function' mean and where the
> insertion happens.

I've removed '_start' and '_end' from the example: they are advanced
features (see the documentation of the field insert-details-function).

The insertion happens in the description buffer, at a position deemed
suitable (the current implementation inserts it at the end of the
buffer).

Thanks!

Bruno


Reply via email to