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

[Ihor Radchenko <yantar92@posteo.net>]

Bruno Barbier <brubar.cs@gmail.com> writes:

>> I feel that org-pending-penreg (org-pending-<pending region>) is
>> redundant. Maybe better use org-pending-region?
>
> PENREG is the name of the structure; the "org-pending" is the
> mandatory library prefix; this mechanically gives the name.  A PENDREG
> object is not a "region" in Emacs sense.
>
> Do you see a better name for the structure PENREG, so that it doesn't
> sound like a "region" ?

Library prefix is also a part of the name and delivers useful
information. "org-pending-region" and "region" and not the same names.

We make use of prefix semantic in various places:
- org-export-backend, implying not just "backend", but also "export"
- org-cite-processor, implying not just "processor", but also "cite"
- org-lint-checker - "org-lint" + "checker"
- org-element-deferred - "org-element" + "deferred"

So, there is no need to duplicate information from the prefix - it is an
integral part of the struct name. Doing otherwise would go again the
existing naming in Org code base.

>> 1. It is not clear why you need a separate ~virtual~ field. When
>>    ~region~ is nil it already implies that the pending region is
>>    virtual.
>
> It's a constant.  Calling a function looks more like we need to
> recompute it each time, and, we could even change it.  And
> cl-defstruct writes the function for us.
>
> Do you prefer a manually written function ?

Either a function or a clear indication in the docstring that ~virtual~
and ~region~ are connected and both read-only.

Also, ~virtual~ field is unused. So, maybe we can even drop it
completely. We can always add new fields in future, if a need arises.

>> 3. ~source~ field must match the ~region~ marker buffer. Then, why do we
>>    need this field at all? May as well just use (marker-buffer (car region))
>
> The "source" is the region requesting the update.

The docstring of `org-pending' states that it is a buffer position:

    The SOURCE is the buffer position that requested this pending region.

> ... The pending region
> is the "target" of the update, i.e. the region that will be updated.
>
>
> For example, in DEMO_ONLY, with org-babel, these 2 regions are never
> the same:
>    1. the source is the source code block,
>    2. the target (pending region) is the result region.

I am wondering why source must be a buffer position.
What if we want to mark a region pending for some task not associated
with a source? And why do we need to know the source at all?

>      2. insert-details: If, and only if, the user decides to
>      investigate what happened, Emacs will ask the task if it has any
>      details to add, that might help the user (like exit-code for an
>      OS process, stderr for an OS process or link to a log file, etc.)

I have to say that I am confused about "insert-details" part. Mostly
because it is not per se connected to the associated task. It is rather
an additional handler used to provide debug information about the task
status and outcome.

AFAIU, it is conceptually very similar to HANDLE-RESULT function.

I think that rather than handing HANDLE-RESULT and also TASK-CONTROL, we
may reduce everything to a single "handler" object that will serve as a
way for PENREG to communicate back to Elisp. That way, we do not need to
have a concept of a "task". Instead, it will be a familiar async API
with ability to (1) create (2) send signals to (3) receive signals from
PENREG object.

`org-pending' will be the entry point to create PENREG object.

`org-pending-ti-send-update' (or maybe simply
`org-pending-send-update'?) will be a way to send data to PENREG object.

HANLDER will be another object we may expose via something like
(org-pending-handler (&key on-success-function on-cancel-function on-await on-insert-logs) ...)
Then, PENREG will call appropriate handler function as needed.

>>    If the argument to ~org-pending-task-connect~ is a lambda, we can use
>>    the current approach you implemented on the branch.
>
>> 2. ~org-pending-task-send-update~ name is confusing - it reads as if we
>>    send an update _to_ the task. Maybe better ~org-pending-region-update~?
>
> Yes ... I wanted a common prefix for the 3 functions that a "task"
> implementation is allowed to use:
>     - org-pending-task-connect,
>     - org-pending-task-send-update,
>     - org-pending-task-not-implemented.
>
> It's not confusing if one ignores the common prefix :-)
>
> I've renamed all these functions from "org-pending-task-" to
> "org-pending-ti-" where "ti" stands for "task implementation".

