Re: [PATCH] Async evaluation in ob-shell

2023-04-17 Thread Ihor Radchenko
Matt  writes:

>  > Thanks, but I am not seeing 6c9104f59.
>
> I'm sorry, I was unclear about which repo these commits were in.  ORG-NEWS 
> was updated here: 
> https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=6c9104f59ca8085abe477a81857548461bf88f23

Now, see. I was again confused by savannah interface...
Thanks for the clarification!

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 



Re: [PATCH] Async evaluation in ob-shell

2023-04-17 Thread Matt


  On Mon, 17 Apr 2023 14:53:18 -0400  Ihor Radchenko  wrote --- 
 > Matt m...@excalamus.com> writes:
 > 
 > >  > A small note on the WORG page: it may be more natural to use :async yes
 > >  > rather than :async t. Both are viable - in fact, anything other than
 > >  > :async no and :async none will be treated as "t".
 > >
 > > Updated WORG with commits 9d153ea4, 754c72cb, and 18333299.  Updated 
 > > ORG-NEWS with 6c9104f59.
 > 
 > Thanks, but I am not seeing 6c9104f59.

I'm sorry, I was unclear about which repo these commits were in.  ORG-NEWS was 
updated here: 
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=6c9104f59ca8085abe477a81857548461bf88f23



Re: [PATCH] Async evaluation in ob-shell

2023-04-17 Thread Ihor Radchenko
Matt  writes:

>  > A small note on the WORG page: it may be more natural to use :async yes
>  > rather than :async t. Both are viable - in fact, anything other than
>  > :async no and :async none will be treated as "t".
>
> Updated WORG with commits 9d153ea4, 754c72cb, and 18333299.  Updated ORG-NEWS 
> with 6c9104f59.

Thanks, but I am not seeing 6c9104f59.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 



Re: [PATCH] Async evaluation in ob-shell

2023-04-17 Thread Matt


  On Fri, 24 Mar 2023 05:11:38 -0400  Ihor Radchenko  wrote --- 
 > Matt m...@excalamus.com> writes:
 > 
 > >   On Thu, 23 Mar 2023 07:48:44 -0400  Ihor Radchenko  wrote --- 
 > >  > May you also document this new feature in ORG-NEWS and in
 > >  > https://orgmode.org/worg/org-contrib/babel/languages/ob-doc-shell.html ?
 > >
 > > Done.
 > 
 > A small note on the WORG page: it may be more natural to use :async yes
 > rather than :async t. Both are viable - in fact, anything other than
 > :async no and :async none will be treated as "t".

Updated WORG with commits 9d153ea4, 754c72cb, and 18333299.  Updated ORG-NEWS 
with 6c9104f59.



Re: [PATCH] Async evaluation in ob-shell

2023-03-28 Thread Ihor Radchenko
Matt  writes:

