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 +0000 Ihor Radchenko wrote --- > Matt <m...@excalamus.com> 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.