I still feel confused. As stated above, it might be a good idea to get
rid of the concept of "task" completely.

>>    Then, we might even drop ~-sentinel~ field in org-pending-penreg
>>    object and instead implement that hard-coded ~update~ lambda from
>>    ~org-pending~ as a part of ~org-pending-region-update~.
>
> That would require to manually capture (dump/load) the context that
> the sentinel closure is automatically capturing.
>
> Why would it be better ? Debugging purposes ?

Yes. Lexical context is implicit and harder to debug, while storing
necessary data explicitly in the struct slots is more robust as we are
very clear which context is intended to be captured.

>> 3. I feel that different handling of "owner" and indirect buffers is not
>>    right.
>>    From the user perspective, it does not matter whether we run an src
>>    block from one or another indirect buffers - it makes sense to see
>>    the status in all the indirect org-mode buffers.
>
> I just tried to followe Emacs: a buffer owns its overlays; a pending
> region is (kind of) an overlay.  Thus, a buffer owns its pending
> region.

I do not think that it is a good analogy. Not when we also mark the text
read-only in all the indirect buffers as well.

Let me state my idea differently - if some text in buffer is "pending",
it should be visible in all indirect buffers. Otherwise, as a user, I
may be confused why some parts of the buffer are read-only.

>>    Maybe we can hook into org-mode's fontification and display
>>    pending overlays in all the indirect buffers.
>
> Well ... "adding overlays in indirect buffers using font-lock" looks
> like a very bumpy road to me ... (being very positive, assuming there
> is even a road there ... :-) ). As jit-lock is explicitly disabled in
> indirect buffers, I'm not even sure what it would technically mean.

Right. font-lock is linked to text properties, so font-lock in indirect
buffers is finicky (it relies upon text properties to function).

We might try to brew something with `pre-redisplay-functions', but let's
not dive into that rabbit hole for now.

>>    Further, it is very confusing that running src block twice from the
>>    same buffer is not the same as running the same src block from one
>>    buffer and then from another indirect buffer. The current
>>    implementation of ~remove-previous-overlays~ makes such distinction
>>    for reasons I do not understand.
>
> Technically, the outcomes are overlays too; thus, they belong to
> one buffer.
>
> If a user created an indirect buffer to focus on some source blocks,
> he should expect to manage everything about them from that buffer.
> ... that looks to me like a plausible explanation that matches the
> technical limitations :-)

This sounds like trying to fit (by force) expectations to technical
limitations. As a user, I do not really manage everything from the same
buffer and do not expect that Org mode expects me to do so :)

But let's postpone indirect buffer discussion to later and focus on more
high-level design first.

> ... would prefer to put the no-clone-indirect
> property to the org-mode personally :-)

We cannot do it. People use indirect buffers with Org mode extensively.

> Couldn't we just to forbid "pending regions" in indirect buffers ?
> (pending regions don't exist today, so, that doesn't look that bad, at
> least for now)

This might be ok. But we should be prepared that "read-only" on the
pending regions is not going to be reliable - the regions marked pending
can be changed out of sight.

>>  but I can easily see that we need to
>> handle things specially on failure as well. For example, insert
>> stderr or perform other actions like displaying some buffer.  Or we
>> may even hook some special action to clicking on status overlay. For
>> example, clicking on "failure" status overlay may raise stderr log.
>
> It's already there, no?
>
> If you click on any result (success or failure, inline block or not,
> even dynamic blocks), Emacs pops up a buffer with all the details
> (source, start, end, duration, stderr, etc.). The function
> `org-pending-describe-penreg' defines what is inserted. A given task is
> free to insert log, links, widgets, images, diffs, etc. (by providing
> the relevant :insert-details method).

Your thinking also makes sense, if I use a different definition of
"failure" (in the context of PENREG, not in the context of exit code of
the attached process)

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>