From: Matt <email@example.com>
To: "Ihor Radchenko" <firstname.lastname@example.org>
Cc: "emacs-orgmode" <email@example.com>,
Subject: Re: [PATCH] Async evaluation in ob-shell
Date: Thu, 09 Mar 2023 12:36:17 -0500 [thread overview]
Message-ID: <firstname.lastname@example.org> (raw)
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 email@example.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 +0000 Ihor Radchenko wrote ---
> Matt <firstname.lastname@example.org> 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.
+ (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.
next prev parent reply other threads:[~2023-03-09 17:37 UTC|newest]
Thread overview: 60+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-02-06 19:39 [PATCH] Async evaluation in ob-shell Matt
2023-02-07 11:40 ` Ihor Radchenko
2023-02-09 4:33 ` Matt
2023-02-09 11:24 ` Ihor Radchenko
2023-02-10 22:19 ` Matt
2023-02-11 11:44 ` Ihor Radchenko
2023-02-12 19:32 ` Matt
2023-02-15 15:08 ` Ihor Radchenko
2023-02-16 4:02 ` Matt
2023-02-17 10:44 ` Ihor Radchenko
2023-02-19 23:14 ` Matt
2023-02-20 11:24 ` Ihor Radchenko
2023-02-20 17:24 ` Matt
2023-02-22 10:30 ` Ihor Radchenko
2023-03-02 1:36 ` Matt
2023-03-03 14:52 ` Ihor Radchenko
2023-03-03 17:53 ` Matt
2023-03-05 12:15 ` Ihor Radchenko
2023-03-06 6:45 ` Matt
2023-03-07 12:45 ` Ihor Radchenko
2023-03-09 17:36 ` Matt [this message]
2023-03-10 1:52 ` Max Nikulin
2023-03-12 16:28 ` Jack Kamm
2023-03-18 10:48 ` Ihor Radchenko
2023-03-21 20:29 ` Matt
2023-03-22 12:12 ` Ihor Radchenko
2023-03-23 11:50 ` Ihor Radchenko
2023-03-23 19:35 ` Matt
2023-03-24 9:13 ` Ihor Radchenko
2023-03-28 2:53 ` Matt
2023-03-28 10:06 ` Ihor Radchenko
2023-04-17 15:31 ` Matt
2023-04-17 18:55 ` Ihor Radchenko
2023-04-17 18:56 ` Matt
2023-04-17 19:05 ` Ihor Radchenko
2023-03-23 3:25 ` [SUGGESTION] ob-shell async result output should not contains shell prompt Christopher M. Miles
2023-03-23 4:21 ` Matt
2023-03-23 11:12 ` Christopher M. Miles
2023-03-23 16:23 ` Matt
2023-03-24 11:20 ` Ihor Radchenko
2023-03-23 16:26 ` Remove "shell" as a supported Babel language within ob-shell.el (was Re: [SUGGESTION] ob-shell async result output should not contains shell prompt) Matt
2023-03-24 1:53 ` Remove "shell" as a supported Babel language within ob-shell.el Christopher M. Miles
2023-03-24 11:38 ` Remove "shell" as a supported Babel language within ob-shell.el (was Re: [SUGGESTION] ob-shell async result output should not contains shell prompt) Ihor Radchenko
2023-03-25 5:47 ` Samuel Wales
2023-03-25 18:07 ` Ihor Radchenko
2023-03-28 2:33 ` Matt
2023-02-11 20:56 ` [PATCH] Async evaluation in ob-shell jackkamm
2023-02-12 19:02 ` Matt
2023-02-13 3:16 ` Jack Kamm
2023-02-13 17:07 ` [BUG] shell sessions started outside of Babel broken Matt
2023-02-15 6:19 ` Jack Kamm
2023-02-16 12:53 ` Ihor Radchenko
2023-02-19 15:04 ` Jack Kamm
2023-02-20 11:22 ` Ihor Radchenko
2023-02-21 5:23 ` Jack Kamm
2023-02-22 10:38 ` Ihor Radchenko
2023-03-25 16:55 ` Jack Kamm
2023-03-25 16:59 ` [PATCH] Fix externally started sessions with ob-python Jack Kamm
2023-02-13 20:11 ` [BUG] conda doesn't work in ob-shell sessions Matt
2023-02-15 6:21 ` Jack Kamm
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
List information: https://www.orgmode.org/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).