> -(session (assq :session params)))
> +(sessionp (not (member (cdr (assq :session params)) '("no" 
> "none")
> +(if (and async (not sessionp))
> +  (error (org-babel-eval-error-notify 1 "ERROR: must use 'async' 
> with 'session'")))

This is reasonable, but please do not use `org-babel-eval-error-notify'.
Just a simple `user-error'.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 



Re: [PATCH] Async evaluation in ob-shell

2023-03-27 Thread Matt

  On Fri, 24 Mar 2023 05:13:34 -0400  Ihor Radchenko  wrote --- 

 > A small note on the WORG page: it may be more natural to use :async yes
 > rather than :async t. Both are viable - in fact, anything other than
 > :async no and :async none will be treated as "t".
 
Ah, okay.  I'll make that more clear.

Somewhat related, I had this left over from when I was working out the async 
code.  It prevents the user from running :async without the (required) :session 
header.The :async header runs the default (blocking) process when :session 
is missing.

diff --git a/lisp/ob-comint.el b/lisp/ob-comint.el
index 86c2bf7a7..384bfcda8 100644
--- a/lisp/ob-comint.el
+++ b/lisp/ob-comint.el
@@ -206,11 +206,12 @@ comint outputs due to buffering.")
 PARAMS are the header arguments as passed to
 `org-babel-execute:lang'."
   (let ((async (assq :async params))
-(session (assq :session params)))
+(sessionp (not (member (cdr (assq :session params)) '("no" "none")
+(if (and async (not sessionp))
+  (error (org-babel-eval-error-notify 1 "ERROR: must use 'async' with 
'session'")))
 (and async
-(not org-babel-exp-reference-buffer)
- (not (equal (cdr async) "no"))
- (not (equal (cdr session) "none")
+ sessionp
+ (not org-babel-exp-reference-buffer
 
 (defun org-babel-comint-async-filter (string)
   "Captures Babel async output from comint buffer back to Org mode buffers.

It's really just a nicety.  The user can cancel the accidental blocking process 
with C-g.  However, the block is run in a different shell than expected and 
it's jarring to have Emacs freeze when you expect async.

Thoughts on including it or something similar?

error-on-async-session-mismatch.diff
Description: Binary data


Re: [PATCH] Async evaluation in ob-shell

2023-03-24 Thread Ihor Radchenko
Matt  writes:

>   On Thu, 23 Mar 2023 07:48:44 -0400  Ihor Radchenko  wrote --- 
>  > May you also document this new feature in ORG-NEWS and in
>  > https://orgmode.org/worg/org-contrib/babel/languages/ob-doc-shell.html ?
>
> Done.

A small note on the WORG page: it may be more natural to use :async yes
rather than :async t. Both are viable - in fact, anything other than
:async no and :async none will be treated as "t".

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 



Re: [PATCH] Async evaluation in ob-shell

2023-03-23 Thread Matt


  On Thu, 23 Mar 2023 07:48:44 -0400  Ihor Radchenko  wrote --- 
 > May you also document this new feature in ORG-NEWS and in
 > https://orgmode.org/worg/org-contrib/babel/languages/ob-doc-shell.html ?

Done.



Re: [PATCH] Async evaluation in ob-shell

2023-03-23 Thread Ihor Radchenko
Matt  writes:

> diff --git a/lisp/ob-shell.el b/lisp/ob-shell.el
> index 9e7b45a89..eab8ea935 100644
> --- a/lisp/ob-shell.el
> +++ b/lisp/ob-shell.el
> @@ -269,12 +269,22 @@ var of the same value."
>   (set-marker comint-last-output-start (point))
>   (get-buffer (current-buffer)))
>  
> +(defconst ob-shell-async-indicator "echo 'ob_comint_async_shell_%s_%s'"
> +  "Session output delimiter template.
> +See `org-babel-comint-async-indicator'.")
> ...

I see that you pushed this onto main.
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=f7aa8c19f5170dbf09538686fb569f9b60acbd6c
May you also document this new feature in ORG-NEWS and in
https://orgmode.org/worg/org-contrib/babel/languages/ob-doc-shell.html ?

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 



Re: [PATCH] Async evaluation in ob-shell

2023-03-22 Thread Ihor Radchenko
Matt  writes:

> Unfortunately, I was mistaken and the second option (removing the space from 
> `org-babel-sh-prompt') doesn't fix the issue.  The TLDR is that the code in 
> `org-babel-comint-async-filter' which grabs the region between the indicators 
> (incorrectly) fails to include the prompt's trailing space.
> --- a/lisp/ob-comint.el
> +++ b/lisp/ob-comint.el
> @@ -273,7 +273,7 @@ STRING contains the output originally inserted into the 
> comint buffer."
>  (res-str-raw
>   (buffer-substring
>;; move point to beginning of indicator
> - (- (match-beginning 0) 1)
> + (match-beginning 0)
>;; find the matching start indicator
>(cl-loop
>do (re-search-backward indicator)

This change looks reasonable.
If it does not break tests, I see not problem with pushing it as a
separate commit (to make things easier in case if that -1 did matter at
the end).

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 



Re: [PATCH] Async evaluation in ob-shell

2023-03-21 Thread Matt

 > Matt m...@excalamus.com> writes:
 >
 > I see only two options to fix it: remove a space from the concat expression 
 > (which I did in my latest patch) or remove a space from 
 > `org-babel-sh-prompt'.

Unfortunately, I was mistaken and the second option (removing the space from 
`org-babel-sh-prompt') doesn't fix the issue.  The TLDR is that the code in 
`org-babel-comint-async-filter' which grabs the region between the indicators 
(incorrectly) fails to include the prompt's trailing space.

#+begin_longwinded_explanation
I'll first explain why removing the space from `org-babel-sh-prompt' doesn't 
fix the issue because it well also highlight the underlying problem.

If we remove the space from the `org-babel-sh-prompt', then 
`comint-prompt-regexp' becomes "^org_babel_sh_prompt> *" (with one space).   
This would work if the string passed to the `ob-shell-async-chunk-callback' 
stayed the same.  It doesn't (this is where my reasoning and testing failed).  
Changing the `org-babel-sh-prompt' to "org_babel_sh_prompt>" (without a space) 
causes the following string to be passed to the callback:

"org_babel_sh_prompt>1
org_babel_sh_prompt>2
org_babel_sh_prompt"

Note that the final prompt doesn't have a ">" and therefore the 
`comint-prompt-regexp' (which becomes "^org_babel_sh_prompt> * (with one 
space)) used in the callback fails to match it.  When we remove the space from 
the `org-babel-sh-prompt', the session buffer looks like this:

"sh-5.1$ PROMPT_COMMAND=;PS1="org_babel_sh_prompt>";PS2=
org_babel_sh_prompt>echo 
'ob_comint_async_shell_start_39610981-1020-4baf-9dfb-f96d10af1cf8'
echo 1
echo 2
echo 'ob_comint_async_shell_end_39610981-1020-4baf-9dfb-f96d10af1cf8'
ob_comint_async_shell_start_39610981-1020-4baf-9dfb-f96d10af1cf8
org_babel_sh_prompt>1
org_babel_sh_prompt>2
org_babel_sh_prompt>ob_comint_async_shell_end_39610981-1020-4baf-9dfb-f96d10af1cf8
org_babel_sh_prompt>"

The `org-babel-comint-async-filter' is what calls the 
`ob-shell-async-chunk-callback' (ob-comint.el:284).  It monitors for the end 
indicator.  When that appears, it passes the region between the beginning of 
the end indicator **less 1** and the character after the end of the start 
indicator to the callback.  For a clean run of 
`test-ob-shell/session-async-evaluation', the beginning of the end indicator is 
at 361 and the character after the end of the start indicator is at 298.  This 
is the string I gave above which is missing the ">".  

In order to make the second option work, we'd need to change the "less 1" part 
of `org-babel-comint-async-filter' from (- (match-beginning 0) 1) to 
(match-beginning 0).   It turns out that's actually all we need to do.

When `org-babel-sh-prompt' is "org_babel_sh_prompt> " (with one space), then 
the session buffer looks like:

"sh-5.1$ PROMPT_COMMAND=;PS1="org_babel_sh_prompt> ";PS2=
org_babel_sh_prompt> echo 
'ob_comint_async_shell_start_3270ed43-a99b-423f-a5fa-b15fb2e4ae26'
echo 1
echo 2
echo 'ob_comint_async_shell_end_3270ed43-a99b-423f-a5fa-b15fb2e4ae26'
ob_comint_async_shell_start_3270ed43-a99b-423f-a5fa-b15fb2e4ae26
org_babel_sh_prompt> 1
org_babel_sh_prompt> 2
org_babel_sh_prompt> 
ob_comint_async_shell_end_3270ed43-a99b-423f-a5fa-b15fb2e4ae26
org_babel_sh_prompt> "

The region passed to the callback is then defined as 366 to 300, or

"org_babel_sh_prompt> 1
org_babel_sh_prompt> 2
org_babel_sh_prompt>"  (<-- no space)

This looks okay at first glance.  However, **the last line is not a valid 
prompt**.  A prompt must end in a space!  When the `org-babel-sh-prompt' is set 
to  "org_babel_sh_prompt> " (with one space), the `comint-prompt-regexp' is 
"^org_babel_sh_prompt>  *" (with two spaces).  This means that the 
`comint-prompt-regexp' matches on a trailing space which the **region passed to 
the callback doesn't have**.  Therefore, the match fails.

Instead, if we modify the `org-babel-comint-async-filter' like

modified   lisp/ob-comint.el
@@ -273,7 +273,7 @@ STRING contains the output originally inserted into the 
comint buffer."
   (res-str-raw
(buffer-substring
 ;; move point to beginning of indicator
- (- (match-beginning 0) 1)
+ (match-beginning 0)
 ;; find the matching start indicator
 (cl-loop
   do (re-search-backward indicator)

then the region passed to the callback will be from 367 to 300, or

"org_babel_sh_prompt> 1
org_babel_sh_prompt> 2
org_babel_sh_prompt> " (<-- with one space)

The `comint-prompt-regexp' will now match the last prompt in the region.

With this change, the `org-babel-sh-prompt' keeps the trailing space (like it 
should), the `comint-prompt-regexp' becomes "^org_babel_sh_prompt>  *" (with 
two spaces, requiring a prompt to have a trailing space like it should), the 
`ob-shell-async-chunk-callback' can use `comint-prompt-regexp' without 
modification, and the tests all pass.

Re: [PATCH] Async evaluation in ob-shell

2023-03-18 Thread Ihor Radchenko
Matt  writes:

> Way back in https://list.orgmode.org/87sfeyc7qr.fsf@localhost/, we had this 
> exchange:
>
>   On Mon, 20 Feb 2023 11:24:52 +  Ihor Radchenko  wrote --- 
>> Matt  writes:
>>
>> > +(defun ob-shell-async-chunk-callback (string)
>> > +  "Filter applied to results before insertion.
>> > +See `org-babel-comint-async-chunk-callback'."
>> > +  (replace-regexp-in-string (concat org-babel-sh-prompt "*") "" string))
>>
>  > Why not using `comint-prompt-regexp'?
>
> I switched out `org-babel-sh-prompt'  with `comint-prompt-regexp' so that the 
> expression looks like:
>
> +(defun ob-shell-async-chunk-callback (string)
> +  "Filter applied to results before insertion.
> +See `org-babel-comint-async-chunk-callback'."
> +  (replace-regexp-in-string comint-prompt-regexp "" string))
>
> This causes the new test `test-ob-shell/session-async-evaluation' to fail, as 
> you pointed out: https://list.orgmode.org/87bkl96g6e.fsf@localhost/
>
> The test fails when we switch out the prompt in the callback because 
> `comint-prompt-regexp' has two spaces in it.  The second space causes a 
> prompt to not be filtered (by the callback).  The output becomes ": 1\n: 2\n: 
> org_babel_sh_prompt>\n" instead of  ": 1\n: 2\n" .  This looks like a bug in 
> the `comint-prompt-regexp''.

No, this looks like a bug in comint.el. ob-shell correctly sets
PROMPT to be org-babel-sh-prompt, which is "org_babel_sh_prompt> ", with
space! The fact that the comint output does not, in fact, contain space
is wrong.

We can probably remove the space in org-babel-sh-prompt to work around
this (likely, Emacs) bug.

> It could be that `test-ob-shell/session-async-evaluation' doesn't test 
> correctly, but it looks right to me (I could certainly be mistaken).  
> Therefore, I see only two options to fix it: remove a space from the concat 
> expression (which I did in my latest patch) or remove a space from 
> `org-babel-sh-prompt'.

Removing space from the concat expression in plain wrong, as Max
explained. 

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 



Re: [PATCH] Async evaluation in ob-shell

2023-03-12 Thread Jack Kamm
Matt  writes:

> Hi Jack and Jeremie!  I'm curious your thoughts about what Ihor and I are 
> discussing at the end of this message about `md5'.

Thanks for checking in. I don't see any problems with switching to
org-id-uuid or similar.

Feel free to update ob-python to do this, or I can also do it later
following ob-shell's example.



Re: [PATCH] Async evaluation in ob-shell

2023-03-09 Thread Max Nikulin

On 10/03/2023 00:36, Matt wrote:

(concat "^" (regexp-quote org-babel-sh-prompt) " *") -> "^org_babel_sh_prompt>  
*"

Currently, the two combine via the concat to give*two*  spaces in the regexp.


"*" means zero or more, so "  *" with two spaces acts like " +" one or 
more space characters. It is unsafe to append just "*" or "+" to another 
regexp since you can not ensure what is the trailing pattern of the 
preceding regexp.





Re: [PATCH] Async evaluation in ob-shell

2023-03-09 Thread Matt
Hi Jack and Jeremie!  I'm curious your thoughts about what Ihor and I are 
discussing at the end of this message about `md5'.

  On Tue, 07 Mar 2023 07:45:02 -0500  Ihor Radchenko  wrote --- 
 > Matt m...@excalamus.com> writes:
 > 
 > >  > The actual prompt is "org_babel_sh_prompt> ".
 > >
 > > Agreed.
 > >
 > >  > And we want to skip leading spaces in addition.
 > >  
 > > What do you mean by this?
 > 
 > I was following the pattern described in the docstring of
 > `comint-prompt-regexp', where it is suggested that prompts should follow
 > the pattern "^ *".
 > 
 > In the case of ob-shell.el, `org-babel-sh-prompt' is a string to be used
 > as a prompt and the corresponding regexp patter will be
 > "^ *". Hence
 > 
 >   (concat "^" (regexp-quote org-babel-sh-prompt) " *")
 > 
 > >  > Adding " *" does not make the prompt match 2 spaces, but 1+.
 > >  
 > > Agreed.  
 > >
 > > It's not clear to me what pattern you're looking to match.
 > 
 > I hope the above clarified things.

I'm confused because when I run the Org from HEAD, I get:

(concat "^" (regexp-quote org-babel-sh-prompt) " *") -> "^org_babel_sh_prompt>  
*"

That's *two* spaces between '>' and '*', not one.  The second space comes from 
either 1) the definition of `org-babel-sh-prompt', which is 
"org_babel_sh_prompt> " (with a single space) or 2) the " *" in the  (concat 
"^" (regexp-quote org-babel-sh-prompt) " *") expression.   Currently, the two 
combine via the concat to give *two* spaces in the regexp.

If I understand you correctly, you're trying to match the pattern given in the 
`comint-prompt-regexp' which is *one* space.   That's what I'm trying to do, 
too.

Way back in https://list.orgmode.org/87sfeyc7qr.fsf@localhost/, we had this 
exchange:

  On Mon, 20 Feb 2023 11:24:52 +  Ihor Radchenko  wrote --- 
> Matt  writes:
>
> > +(defun ob-shell-async-chunk-callback (string)
> > +  "Filter applied to results before insertion.
> > +See `org-babel-comint-async-chunk-callback'."
> > +  (replace-regexp-in-string (concat org-babel-sh-prompt "*") "" string))
>
 > Why not using `comint-prompt-regexp'?

I switched out `org-babel-sh-prompt'  with `comint-prompt-regexp' so that the 
expression looks like:

+(defun ob-shell-async-chunk-callback (string)
+  "Filter applied to results before insertion.
+See `org-babel-comint-async-chunk-callback'."
+  (replace-regexp-in-string comint-prompt-regexp "" string))

This causes the new test `test-ob-shell/session-async-evaluation' to fail, as 
you pointed out: https://list.orgmode.org/87bkl96g6e.fsf@localhost/

The test fails when we switch out the prompt in the callback because 
`comint-prompt-regexp' has two spaces in it.  The second space causes a prompt 
to not be filtered (by the callback).  The output becomes ": 1\n: 2\n: 
org_babel_sh_prompt>\n" instead of  ": 1\n: 2\n" .  This looks like a bug in 
the `comint-prompt-regexp''.

It could be that `test-ob-shell/session-async-evaluation' doesn't test 
correctly, but it looks right to me (I could certainly be mistaken).  
Therefore, I see only two options to fix it: remove a space from the concat 
expression (which I did in my latest patch) or remove a space from 
`org-babel-sh-prompt'.

Am I missing something?

 > >  > `md5' will be slightly faster compared to `org-id-uuid'. But it should
 > >  > not matter.
 > >  > 
 > >  > If we want use `org-id-uuid', lets move it to org-macs.el. Requiring the
 > >  > whole org-id.el must not be done. It has side effects of defining id:
 > >  > links.
 > >
 > > In the next revision (once we figure out the regex), I can create a 
 > > separate commit moving `org-id-uuid' to org-macs.el and updating ob-R and 
 > > ob-python from `md5' to `org-id-uuid' (assuming that's not an issue for 
 > > the maintainers of those).  If you think speed is a concern, however, I 
 > > can switch ob-shell.el to use plain `md5'.
 > 
 > I am in favour of using `org-id-uuid'. It might also be a useful generic
 > function for other purposes.
 > 
 > A slight concern is that some third-party code might depend on the
 > current pattern used for UUID string in ob-python. But we made no
 > promises here.
 > 
 > To be a bit safer, we can also refactor `org-uuidgen-p' exposing the
 > regexp used to match UUID. Also, it will make sense to move
 > `org-uuidgen-p' to org-macs.el.

I'm okay with all that.  I wonder, do Jack Kamm (of ob-python fame) and Jeremie 
Juste (of ob-R fame) have any thoughts on the matter.  I ask out of courtesy 
since they're the maintainers of those packages and I don't want to cross any 
boundaries by changing their implementations beneath them.  



Re: [PATCH] Async evaluation in ob-shell

2023-03-07 Thread Ihor Radchenko
Matt  writes:

>  > The actual prompt is "org_babel_sh_prompt> ".
>
> Agreed.
>
>  > And we want to skip leading spaces in addition.
>  
> What do you mean by this?

I was following the pattern described in the docstring of
`comint-prompt-regexp', where it is suggested that prompts should follow
the pattern "^ *".

In the case of ob-shell.el, `org-babel-sh-prompt' is a string to be used
as a prompt and the corresponding regexp patter will be
"^ *". Hence

  (concat "^" (regexp-quote org-babel-sh-prompt) " *")

>  > Adding " *" does not make the prompt match 2 spaces, but 1+.
>  
> Agreed.  
>
> It's not clear to me what pattern you're looking to match.

I hope the above clarified things.

>  > `md5' will be slightly faster compared to `org-id-uuid'. But it should
>  > not matter.
>  > 
>  > If we want use `org-id-uuid', lets move it to org-macs.el. Requiring the
>  > whole org-id.el must not be done. It has side effects of defining id:
>  > links.
>
> In the next revision (once we figure out the regex), I can create a separate 
> commit moving `org-id-uuid' to org-macs.el and updating ob-R and ob-python 
> from `md5' to `org-id-uuid' (assuming that's not an issue for the maintainers 
> of those).  If you think speed is a concern, however, I can switch 
> ob-shell.el to use plain `md5'.

I am in favour of using `org-id-uuid'. It might also be a useful generic
function for other purposes.

A slight concern is that some third-party code might depend on the
current pattern used for UUID string in ob-python. But we made no
promises here.

To be a bit safer, we can also refactor `org-uuidgen-p' exposing the
regexp used to match UUID. Also, it will make sense to move
`org-uuidgen-p' to org-macs.el.

>  > 
>  > >  (concat "^" (regexp-quote org-babel-sh-prompt)
>  > > -" *"))
>  > > +"*"))
>  > 
>  > This is wrong. It unconditionally makes the last char in
>  > `org-babel-sh-prompt' 0+. (Imagine it is changed to non-space in
>  > future).
>
> When you say "imagine it is changed to non-space...", do you refer to 
> `org-babel-sh-prompt'?

Yes.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 



Re: [PATCH] Async evaluation in ob-shell

2023-03-05 Thread Matt


  On Sun, 05 Mar 2023 07:14:21 -0500  Ihor Radchenko  wrote --- 
 > > Matt m...@excalamus.com> writes:
 > >
 > > Sorry for missing that.  The issue is that when I replaced
 > > `org-babel-sh-prompt' with `comint-prompt-regexp', the regexp no
 > > longer matches the output and grabs the next prompt.  It looks like
 > > the reason is `comint-prompt-regexp' is set to "^org_babel_sh_prompt>
 > > *" (with two spaces between the '>' and '*').  Attached is a revised
 > > patch which removes one of the spaces by changing how
 > > `org-babel-sh-initiate-session' sets the `comint-prompt-regexp'.
 > > Another place this could be done is in the defvar for
 > > `org-babel-sh-prompt' itself (which ends with a space).  However, I
 > > think it's customary to leave a trailing space for prompts?
 > 
 > The actual prompt is "org_babel_sh_prompt> ".

Agreed.

 > And we want to skip leading spaces in addition.
 
What do you mean by this?

 > Adding " *" does not make the prompt match 2 spaces, but 1+.
 
Agreed.  

It's not clear to me what pattern you're looking to match.

 > >  > > +  (let ((uuid (org-id-uuid)))
 > >  > 
 > >  > Do you need to use `org-id-uuid' here? ob-python directly uses `md5'.
 > >  > If you still prefer org-id-uuid, we probably need to move it to
 > >  > org-macs.el
 > >
 > > I just need a random string.  `md5' would work for that.  However,
 > > might it be better to update ob-R and ob-python to use `org-id-uuid'?
 > > Both of those manually declare the randomness.  It's conceivable that
 > > someone may delete or mistype the number (1), resulting in a
 > > lower entropy.  An md5 is also not a uuid, strictly speaking.  Of
 > > course, the chance of collision is still basically zero and the cost
 > > of collision about the same.  Using `org-id-uuid' would only provide a
 > > consistent way to do things.
 > 
 > `md5' will be slightly faster compared to `org-id-uuid'. But it should
 > not matter.
 > 
 > If we want use `org-id-uuid', lets move it to org-macs.el. Requiring the
 > whole org-id.el must not be done. It has side effects of defining id:
 > links.

In the next revision (once we figure out the regex), I can create a separate 
commit moving `org-id-uuid' to org-macs.el and updating ob-R and ob-python from 
`md5' to `org-id-uuid' (assuming that's not an issue for the maintainers of 
those).  If you think speed is a concern, however, I can switch ob-shell.el to 
use plain `md5'.

 > 
 > >  (concat "^" (regexp-quote org-babel-sh-prompt)
 > > -" *"))
 > > +"*"))
 > 
 > This is wrong. It unconditionally makes the last char in
 > `org-babel-sh-prompt' 0+. (Imagine it is changed to non-space in
 > future).

When you say "imagine it is changed to non-space...", do you refer to 
`org-babel-sh-prompt'?

Honestly, it's not clear to me what pattern(s) we need to match.



Re: [PATCH] Async evaluation in ob-shell

2023-03-05 Thread Ihor Radchenko
Matt  writes:

>   On Fri, 03 Mar 2023 09:52:09 -0500  Ihor Radchenko  wrote --- 
>  > I tried the patch, and I am getting failed tests:
>  > 
>  > 1 unexpected results:
>
> Sorry for missing that.  The issue is that when I replaced
> `org-babel-sh-prompt' with `comint-prompt-regexp', the regexp no
> longer matches the output and grabs the next prompt.  It looks like
> the reason is `comint-prompt-regexp' is set to "^org_babel_sh_prompt>
> *" (with two spaces between the '>' and '*').  Attached is a revised
> patch which removes one of the spaces by changing how
> `org-babel-sh-initiate-session' sets the `comint-prompt-regexp'.
> Another place this could be done is in the defvar for
> `org-babel-sh-prompt' itself (which ends with a space).  However, I
> think it's customary to leave a trailing space for prompts?

The actual prompt is "org_babel_sh_prompt> ".
And we want to skip leading spaces in addition.

Adding " *" does not make the prompt match 2 spaces, but 1+.

Prompts with no single space are not prompts.

>
>  > > +  (let ((uuid (org-id-uuid)))
>  > 
>  > Do you need to use `org-id-uuid' here? ob-python directly uses `md5'.
>  > If you still prefer org-id-uuid, we probably need to move it to
>  > org-macs.el
>
> I just need a random string.  `md5' would work for that.  However,
> might it be better to update ob-R and ob-python to use `org-id-uuid'?
> Both of those manually declare the randomness.  It's conceivable that
> someone may delete or mistype the number (1), resulting in a
> lower entropy.  An md5 is also not a uuid, strictly speaking.  Of
> course, the chance of collision is still basically zero and the cost
> of collision about the same.  Using `org-id-uuid' would only provide a
> consistent way to do things.

`md5' will be slightly faster compared to `org-id-uuid'. But it should
not matter.

If we want use `org-id-uuid', lets move it to org-macs.el. Requiring the
whole org-id.el must not be done. It has side effects of defining id:
links.

>  (concat "^" (regexp-quote org-babel-sh-prompt)
> -" *"))
> +"*"))

This is wrong. It unconditionally makes the last char in
`org-babel-sh-prompt' 0+. (Imagine it is changed to non-space in
future).

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 



Re: [PATCH] Async evaluation in ob-shell

2023-03-03 Thread Matt

  On Fri, 03 Mar 2023 09:52:09 -0500  Ihor Radchenko  wrote --- 
 > I tried the patch, and I am getting failed tests:
 > 
 > 1 unexpected results:
 >FAILED  test-ob-shell/session-async-evaluation  ((should (string= ": 1\n: 
 > 2\n" (buffer-substring-no-properties (point) (point-max :form (string= 
 > ": 1\n: 2\n" ": 1\n: 2\n: org_babel_sh_prompt>\n") :value nil :explanation 
 > (arrays-of-different-length 8 31 ": 1\n: 2\n" ": 1\n: 2\n: 
 > org_babel_sh_prompt>\n" first-mismatch-at 8))

Sorry for missing that.  The issue is that when I replaced 
`org-babel-sh-prompt' with `comint-prompt-regexp', the regexp no longer matches 
the output and grabs the next prompt.  It looks like the reason is 
`comint-prompt-regexp' is set to "^org_babel_sh_prompt>  *" (with two spaces 
between the '>' and '*').  Attached is a revised patch which removes one of the 
spaces by changing how `org-babel-sh-initiate-session' sets the 
`comint-prompt-regexp'.  Another place this could be done is in the defvar for 
`org-babel-sh-prompt' itself (which ends with a space).  However, I think it's 
customary to leave a trailing space for prompts?

 > > +  (let ((uuid (org-id-uuid)))
 > 
 > Do you need to use `org-id-uuid' here? ob-python directly uses `md5'.
 > If you still prefer org-id-uuid, we probably need to move it to
 > org-macs.el

I just need a random string.  `md5' would work for that.  However, might it be 
better to update ob-R and ob-python to use `org-id-uuid'?  Both of those 
manually declare the randomness.  It's conceivable that someone may delete or 
mistype the number (1), resulting in a lower entropy.  An md5 is also 
not a uuid, strictly speaking.  Of course, the chance of collision is still 
basically zero and the cost of collision about the same.  Using `org-id-uuid' 
would only provide a consistent way to do things.

0003-ob-shell-Add-async-evaluation.patch
Description: Binary data


Re: [PATCH] Async evaluation in ob-shell

2023-03-03 Thread Ihor Radchenko
Matt  writes:

> From b66d776346c992ec085bd719ab73f3d1773f71cc Mon Sep 17 00:00:00 2001
> From: Matthew Trzcinski 
> Date: Wed, 1 Mar 2023 20:31:46 -0500
> Subject: [PATCH] ob-shell: Add async evaluation

I tried the patch, and I am getting failed tests:

1 unexpected results:
   FAILED  test-ob-shell/session-async-evaluation  ((should (string= ": 1\n: 
2\n" (buffer-substring-no-properties (point) (point-max :form (string= ": 
1\n: 2\n" ": 1\n: 2\n: org_babel_sh_prompt>\n") :value nil :explanation 
(arrays-of-different-length 8 31 ": 1\n: 2\n" ": 1\n: 2\n: 
org_babel_sh_prompt>\n" first-mismatch-at 8))

> +  (let ((uuid (org-id-uuid)))

Do you need to use `org-id-uuid' here? ob-python directly uses `md5'.
If you still prefer org-id-uuid, we probably need to move it to
org-macs.el

-- 
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>



Re: [PATCH] Async evaluation in ob-shell

2023-03-01 Thread Matt

  On Wed, 22 Feb 2023 05:29:59 -0500  Ihor Radchenko  wrote --- 
 > > +(defun ob-shell-async-chunk-callback (string)
 > > +  "Filter applied to results before insertion.
 > > +See `org-babel-comint-async-chunk-callback'."
 > > +  (replace-regexp-in-string (concat org-babel-sh-prompt "*") "" string))
 > 
 > Why not using `comint-prompt-regexp'?
 > 
 > > +(ert-deftest test-ob-shell/session-async-valid-header-arg-values ()
 > > +  "Test that session runs asynchronously for certain :async values."
 > > +  (let ((session-name 
 > > "test-ob-shell/session-async-valid-header-arg-values")
 > > +(kill-buffer-query-functions nil))
 > > +(cl-loop
 > 
 > A simple `dolist' would do here. There is no reason to use `cl-loop'.

Great points!  Changed.

0002-ob-shell-Add-async-evaluation.patch
Description: Binary data


Re: [PATCH] Async evaluation in ob-shell

2023-02-22 Thread Ihor Radchenko
Matt  writes:

> I've rewritten the test and updated the patch. 

Thanks!

> +(defun ob-shell-async-chunk-callback (string)
> +  "Filter applied to results before insertion.
> +See `org-babel-comint-async-chunk-callback'."
> +  (replace-regexp-in-string (concat org-babel-sh-prompt "*") "" string))

Why not using `comint-prompt-regexp'?

> +(ert-deftest test-ob-shell/session-async-valid-header-arg-values ()
> +  "Test that session runs asynchronously for certain :async values."
> +  (let ((session-name "test-ob-shell/session-async-valid-header-arg-values")
> +(kill-buffer-query-functions nil))
> +(cl-loop

A simple `dolist' would do here. There is no reason to use `cl-loop'.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 



Re: [PATCH] Async evaluation in ob-shell

2023-02-20 Thread Matt

  On Mon, 20 Feb 2023 06:24:52 -0500  Ihor Radchenko  wrote --- 

 > Why not simply doing the `should' test when the
 > `test-ob-shell/uuid-regex' match fails? Instead of returning `t'. Then,
 > we will not need to use advise.

Great point. I had originally used advice to avoid a loop.  However, when it 
became apparent that the loop was necessary, I overlooked that the advice was 
no longer needed.

I've rewritten the test and updated the patch. 

0001-ob-shell-Add-async-evaluation.patch
Description: Binary data


Re: [PATCH] Async evaluation in ob-shell

2023-02-20 Thread Ihor Radchenko
Matt  writes:

> +(advice-add
> + 'ob-shell-async-chunk-callback
> + :filter-return
> + (lambda ( r)
> +   (let ((result (car r)))
> + (should (string= result "1\n2\n"))  ; expect value
> + result))
> + `((name . ,session-name)))
> ...
> +(catch 'too-long
> +  (while (string-match test-ob-shell/uuid-regex 
> (buffer-string))
> +(progn
> +  (sleep-for 0.01)
> +  (when (time-less-p wait-time (current-time))
> +(throw 'too-long (ert-fail "Took too long to get 
> result from callback")
> +  t)))

Why not simply doing the `should' test when the
`test-ob-shell/uuid-regex' match fails? Instead of returning `t'. Then,
we will not need to use advise.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 



Re: [PATCH] Async evaluation in ob-shell

2023-02-19 Thread Matt

  On Fri, 17 Feb 2023 05:44:20 -0500  Ihor Radchenko  wrote --- 
 > What I mean is...

I follow you now.  Thank you.

I've attached a patch (with commit message) for adding async to ob-shell.  If 
it looks good, I can apply it to main.

0001-ob-shell-Add-async-evaluation.patch
Description: Binary data


Re: [PATCH] Async evaluation in ob-shell

2023-02-17 Thread Ihor Radchenko
Matt  writes:

>  > Doesn't (while ... (sleep-for ...)) work?
>
> I'm afraid I don't follow what you mean.

What I mean is:

1. Execute src block asynchronously
2. Run (while ... (sleep-for ...)) until the result appears (up to
   threshold seconds)
3. Check the result as usual or fail the test when we wait too long and
   no results is added.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 



Re: [PATCH] Async evaluation in ob-shell

2023-02-15 Thread Matt


  On Wed, 15 Feb 2023 10:08:47 -0500  Ihor Radchenko  wrote --- 
 > Matt m...@excalamus.com> writes:
 > 
 > > Checking the final result from the callback is trickier.   The following 
 > > works, but requires advice (which could potentially persist beyond the 
 > > test) and a delay (which slows testing overall and whose duration likely 
 > > depends on the hardware the test runs on).  Without the delay, I could not 
 > > get the callback to execute within the test.  It would execute when 
 > > running manually in an Org buffer, however.  I'm not sure why. 
 > 
 > Doesn't (while ... (sleep-for ...)) work?

I'm afraid I don't follow what you mean.

My biggest concern is the sleep-for itself.  Aside from the concerns above, the 
test doesn't work without it. 

For example, the check made by the advice looks for "1\n2\n".  I've changed the 
source block from "echo 1" to "echo 2"  to induce a failure (it will now 
receive "2\n2\n").  I re-evaluate the test and call M-: (ert 
"test-ob-shell/session-async-evaluation").  The test passes!  I can put a 
message or debug in the advice and see that it's never called.  However, if I 
uncomment the sleep-for, the advice runs, and the test fails as expected.

(ert-deftest test-ob-shell/session-async-evaluation ()
  (let ((session-name "test-ob-shell/session-async-evaluation")
(kill-buffer-query-functions nil)
result)
;; check callback return for correct results
(advice-add
 'ob-shell-async-chunk-callback
 :filter-return
 (lambda ( r)
   (let ((result (car r)))
 (should (string= result "1\n2\n"))  ; this was previously an if 
statement
 result))
 `((name . ,session-name)))
;; always remove advice, regardless of test outcome
(unwind-protect
(org-test-with-temp-text
(concat "#+begin_src sh :session " session-name " :async t
echo 2
echo 2
#+end_src")
  ;; execute block; callback only executes when delay
  (setq result (org-trim (org-babel-execute-src-block)))
  (if (should
   (and 
(string-match test-ob-shell/uuid-regex result)  ; block runs
(not (sleep-for 0.1))   ; callback 
doesn't fail
)) 
  (kill-buffer session-name)))
  (advice-remove 'ob-shell-async-chunk-callback session-name






Re: [PATCH] Async evaluation in ob-shell

2023-02-15 Thread Ihor Radchenko
Matt  writes:

> Checking the final result from the callback is trickier.   The following 
> works, but requires advice (which could potentially persist beyond the test) 
> and a delay (which slows testing overall and whose duration likely depends on 
> the hardware the test runs on).  Without the delay, I could not get the 
> callback to execute within the test.  It would execute when running manually 
> in an Org buffer, however.  I'm not sure why. 

Doesn't (while ... (sleep-for ...)) work?

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 



Re: [PATCH] Async evaluation in ob-shell

2023-02-12 Thread Matt


  On Sat, 11 Feb 2023 06:44:56 -0500  Ihor Radchenko  wrote --- 
 > 1. You should provide all the docstrings.
 > 2. I generally feel that separate async and separate session code are
 >either duplicating the same code or edge cases considered by session
 >code may popup in async code unexpectedly.

Excellent points.  Thank you.

As part of the commit, I want to include tests.

How to test an async block is non-obvious.  The initial evaluation returns a 
uuid which returns immediately and can be checked using a regex:

(defconst test-ob-shell/uuid-regex
  
"[0-9a-fA-F]\\{8\\}\\b-[0-9a-fA-F]\\{4\\}\\b-[0-9a-fA-F]\\{4\\}\\b-[0-9a-fA-F]\\{4\\}\\b-[0-9a-fA-F]\\{12\\}")

Technically, this is a ob-comint aspect and may be more rightly included in 
(the currently non-existant) tests for that module.

Checking the final result from the callback is trickier.   The following works, 
but requires advice (which could potentially persist beyond the test) and a 
delay (which slows testing overall and whose duration likely depends on the 
hardware the test runs on).  Without the delay, I could not get the callback to 
execute within the test.  It would execute when running manually in an Org 
buffer, however.  I'm not sure why. 

(ert-deftest test-ob-shell/session-async-evaluation ()
  (let ((session-name "test-ob-shell/session-async-evaluation")
(kill-buffer-query-functions nil)
result)
;; perform check after the callback executes which looks for the
;; expected result
(advice-add
 'ob-shell-async-chunk-callback
 :filter-return
 (lambda ( r)
   (let ((result (car r)))
 (if (not (string= result "1\n2\n"))
 (ert-fail (format "Expected 1\n2\n: %s" result)))
 result))
 `((name . ,session-name)))
;; always remove the advice, regardless of test outcome
(unwind-protect
(org-test-with-temp-text
(concat "#+begin_src sh :session " session-name " :async t
echo 1
echo 2
#+end_src")
  ;; execute the block; delay momentarily so that the callback
  ;; executes
  (setq result (org-trim (org-babel-execute-src-block)))
  (if (should
   (and
;; if the block runs...
(string-match
 test-ob-shell/uuid-regex
 result)
;; ...and the callback executes without fail
(not (sleep-for 0.1
  ;; clean up buffer on success
  (kill-buffer session-name)))
  (advice-remove 'ob-shell-async-chunk-callback session-name

This works for me using the last patch.

Thoughts?




Re: [PATCH] Async evaluation in ob-shell

2023-02-12 Thread Matt


  On Sat, 11 Feb 2023 15:56:00 -0500wrote --- 
 > org-babel-comint-async-filter is capable of taking a similar approach,
 > and reading/writing to tempfile.

There is some precedence in ob-shell for this.  Currently, the cmdline, stdin, 
and shebang headers use temp files.  It may be that implementing async versions 
of these could use this part of the async API.  However, cmdline, stdin, and 
shebang each use a temporary shell process rather than a dedicated comint, even 
when the :session header is present.  The async implementation requires a 
comint buffer.

 > I believe this approach would be
 > generally more robust, but the major weakness is that it would break if
 > you SSH'd to a remote host in the middle of the session.

Good point.

 > There was an interesting patch to ob-shell that was never applied, that
 > took the approach of wrapping the code block in a bash function before
 > executing; I think it might be a promising approach:
 > 
 > https://lists.gnu.org/archive/html/emacs-orgmode/2020-02/msg00923.html
 
This is interesting.  Thanks for sharing!  Instead of writing to a temp file, 
it could use a here document.  Here documents, AFAIK, are POSIX portable, 
although function definitions aren't.  Of course, if we're considering Windows 
cmd (as we did in a recent bug report), there are all sorts of problems: batch 
syntax (executed from a file) is different than CLI syntax, functions will 
implemented using labels (I'm not sure how robust that is), and no here 
documents.  A comint would probably be best for the Windows use-case.

ahab@pequod ~$ guix shell dash
ahab@pequod ~ [env]$ dash <<- EOF 
my_function() { echo "hello, world!"; echo "the end"; }
my_function;
EOF
hello, world!
the end

ahab@pequod ~$ guix shell fish
ahab@pequod ~ [env]$ fish <<- EOF
> function my_function
>   echo "hello, world!"
>   echo "the end"
> end
> my_function
> EOF
hello, world!
the end

 > By the way, I took a look at ob-shell for the first time in awhile, and
 > noticed that ob-shell now forces the prompt to be like:
 > 
 > org_babel_sh_prompt>
 > 
 > Which I think makes cleaning up the prompt markers a lot more
 > robust. But it is a bit ugly, and also seems to break working with shell
 > sessions started outside of Babel with M-x shell.

Is this something you consider a bug?



Re: [PATCH] Async evaluation in ob-shell

2023-02-11 Thread Ihor Radchenko
Matt  writes:

> It seems to me like 02-ob-shell-async-non-file.patch is all that's needed for 
> basic async in ob-shell.  I'm able to run long processes like `guix pull' and 
> `guix package -u' calls without issue and the results look like I expect.  
> Similarly for running a web server, such as `python3 -m http.server' and 
> killing it.  
>
> Unless there's something you or others think needs to be done, I can get it 
> committed (and try to write a test or two for it).

1. You should provide all the docstrings.
2. I generally feel that separate async and separate session code are
   either duplicating the same code or edge cases considered by session
   code may popup in async code unexpectedly.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 



Re: [PATCH] Async evaluation in ob-shell

2023-02-10 Thread Matt
  On Thu, 09 Feb 2023 06:23:42 -0500  Ihor Radchenko  wrote --- 
 > Could you please elaborate? What was the problem with
 > `buffer-substring'? I am asking because one of the previous versions of
 > `org-babel-comint-wait-for-output' relied upon 'face text property. See
 > a35d16368.

The problem is I got mixed up about the printed string representation and 
thought having properties changed how functions operated on the string.  The 
02-ob-shell-async-non-file.patch works fine with `buffer-substring'.  No need 
for the other patch.

It seems to me like 02-ob-shell-async-non-file.patch is all that's needed for 
basic async in ob-shell.  I'm able to run long processes like `guix pull' and 
`guix package -u' calls without issue and the results look like I expect.  
Similarly for running a web server, such as `python3 -m http.server' and 
killing it.  

Unless there's something you or others think needs to be done, I can get it 
committed (and try to write a test or two for it).



Re: [PATCH] Async evaluation in ob-shell

2023-02-09 Thread Ihor Radchenko
Matt  writes:

> I found cleaning the output was dramatically helped by calling 
> `buffer-substring-no-properties' instead of `buffer-substring' in 
> `org-babel-comint-async-filter'.  I'm not sure why `buffer-substring' was 
> originally used.  `make test' shows no failures, so I assume it doesn't make 
> a difference...?

Could you please elaborate? What was the problem with
`buffer-substring'? I am asking because one of the previous versions of
`org-babel-comint-wait-for-output' relied upon 'face text property. See
a35d16368.

>   On Tue, 07 Feb 2023 06:40:51 -0500  Ihor Radchenko  wrote --- 
>  > That's likely because of the same reasons why prompt did not get cleaned
>  > in synchronous blocks earlier. See `org-babel-comint-with-output'.
>
> That, my friend, was a wild ride.
>
> I'm curious about people's feelings on `org-babel-comint-with-output'.

My feelings are: WTF and how does it even work :)
More in https://yhetil.org/emacs-devel/87y1tgqhmc.fsf@localhost/
It is a mess, because comint.el is not designed for programmatic use.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 



Re: [PATCH] Async evaluation in ob-shell

2023-02-08 Thread Matt
I've attached two patches which replace the previous.

I found cleaning the output was dramatically helped by calling 
`buffer-substring-no-properties' instead of `buffer-substring' in 
`org-babel-comint-async-filter'.  I'm not sure why `buffer-substring' was 
originally used.  `make test' shows no failures, so I assume it doesn't make a 
difference...?

  On Tue, 07 Feb 2023 06:40:51 -0500  Ihor Radchenko  wrote --- 
 > That's likely because of the same reasons why prompt did not get cleaned
 > in synchronous blocks earlier. See `org-babel-comint-with-output'.

That, my friend, was a wild ride.

I'm curious about people's feelings on `org-babel-comint-with-output'.

Personally, I get the heebie-jeebies.  I can't shake feeling that there's a 
better way, especially since `org-babel-comint-async-filter' achieves similar 
ends.  My hunch is that other Babel languages may want async and that now would 
be a good time to consolidate the common functionalities of 
`org-babel-comint-with-output' and  `org-babel-comint-async-filter' .   Maybe 
even unify the API.  So far, `org-babel-comint-with-output' touches 9 languages 
and `org-babel-comint-async-filter' appears to touch 2 (soon to be 3).   I 
suspect those numbers will only grow.

I also can't shake the feeling that I might become ob-comint maintainer at some 
point...(not yet!).  I'm curious what people's thoughts are.






01-ob-shell-remove-properties-from-callback-string.patch
Description: Binary data


02-ob-shell-async-non-file.patch
Description: Binary data


Re: [PATCH] Async evaluation in ob-shell

2023-02-07 Thread Ihor Radchenko
Matt  writes:

> I'm excited to share that I've got async evaluation working (crudely) with 
> ob-shell.  A rough implementation is attached.  
>
> It has clear issues, such as the prompt being present in the output:

That's likely because of the same reasons why prompt did not get cleaned
in synchronous blocks earlier. See `org-babel-comint-with-output'.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 



[PATCH] Async evaluation in ob-shell

2023-02-06 Thread Matt
I'm excited to share that I've got async evaluation working (crudely) with 
ob-shell.  A rough implementation is attached.  

It has clear issues, such as the prompt being present in the output:

#+begin_src sh :session tester :async t
echo "By sending delimiters separately..."
sleep 3
slep 1
echo "typos don't cause problems--^"
#+end_src

#+RESULTS:
: org_babel_sh_prompt> By sending delimiters separately...
: org_babel_sh_prompt> org_babel_sh_prompt> sh: slep: command not found
: org_babel_sh_prompt> typos don't cause problems--^
: org_babel_sh_prompt>

It's not clear to me if that's something that a better regexp would handle or 
if it should be cleaned up in the callback function.  I'm still figuring out 
how it's done in ob-python and ob-R.

Any feedback or advice is welcome.




ob-shell-async-separate-calls.patch
Description: Binary data