[-- Attachment #1: Type: text/plain, Size: 748 bytes --] Hi Org Maintainers, Please see the attached patch to remove "Prelude> " and "Prelude| " line continuations from the block result output when parsing blocks that contain multi-line function declarations. This likely requires yesterday's patch to return value-type results from Haskell blocks. This patch applies cleanly against the org-mode master. #+name: chain-ecm #+BEGIN_SRC haskell :{ chain :: (Integral a) => a -> [a] chain 1 = [1] chain n | even n = n:chain (n `div` 2) | odd n = n:chain (n*3 + 1) :} chain 10 #+END_SRC Results without patch: : Prelude| Prelude| Prelude| Prelude| Prelude| Prelude| Prelude> [10,5,16,8,4,2,1] Results with patch: : | 10 | 5 | 16 | 8 | 4 | 2 | 1 | Thank you for your time, Nick [-- Attachment #2: ob-haskell-trim-prelude.diff --] [-- Type: text/x-patch, Size: 783 bytes --] --- lisp/ob-haskell.el +++ lisp/ob-haskell.el @@ -84,9 +84,11 @@ (reverse (mapcar #'org-trim raw))))))) (org-babel-reassemble-table (let ((result - (pcase result-type - (`output (mapconcat #'identity (reverse results) "\n")) - (`value (car results))))) + (replace-regexp-in-string + "Prelude[|>] " "" + (pcase result-type + (`output (mapconcat #'identity (reverse results) "\n")) + (`value (car results)))))) (org-babel-result-cond (cdr (assq :result-params params)) result (if (stringp result) (org-babel-script-escape result)))) (org-babel-pick-name (cdr (assq :colname-names params)) Diff finished. Sun May 17 14:26:21 2020
[-- Attachment #1: Type: text/plain, Size: 1031 bytes --] Hi Org Maintainers, Attached is an updated patch that makes output trimming work with blocks that do and don't produce results. The old patch creates a =let: Wrong type argument: arrayp, nil= error when evaluating blocks that don't produce output. This necessarily incorporates yesterday's patch. Thanks for your time, Nick Multi-line function declarations with output still work fine. #+BEGIN_SRC haskell :{ chain :: (Integral a) => a -> [a] chain 1 = [1] chain n | even n = n:chain (n `div` 2) | odd n = n:chain (n*3 + 1) :} chain 10 #+END_SRC #+RESULTS: | 10 | 5 | 16 | 8 | 4 | 2 | 1 | Silent declaration-only blocks correctly evaluate silently. #+BEGIN_SRC haskell :results silent :{ flip' :: (a -> b -> c) -> (b -> a -> c) flip' f = \x y -> f y x :} #+END_SRC Single-line function calls also return the expected results. #+name: flip'-hello #+BEGIN_SRC haskell flip' zip [1,2,3,4,5,6] "hello" #+END_SRC #+RESULTS: flip'-hello | h | 1 | | e | 2 | | l | 3 | | l | 4 | | o | 5 | [-- Attachment #2: ob-haskell-trim-prelude-2.diff --] [-- Type: text/x-patch, Size: 1031 bytes --] diff --git a/lisp/ob-haskell.el b/lisp/ob-haskell.el index bea162528..cb581fe3b 100644 --- a/lisp/ob-haskell.el +++ b/lisp/ob-haskell.el @@ -83,12 +83,16 @@ (cdr (member org-babel-haskell-eoe (reverse (mapcar #'org-trim raw))))))) (org-babel-reassemble-table - (let ((result + (let* ((result (pcase result-type (`output (mapconcat #'identity (reverse results) "\n")) - (`value (car results))))) + (`value (car results)))) + (result + (if (stringp result) + (replace-regexp-in-string "Prelude[|>] " "" result) + result))) (org-babel-result-cond (cdr (assq :result-params params)) - result (org-babel-script-escape result))) + result (if (stringp result) (org-babel-script-escape result)))) (org-babel-pick-name (cdr (assq :colname-names params)) (cdr (assq :colname-names params))) (org-babel-pick-name (cdr (assq :rowname-names params))
Nick Daly writes: > Attached is an updated patch that makes output trimming work with > blocks that do and don't produce results. The old patch creates a > =let: Wrong type argument: arrayp, nil= error when evaluating blocks > that don't produce output. This necessarily incorporates yesterday's > patch. Thanks for the patch. > diff --git a/lisp/ob-haskell.el b/lisp/ob-haskell.el > index bea162528..cb581fe3b 100644 > --- a/lisp/ob-haskell.el > +++ b/lisp/ob-haskell.el > @@ -83,12 +83,16 @@ > (cdr (member org-babel-haskell-eoe > (reverse (mapcar #'org-trim raw))))))) > (org-babel-reassemble-table > - (let ((result > + (let* ((result > (pcase result-type > (`output (mapconcat #'identity (reverse results) "\n")) > - (`value (car results))))) > + (`value (car results)))) > + (result > + (if (stringp result) > + (replace-regexp-in-string "Prelude[|>] " "" result) > + result))) Oy, it's pretty nasty that those leak through. I know ob-python (and probably other languages) also suffers from similar brittleness. It'd be nice of course to figure out how to prevent the prompts leakage in the first place, but, short of that, I think we should at least make the regexp stricter so that it matches just the start of the string. And that (stringp result) check is for the same reason as the one from your first patch from yesterday which is now included ... > (org-babel-result-cond (cdr (assq :result-params params)) > - result (org-babel-script-escape result))) > + result (if (stringp result) (org-babel-script-escape result)))) ... here. I believe result is nil in the problematic case, so this could be (and result (org-babel-script-escape result)) However, based on stepping through the example in your patch from yesterday, I think these two issues might be more closely related than you realize. In the (cdr (member org-babel-haskell-eoe (reverse (mapcar #'org-trim raw)))) bit visible as a context line above, this is what I see for raw when I step through org-babel-execute:haskell: ("Prelude| Prelude| Prelude| Prelude> \"org-babel-haskell-eoe\"" "") So it looks like the member call above is returning nil because the prompt markers are corrupting the element. If that's the case, it seems like the output cleansing should happen upstream of that call. What do you think?
[-- Attachment #1: Type: text/plain, Size: 2007 bytes --] Hi Kyle, thanks for the thoughtful analysis. On Wed, May 20, 2020 at 12:51 AM Kyle Meyer <kyle@kyleam.com> wrote: > So it looks like the member call above is returning nil because the > prompt markers are corrupting the element. If that's the case, it seems > like the output cleansing should happen upstream of that call. > > What do you think? After a bit of tinkering, I realized there are two things going on here, only one of which I fully understand: 1. My core functional issue is that =comint-prompt-regexp= isn't set up to handle the "Prelude| " entries or the repeated prompts. The other patches I submitted were unnecessary. 2. The =comint-prompt-regexp= gets default values from somewhere I don't understand and can't find with a quick source grep. In ob-haskell, we set =comint-prompt-regexp= to the (undefined) haskell-prompt plus "or optional-lambda": (defvar haskell-prompt-regexp) (defun org-babel-execute:haskell (body params) ... (setq-local comint-prompt-regexp (concat haskell-prompt-regexp "\\|^λ?> ")))) That causes an evaluation error that prevents the first source block evaluation but, strangely, that also results in this mess in the *haskell* buffer on subsequent evaluations: : "^\\*?[[:upper:]][\\._[:alnum:]]*\\(?: \\*?[[:upper:]][\\._[:alnum:]]*\\)*\\( λ\\)?> " =comint-prompt-regexp='s variable documentation calls out much simpler regexps that do basically the same thing as the one above and handles the repeated "Prelude| " entries. This one is based off the Canonical Lisp example: : "^[^>\n]+\\(> \\)?" I've attached a patch against git master that results in fewer undefined variable errors and depends less default-variable magic. Now, =haskell-prompt-regexp= and =comint-prompt-regexp= are explicitly set using defaults that can be M-x customized, and the default value handles the repeated "Prelude| " entries without breaking the original "λ> " prompt handling. Thanks, Nick [-- Attachment #2: ob-haskell-trim-prelude-3.diff --] [-- Type: text/x-patch, Size: 988 bytes --] diff --git a/lisp/ob-haskell.el b/lisp/ob-haskell.el index bea162528..893e4220c 100644 --- a/lisp/ob-haskell.el +++ b/lisp/ob-haskell.el @@ -56,15 +56,25 @@ (defvar org-babel-haskell-eoe "\"org-babel-haskell-eoe\"") -(defvar haskell-prompt-regexp) +(defvar haskell-prompt-regexp "^[^>\n]*\\(> \\)?" + "Filter out prompts from Haskell interpreters: + +GHC: + +- '^Prelude> ' +- '^Prelude| Prelude| Prelude> ' + +Unknown Interpreter: + +- '^> ' +- '^λ> '") (defun org-babel-execute:haskell (body params) "Execute a block of Haskell code." (require 'inf-haskell) (add-hook 'inferior-haskell-hook (lambda () - (setq-local comint-prompt-regexp - (concat haskell-prompt-regexp "\\|^λ?> ")))) + (setq-local comint-prompt-regexp haskell-prompt-regexp))) (let* ((session (cdr (assq :session params))) (result-type (cdr (assq :result-type params))) (full-body (org-babel-expand-body:generic
[-- Attachment #1: Type: text/plain, Size: 1558 bytes --] Apologies, one last patch. On Sat, May 23, 2020 at 7:02 PM Nick Daly <nick.m.daly@gmail.com> wrote: > : "^\\*?[[:upper:]][\\._[:alnum:]]*\\(?: > \\*?[[:upper:]][\\._[:alnum:]]*\\)*\\( λ\\)?> " > > =comint-prompt-regexp='s variable documentation calls out much simpler > regexps > > : "^[^>]+\\(> \\)?" This simplified patch breaks one case that I'd forgotten about: the true one-liner, where the output displays before the "Prelude> " prompt even appears. #+BEGIN_SRC haskell scanl (+) 0 [1,2,3,4] #+END_SRC #+BEGIN_EXAMPLE Prelude> scanl (+) 0 [1,2,3,4] "org-babel-haskell-eoe" [0,1,3,6,10] Prelude> "org-babel-haskell-eoe" Prelude> #+END_EXAMPLE This latest patch updates the original (more complicated) regexp that works with this out-of-order output. This should display the expected result in all known cases: One liners: #+BEGIN_SRC haskell scanl (+) 0 [1,2,3,4] #+END_SRC #+RESULTS: | 0 | 1 | 3 | 6 | 10 | Silent multi-line blocks: #+BEGIN_SRC haskell :results silent :{ flip' :: (a -> b -> c) -> (b -> a -> c) flip' f = \x y -> f y x :} #+END_SRC Multi-line blocks with value results: #+BEGIN_SRC haskell :{ sum' :: (Num a) => [a] -> a sum' xs = foldl (\ acc x -> acc + x) 0 xs :} sum' [1,2,3,4] == 10 #+END_SRC #+RESULTS: : True Multi-line blocks with output results: #+BEGIN_SRC haskell :results output :{ sum' :: (Num a) => [a] -> a sum' xs = foldl (\ acc x -> acc + x) 0 xs :} print "hi" #+END_SRC #+RESULTS: : : hi Thanks again for your time, Nick [-- Attachment #2: ob-haskell-trim-prelude-4.diff --] [-- Type: text/x-patch, Size: 1116 bytes --] diff --git a/lisp/ob-haskell.el b/lisp/ob-haskell.el index bea162528..6ac34f2f5 100644 --- a/lisp/ob-haskell.el +++ b/lisp/ob-haskell.el @@ -56,15 +56,27 @@ (defvar org-babel-haskell-eoe "\"org-babel-haskell-eoe\"") -(defvar haskell-prompt-regexp) +(defvar haskell-prompt-regexp "^\\(\\*?[[:upper:]][\\._[:alnum:]]*\\(?: \\*?[[:upper:]][\\._[:alnum:]]*\\)*\\( λ\\)?[|>] \\)*" + "Filter out prompts from Haskell interpreters: + +GHC: + +- 'output + ^Prelude> EOE' +- '^Prelude> output EOE' +- '^Prelude| Prelude| Prelude> output EOE' + +Unknown Interpreter: + +- '^> ' +- '^λ> '") (defun org-babel-execute:haskell (body params) "Execute a block of Haskell code." (require 'inf-haskell) (add-hook 'inferior-haskell-hook (lambda () - (setq-local comint-prompt-regexp - (concat haskell-prompt-regexp "\\|^λ?> ")))) + (setq-local comint-prompt-regexp haskell-prompt-regexp))) (let* ((session (cdr (assq :session params))) (result-type (cdr (assq :result-type params))) (full-body (org-babel-expand-body:generic
Nick Daly writes: > After a bit of tinkering, I realized there are two things going on > here, only one of which I fully understand: > > 1. My core functional issue is that =comint-prompt-regexp= isn't set > up to handle the "Prelude| " entries or the repeated prompts. The > other patches I submitted were unnecessary. > > 2. The =comint-prompt-regexp= gets default values from somewhere I > don't understand and can't find with a quick source grep. Here's what I can gather. inf-haskell used to set comint-prompt-regexp in the body of inferior-haskell-mode. Here's an example from 11d6abf (2017-08-24): (setq-local comint-prompt-regexp ;; Why the backslash in [\\._[:alnum:]]? "^\\*?[[:upper:]][\\._[:alnum:]]*\\(?: \\*?[[:upper:]][\\._[:alnum:]]*\\)*\\( λ\\)?> \\|^λ?> $") In ca94d81 (revamped inf-haskell, 2017-08-26), which was included in the v17.1 release, haskell-prompt-regexp was introduced and the line above is now (setq-local comint-prompt-regexp haskell-prompt-regexp) > In ob-haskell, we set =comint-prompt-regexp= to the (undefined) > haskell-prompt plus "or optional-lambda": With a haskell-mode after the commit I point to above, it shouldn't be undefined at the time we set comint-prompt-regexp because org-babel-interpret-haskell loads inf-haskell before that. However, I'm confused why Org's b46787743 (Fix ob-haskell.el to work with custom ghci prompts, 2017-12-02) added the λ bit (author of that patch cc'd). As far as I can tell, that is a part of inferior-haskell-mode's default comint-prompt-regexp and has been since 28997b2 (Add support for popular "λ> " prompt to inf-haskell, 2013-07-04). You've sent an updated patch in a follow-up message, so I'll continue there...
Nick Daly writes: > On Sat, May 23, 2020 at 7:02 PM Nick Daly <nick.m.daly@gmail.com> wrote: >> : "^\\*?[[:upper:]][\\._[:alnum:]]*\\(?: >> \\*?[[:upper:]][\\._[:alnum:]]*\\)*\\( λ\\)?> " >> >> =comint-prompt-regexp='s variable documentation calls out much simpler >> regexps >> >> : "^[^>]+\\(> \\)?" > > This simplified patch breaks one case that I'd forgotten about: the > true one-liner, where the output displays before the "Prelude> " > prompt even appears. > > #+BEGIN_SRC haskell > [... several examples ... ] Thanks for the nice examples. It'd be great to eventually include at least some of them as tests. > diff --git a/lisp/ob-haskell.el b/lisp/ob-haskell.el > index bea162528..6ac34f2f5 100644 > --- a/lisp/ob-haskell.el > +++ b/lisp/ob-haskell.el > @@ -56,15 +56,27 @@ For the next iteration, could you send a patch generated by git-format-patch? See <https://orgmode.org/worg/org-contribute.html>. Please also consider signing copyright papers, but I suspect the ob-haskell.el changes will end up being few enough lines that they can be accepted as a TINYCHANGE. > (defvar org-babel-haskell-eoe "\"org-babel-haskell-eoe\"") > > -(defvar haskell-prompt-regexp) > +(defvar haskell-prompt-regexp "^\\(\\*?[[:upper:]][\\._[:alnum:]]*\\(?: \\*?[[:upper:]][\\._[:alnum:]]*\\)*\\( λ\\)?[|>] \\)*" > + "Filter out prompts from Haskell interpreters: > + > +GHC: > + > +- 'output > + ^Prelude> EOE' > +- '^Prelude> output EOE' > +- '^Prelude| Prelude| Prelude> output EOE' > + > +Unknown Interpreter: > + > +- '^> ' > +- '^λ> '") This is inf-haskell's variable. ob-haskell shouldn't set it. The original (defvar haskell-prompt-regexp) just silenced the bytecompiler. If ob-haskell needs to tweak comint-prompt-regexp to deal with the "^Prelude| Prelude| Prelude" above, it should done without overwriting an inf-haskell value. As touched on in my other email, there's also the issue of compatibility with versions of haskell mode before v17.1 that lack a haskell-prompt-regexp. I believe this is why you're seeing the undefined errors. So, I dunno. I'm not a ob-haskell user or a really a Babel user. But in my view it'd be cleaner to just leave inferior-haskell-mode's comint-prompt-regexp alone and strip the "Prelude| Prelude| ..." once the output comes out of org-babel-comint-with-output. It's hacky and error-prone, but I don't think it's so different than what org-babel-comint-with-output already does.
[-- Attachment #1: Type: text/plain, Size: 1662 bytes --] Hi Kyle, thank you again for your help and analysis. In summary, org-mode needs no patches, but inf-haskell's comint-prompt-regexp needs updates. I'll follow up with the inf-haskell maintainers. ---- > inf-haskell used to set comint-prompt-regexp in the body of > inferior-haskell-mode... This is inf-haskell's variable. > ob-haskell shouldn't set it. Thank you for your help in all this. I couldn't find where the regex was set in the org-mode source because it wasn't coming from org-mode at all. Putting this all together suggests that the correct fix is to update the inf-haskell regex to handle "Prelude| " correctly in the first place. I believe it's possible to compress the current inf-haskell regexp from this: (setq-local comint-prompt-regexp ;; Why the backslash in [\\._[:alnum:]]? "^\\*?[[:upper:]][\\._[:alnum:]]*\\(?: \\*?[[:upper:]][\\._[:alnum:]]*\\)*\\( λ\\)?> \\|^λ?> $") Down into this, without any significant loss of fidelity. We really don't care about any characters at the start of the line before the final "> ". (setq-local comint-prompt-regexp "^[[:alnum:].*_() |λ]*> ") This seems useful because, as I discovered this morning, importing modules mangles the prompt further: Prelude> import Data.Time Prelude Data.Time> :m + Data.Time.Clock Prelude Data.Time Data.Time.Clock> The testing data that this configuration correctly parsed is attached for reference. I'll figure out where to submit that patch to, and send it off. It's a single line change that shouldn't require any assignment papers. Thanks again, Nick [-- Attachment #2: ghc-prelude-prompt-testing.txt --] [-- Type: text/plain, Size: 1257 bytes --] Prelude> import Data.Time Prelude Data.Time> :m + Data.Time.Clock Prelude Data.Time Data.Time.Clock> scanl (+) 0 [1,2,3,4] "org-babel-haskell-eoe" [0,1,3,6,10] Prelude Data.Time Data.Time.Clock> "org-babel-haskell-eoe" Prelude Data.Time Data.Time.Clock> :{ flip' :: (a -> b -> c) -> (b -> a -> c) flip' f = \x y -> f y x :} "org-babel-haskell-eoe" Prelude Data.Time Data.Time.Clock| Prelude Data.Time Data.Time.Clock| Prelude Data.Time Data.Time.Clock| Prelude Data.Time Data.Time.Clock> "org-babel-haskell-eoe" Prelude Data.Time Data.Time.Clock> :{ sum' :: (Num a) => [a] -> a sum' xs = foldl (\ acc x -> acc + x) 0 xs :} sum' [1,2,3,4] == 10 "org-babel-haskell-eoe" Prelude Data.Time Data.Time.Clock| Prelude Data.Time Data.Time.Clock| Prelude Data.Time Data.Time.Clock| Prelude Data.Time Data.Time.Clock> True Prelude Data.Time Data.Time.Clock> "org-babel-haskell-eoe" Prelude Data.Time Data.Time.Clock> :{ sum' :: (Num a) => [a] -> a sum' xs = foldl (\ acc x -> acc + x) 0 xs :} print "hi" "org-babel-haskell-eoe" Prelude Data.Time Data.Time.Clock| Prelude Data.Time Data.Time.Clock| Prelude Data.Time Data.Time.Clock| Prelude Data.Time Data.Time.Clock> "hi" Prelude Data.Time Data.Time.Clock> "org-babel-haskell-eoe" Prelude Data.Time Data.Time.Clock>