[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] Async evaluation in ob-shell

From: Matt
Subject: Re: [PATCH] Async evaluation in ob-shell
Date: Fri, 03 Mar 2023 12:53:18 -0500
User-agent: Zoho Mail

 ---- 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 (100000000), 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.

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

reply via email to

[Prev in Thread] Current Thread [Next in Thread]