Ihor Radchenko <yanta...@posteo.net> writes:
> Bruno Barbier <brubar...@gmail.com> writes:
>
> I am attaching some minor edits I'd like to propose on top of your
> latest branch.
I've applied your patch. Thanks.
> Also, some more questions.
>
>> ;; (setq my-rlock
>> ;; (org-pending (cons (point) (mark))
>> ;; (lambda (outcome)
>> ;; (pcase outcome
>> ;; (`(:success ,result) (goto-char END)
>> (insert result))
>> ;; (`(:failure ,err) (message "Failed: %s"
>> err))))))
>
> 1. It is more natural in Elisp to pass regions as two parameters: BEG
> and END, not a cons cell.
I rewrote the example so that it's complete and can be uncommented and
evaluated.
With the new example, it's less obvious which is more natural.
> 2. Note that (point) may be _after_ (mark). AFAIU, you code assumes
> that point is always before the mark. You may want to address this.
Right. I'm not using those in the new example: problem solved ;)
> 3. ON-OUTCOME is optional. What happens if none is provided?
No function is called. And thus, no value is returned, so no mark.
It's like calling a function that does nothing and return nil.
> 4. In the `org-pending' docstring you say "ON-OUTCOME is non-nil, call
> it with the reglock and the outcome", but the example shows a lambda
> accepting a single argument. Something is off. I'm afraid that this
> example will not work if copy-pasted.
>
You're right. Sorry. I'm now using using a fully working example.
>> ;; (org-pending-send-update my-rlock (list :progress "Not ready
>> yet."))
>> ;; (org-pending-send-update my-rlock (list :progress "Coming soon."))
>
> Should the progress message always be a string?
No. It may currently be any data. org-pending will format it as a
string that fits on one line.
>> ;; (org-pending-send-update my-rlock (list :success 1))
>
> What will org-pending do with :success 1? Will it replace region with
> "1" or will it do something else?
That's the job on ON-OUTCOME to convert/format/append/prepend/replace
some content if needed, on :success and/or on :failure.
>> ;; (org-pending-send-update my-rlock (list :failure "Some error!"))
>
> I am slightly confused by this calling convention. Why not simply
>
> (org-pending-send-update my-rlock :failure "Some error!")
Because one message update is one value, either:
(list :success value)
or
(list :failure err)
or
(list :progress data)
On :success/:failure, this message is also a valid outcome, and it's
the same value that ON-OUTCOME will receive, as one single entity.
When listing artificial calls like this, it does look suboptimal; but,
in general, it's simpler to have a real value we can store, pass
around and return.
>
>> ;; (setf (org-pending-reglock-insert-details-function my-reglock)
>> ;; (lambda (rl _start _end)
>> ;; (insert (format "%s" (org-pending-reglock-property rl
>> :my-prop)))))
>
> Are there any standard properties? It would be nice to list them in a
> table as well.
There are no standard properties. All required properties have been
hard-coded in the cl-defstruct.
> Also, you can show an example of _setting_ :my-prop property.
I removed the use of `org-pending-reglock-property' from the example; I
think it was a mistake to introduce that advanced feature here. The
documentation of the function `org-pending-reglock-property' explains
how to set a property.
>> ;; If the user kills a buffer, or, kills Emacs, some locks may have to
>> ;; be killed too. The library will ask the user to confirm if an
>> ;; operation requires to kill some locks. See the field
>> ;; `before-kill-function' of REGLOCK object, if you need to do
>> ;; something before a lock is really killed. For example, if you like
>> ;; to kill a MY-BUFFER before MY-LOCK is killed, you can do:
>> ;;
>> ;; (setf (org-pending-reglock-before-kill-function my-reglock)
>> ;; (lambda (_rl) (kill-buffer my-buffer)))
>
> It would be nice to have an example that will also send a signal to
> process, as it is probably the most commonly used way to utilize
> org-pending.
For my many use cases, that would always be a mistake to kill the
process: an OS process is always in charge of many locks.
More importantly, to find a self-contained working readable example
might be a challenge.
We could add a function 'org-pending-shell-command-on-region' in
org-pending, that could be used as an implementation example, like
`org-pending-user-edit', `org-babel-execute-src-block', etc.
> From `org-pending' docstring:
>
>> 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.
>
> What if ON-OUTCOME returns something that is not a cons cell and not nil?
>
Then org-pending would have raised some error at some point, as it's not
a valid value; the doc says that ON-OUTCOME may return a region or
nothing.
I added a test so that org-pending will now scream immediately if
ON-OUTCOME returns an invalid value.
I've updated my public branch,
Let me know what you think,
Thanks,
Bruno
