Re: [PATCH] Add tests for ob-haskell (GHCi)

[Bruno Barbier]

Ihor Radchenko  writes:

> Bruno Barbier  writes:
>
>> From 86a5443948fc84a6a412ccf49d0c537608f465a7 Mon Sep 17 00:00:00 2001
>> From: Bruno BARBIER 
>> Date: Fri, 18 Nov 2022 20:14:20 +0100
>> Subject: [PATCH 1/7] ob-haskell: Add tests for GHCi
>> ...
>
> Thanks!
> Applied, onto main.
> I also added a missing defvar declaration for
> `inferior-haskell-root-dir'.

Thanks Ihor!

Re: [PATCH] Add tests for ob-haskell (GHCi)

[Ihor Radchenko]

Bruno Barbier  writes:

> From 86a5443948fc84a6a412ccf49d0c537608f465a7 Mon Sep 17 00:00:00 2001
> From: Bruno BARBIER 
> Date: Fri, 18 Nov 2022 20:14:20 +0100
> Subject: [PATCH 1/7] ob-haskell: Add tests for GHCi
> ...

Thanks!
Applied, onto main.
I also added a missing defvar declaration for
`inferior-haskell-root-dir'.
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=91c52e2ab
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=5bffb9a1e
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=d6dd92d44
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=b85217217
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=924c2dd83
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=36a786f7f
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=3674e4024
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=eed3ac9f7

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

Re: [PATCH] Add tests for ob-haskell (GHCi)

[Bruno Barbier]

Hi Ihor,

Sorry for the delay, thanks again for the ping.



Ihor Radchenko  writes:

> Bruno Barbier  writes:
>
 +  (when (bufferp "*haskell*") (error "Conflicting buffer 
 '*haskell*', rename it or kill it."))
 +  (with-current-buffer session (rename-buffer "*haskell*")))
>>>
>>> So, you are now renaming the unique session buffer back to "*haskell*".
>>> And never rename it back to expected :session . Users might be 
>>> confused.
>>
>> I do rename it back once inf-haskell has initialized the buffer (after
>> run-haskell in the last version).
>
> A comment would help to clarify things for the readers.

Right. I've improved the comment. Thanks.


 +(save-window-excursion
 +  ;; We don't use `run-haskell' to not popup the buffer.
 +  ;; And we protect default-directory.
 +  (let ((default-directory default-directory))
 +(inferior-haskell-start-process))
>>>




>> About 'default-directory', I'm not sure. Maybe the side effect is done
>> on purpose in inf-haskell.
>
> I can see the haskell-mode overrides default-directory with
> `inferior-haskell-root-dir', running ghci in that directory, if it is
> non-nil. Even with your let binding, it is calling for trouble when
> source block uses :dir header argument.
>
> Maybe we can bind `inferior-haskell-root-dir' to `default-directory'
> instead? `default-directory' is modified according to :dir by ob-core.el
> when necessary.

You may be right that we should use `inferior-haskell-root-dir' to
tell haskell-mode where to run the interpreter.  Done.



>> The function 'putStr' output the string without a newline on stdout
>> (as opposed to the function putStrLn that does add a newline).
>>
>> So, in GHCi, entering:
>>
>> putStr("4")
>> 
>> outputs "4" on stdout, then GHCi outputs the prompt, so we get:
>>
>> 4ghci> 
>>
>> In the end, 'org-babel-comint-with-output' gets this
>> 1ghci> 2ghci> 3ghci> 
>> ghci> org-babel-haskell-eoe
>> ghci> ghci>
>> 
>> and filters out everything as being GHCi prompts and the EOE.
>>
>> I'm not really expecting this to be fixed; I just wanted to record the
>> fact.
>
> We actually might be able to deal with this if we change the prompt and
> update comint-prompt-regexp to something more accurate.

I couldn't make it work by simply restricting the prompt.  I think
it's OK if character by character output doesn't work; ghci is about
line based interaction afterall.



 Subject: [PATCH 11/13] lisp/ob-haskell.el: Fix how to use sessions

 +  (org-babel-haskell-with-session
>>>
>>> This kind of names are usually dedicated to macro calls. But
>>> `org-babel-haskell-with-session' is instead a function. I think a macro
>>> will be better. And you will be able to get rid of unnecessary lambda.
>>
>> That looks kind of complicated just to avoid one lambda in one call.
>> But, as I couldn't find a better name, I've translated it into a
>> macro.
>
> I think you misunderstood what I meant.
> See the attached diff on top of your patches that simplifies things a
> bit.
> diff --git a/lisp/ob-haskell.el b/lisp/ob-haskell.el
...
>  (defmacro org-babel-haskell-with-session (session-symbol params  body)
>"Get the session identified by PARAMS and run BODY with it.
.. 
> +  `(let* ((params ,params)
> +  (sn (cdr (assq :session params)))
> +  (session (org-babel-haskell-initiate-session sn params))
> +  (,session-symbol session)
> +  (one-shot (equal sn "none")))
> + (unwind-protect
> + (progn ,@body)

I don't think it's correct to create local variables like this in a
macro: we need to use uninterned symbols, else we may capture caller
variables (params, sn, session and one-shot).

I personnaly find it easier when I keep my macros as short as
possible, and, to do any non-trivial work in a function: easier to
read, to modify and to debug.




 -(let ((buffer (org-babel-haskell-initiate-session session)))
 +(let ((buffer (org-babel-haskell-initiate-session session params)))
>>>
>>> PARAMS argument is ignored by `org-babel-haskell-initiate-session'. I am
>>> not sure why you are trying to pass it here.
>>
>> We have the PARAMS, and, org-babel-haskell-initiate-session has a
>> PARAMS arguments. So, at the API level, I think it's better to
>> propagate it than to ignore it.  But you're right that, today, the
>> current implementation ignores it.
>>
>> I'm fine with dropping that change if you so prefer.
>
> I am mostly neutral here. Slightly in favour of keeping things unchanged.

I've removed my change. The function `org-babel-load-session:haskell`
now ignores the parameter PARAMS, as before.


Thanks for the review.


Bruno

>From 86a5443948fc84a6a412ccf49d0c537608f465a7 Mon Sep 17 00:00:00 2001
From: Bruno BARBIER 
Date: Fri, 18 Nov 2022 20:14:20 +0100
Subject: [PATCH 1/7] ob-haskell: Add tests for GHCi

Re: [PATCH] Add tests for ob-haskell (GHCi)

[Bruno Barbier]

Hi Ihor,

Ihor Radchenko  writes:

> A few months have passed since the last activity in this thread.
> Bruno, may I know if you are still interested to work on the patch?

Thanks for the reminder.  I'm definitely interested to closing that
thread.

I'll review my last changes and send my updated patch.

Thank you for your patience,

Bruno.

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

Re: [PATCH] Add tests for ob-haskell (GHCi)

[Ihor Radchenko]

A few months have passed since the last activity in this thread.
Bruno, may I know if you are still interested to work on the patch?

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

Re: [PATCH] Add tests for ob-haskell (GHCi)

[Ihor Radchenko]

Bruno Barbier  writes:

>> I can see that you limited the tests scope to :session blocks.
>> Would it be possible to extend the existing tests to :compile yes case?
>> From a glance, it does not look like you need to change much - Haskell
>> behaviour should be similar with or without ghci.
>
> Except for one line expressions, GHCi inputs and haskell modules are
> two different things.  I doubt there will be much to share; that's why
> I've focused from the start on GHCi only.

Fair point. I thought that the differences are less significant.

>>> +(sleep-for 0.25)
>>> +;; Disable secondary prompt.
>>
>> It would be useful to explain the purpose of disabling the secondary
>> prompt in the source code comment itself, not just in the commit
>> message. It will improve readability.
>
> Are you reviewing your own improvements ? :-)

Sure. Why not :3 Nobody guaranteed that my code is free of errors.

> Fixed. I've copied/pasted your explanation in the code.

Thanks!

>> Also, session may be a killed buffer object. It is still a buffer, but
>> not usable. See `buffer-live-p'.
>
> By construction, if 'session' is a buffer, then, it is a live buffer.

You are probably right. Even though killed buffer objects may appear in
Elisp (28.10 Killing Buffers), their buffer names should be nil, and thus
they cannot be returned by `get-buffer'.

>>> +  (when (bufferp "*haskell*") (error "Conflicting buffer 
>>> '*haskell*', rename it or kill it."))
>>> +  (with-current-buffer session (rename-buffer "*haskell*")))
>>
>> So, you are now renaming the unique session buffer back to "*haskell*".
>> And never rename it back to expected :session . Users might be 
>> confused.
>
> I do rename it back once inf-haskell has initialized the buffer (after
> run-haskell in the last version).

A comment would help to clarify things for the readers.

>>> +(save-window-excursion
>>> +  ;; We don't use `run-haskell' to not popup the buffer.
>>> +  ;; And we protect default-directory.
>>> +  (let ((default-directory default-directory))
>>> +(inferior-haskell-start-process))
>>
>> This is a workaround for a nasty side effect of running
>> `inferior-haskell-start-process'. We should report this to haskell-mode
>> developers, leaving appropriate comment in the code.
>
> About 'run-haskell', I reverted my change: we're inside a
> 'save-window-excursion', which looks like the standard way to get rid
> of unwanted popups (like ob-shell does).
>
> About 'default-directory', I'm not sure. Maybe the side effect is done
> on purpose in inf-haskell.

I can see the haskell-mode overrides default-directory with
`inferior-haskell-root-dir', running ghci in that directory, if it is
non-nil. Even with your let binding, it is calling for trouble when
source block uses :dir header argument.

Maybe we can bind `inferior-haskell-root-dir' to `default-directory'
instead? `default-directory' is modified according to :dir by ob-core.el
when necessary.

> The function 'putStr' output the string without a newline on stdout
> (as opposed to the function putStrLn that does add a newline).
>
> So, in GHCi, entering:
>
> putStr("4")
> 
> outputs "4" on stdout, then GHCi outputs the prompt, so we get:
>
> 4ghci> 
>
> In the end, 'org-babel-comint-with-output' gets this
> 1ghci> 2ghci> 3ghci> 
> ghci> org-babel-haskell-eoe
> ghci> ghci>
> 
> and filters out everything as being GHCi prompts and the EOE.
>
> I'm not really expecting this to be fixed; I just wanted to record the
> fact.

We actually might be able to deal with this if we change the prompt and
update comint-prompt-regexp to something more accurate.

>>> Subject: [PATCH 11/13] lisp/ob-haskell.el: Fix how to use sessions
>>>
>>> +  (org-babel-haskell-with-session
>>
>> This kind of names are usually dedicated to macro calls. But
>> `org-babel-haskell-with-session' is instead a function. I think a macro
>> will be better. And you will be able to get rid of unnecessary lambda.
>
> That looks kind of complicated just to avoid one lambda in one call.
> But, as I couldn't find a better name, I've translated it into a
> macro.

I think you misunderstood what I meant.
See the attached diff on top of your patches that simplifies things a
bit.

diff --git a/lisp/ob-haskell.el b/lisp/ob-haskell.el
index cd930266c..a8240d02b 100644
--- a/lisp/ob-haskell.el
+++ b/lisp/ob-haskell.el
@@ -77,31 +77,28 @@ (defcustom org-babel-haskell-compiler "ghc"
 (defconst org-babel-header-args:haskell '((compile . :any))
   "Haskell-specific header arguments.")
 
-
-(defun org-babel-haskell-with-session--worker (params todo)
-  "See `org-babel-haskell-with-session'."
-  (let* ((sn (cdr (assq :session params)))
- (session (org-babel-haskell-initiate-session sn params))
- (one-shot (equal sn "none")))
-(unwind-protect
-(funcall todo session)
-  (when (and one-shot (buffer-live-p 

Re: [PATCH] Add tests for ob-haskell (GHCi)

[Bruno Barbier]

Hi Ihor,

Thanks for the review.


Ihor Radchenko  writes:
> Bruno Barbier  writes:


> I can see that you limited the tests scope to :session blocks.
> Would it be possible to extend the existing tests to :compile yes case?
> From a glance, it does not look like you need to change much - Haskell
> behaviour should be similar with or without ghci.


Except for one line expressions, GHCi inputs and haskell modules are
two different things.  I doubt there will be much to share; that's why
I've focused from the start on GHCi only.

As I've a lot of other things that I'd like to do to improve my day to
day workflow, and as I'm barely using ob-haskell, I can't promise I'll
work on this any time soon.


>> From 9972b926f55cb970e0b520f8726a3684118017b6 Mon Sep 17 00:00:00 2001
>> From: Ihor Radchenko 
>> Date: Fri, 24 Mar 2023 11:20:22 +0100
>> Subject: [PATCH 02/13] org-babel-haskell-initiate-session: Remove secondary
>>  prompt
>>
>> * lisp/ob-haskell.el (org-babel-haskell-initiate-session): Set
>> secondary prompt to "".  If we do not do this, org-comint may treat
>> secondary prompts as a part of output.
>
>> +(sleep-for 0.25)
>> +;; Disable secondary prompt.
>
> It would be useful to explain the purpose of disabling the secondary
> prompt in the source code comment itself, not just in the commit
> message. It will improve readability.

Are you reviewing your own improvements ? :-)

Fixed. I've copied/pasted your explanation in the code.



>> From 352d18399961fedc45cc2d64007016426e1ecd40 Mon Sep 17 00:00:00 2001
>> From: Ihor Radchenko 
>> Date: Fri, 24 Mar 2023 11:26:00 +0100
>> Subject: [PATCH 04/13] * testing/lisp/test-ob-haskell-ghci.el: Enable fixed
>
> I do not see PATCH 03/13 in the attachments.

Sorry. I forgot it the first time. The email, after handling the
comments from Ruijie Yu, had it though.



>> From 7d66cff5cc23bb786cb2843f4326d2869512ccac Mon Sep 17 00:00:00 2001
>> From: Bruno BARBIER 
>> Date: Sat, 25 Mar 2023 10:06:44 +0100
>> Subject: [PATCH 06/13] ob-haskell: Implement sessions
>>
>> +  (unless session-name
>> +;; As haskell-mode is using the buffer name "*haskell*", we stay
>> +;; away from it.
>> +(setq session-name (generate-new-buffer-name "*ob-haskell*")))
>> +  (let ((session (get-buffer session-name)))
>
> session is not a buffer or nil, if no buffer named session-name exists.

The argument SESSION-NAME must be a string or nil (I added a test to
make clear that it must be a string).  Thus, session will either be nil or
a live buffer.



>> +(save-window-excursion
>> +  (or (org-babel-comint-buffer-livep session)
>
> Below, (org-babel-comint-buffer-livep session) is nil, which implies
> either that session is nil, does not exist, not live, or does not have a
> process attached.

ok. So, in our case, session is either nil, or it's a live buffer
without an attached process.

>
>> +  (let ((inferior-haskell-buffer session))
>> +(when (and (bufferp session) (not 
>> (org-babel-comint-buffer-livep session)))
>
> (not (org-babel-comint-buffer-livep session)) is always t here.

Right. I removed the test. Thanks.


> Also, session may be a killed buffer object. It is still a buffer, but
> not usable. See `buffer-live-p'.

By construction, if 'session' is a buffer, then, it is a live buffer.


>
>> +  (when (bufferp "*haskell*") (error "Conflicting buffer 
>> '*haskell*', rename it or kill it."))
>> +  (with-current-buffer session (rename-buffer "*haskell*")))
>
> So, you are now renaming the unique session buffer back to "*haskell*".
> And never rename it back to expected :session . Users might be 
> confused.

I do rename it back once inf-haskell has initialized the buffer (after
run-haskell in the last version).


>> +(save-window-excursion
>> +  ;; We don't use `run-haskell' to not popup the buffer.
>> +  ;; And we protect default-directory.
>> +  (let ((default-directory default-directory))
>> +(inferior-haskell-start-process))
>
> This is a workaround for a nasty side effect of running
> `inferior-haskell-start-process'. We should report this to haskell-mode
> developers, leaving appropriate comment in the code.

About 'run-haskell', I reverted my change: we're inside a
'save-window-excursion', which looks like the standard way to get rid
of unwanted popups (like ob-shell does).

About 'default-directory', I'm not sure. Maybe the side effect is done
on purpose in inf-haskell.



>
>> +  (sleep-for 0.25)
>> +  (setq session inferior-haskell-buffer)
>> +  (with-current-buffer session (rename-buffer session-name))
>
> This generally looks like a brittle workaround for inner workings of
> haskell-mode. I recommend sending an email to haskell-mode devs,
> requesting multiple session support. Otherwise, this whole code
> eventually be broken.

Yes. It's a workaround.  But it looks reasonably safe to me, 

Re: [PATCH] Add tests for ob-haskell (GHCi)

[Ihor Radchenko]

Bruno Barbier  writes:

> Let me know if you see further improvement before pushing this.

Thanks for the update!

I can see that you limited the tests scope to :session blocks.
Would it be possible to extend the existing tests to :compile yes case?
>From a glance, it does not look like you need to change much - Haskell
behaviour should be similar with or without ghci.

> From 9972b926f55cb970e0b520f8726a3684118017b6 Mon Sep 17 00:00:00 2001
> From: Ihor Radchenko 
> Date: Fri, 24 Mar 2023 11:20:22 +0100
> Subject: [PATCH 02/13] org-babel-haskell-initiate-session: Remove secondary
>  prompt
>
> * lisp/ob-haskell.el (org-babel-haskell-initiate-session): Set
> secondary prompt to "".  If we do not do this, org-comint may treat
> secondary prompts as a part of output.

> +(sleep-for 0.25)
> +;; Disable secondary prompt.

It would be useful to explain the purpose of disabling the secondary
prompt in the source code comment itself, not just in the commit
message. It will improve readability.

> From 352d18399961fedc45cc2d64007016426e1ecd40 Mon Sep 17 00:00:00 2001
> From: Ihor Radchenko 
> Date: Fri, 24 Mar 2023 11:26:00 +0100
> Subject: [PATCH 04/13] * testing/lisp/test-ob-haskell-ghci.el: Enable fixed

I do not see PATCH 03/13 in the attachments.

> From 7d66cff5cc23bb786cb2843f4326d2869512ccac Mon Sep 17 00:00:00 2001
> From: Bruno BARBIER 
> Date: Sat, 25 Mar 2023 10:06:44 +0100
> Subject: [PATCH 06/13] ob-haskell: Implement sessions
>
> +  (unless session-name
> +;; As haskell-mode is using the buffer name "*haskell*", we stay
> +;; away from it.
> +(setq session-name (generate-new-buffer-name "*ob-haskell*")))
> +  (let ((session (get-buffer session-name)))

session is not a buffer or nil, if no buffer named session-name exists.

> +(save-window-excursion
> +  (or (org-babel-comint-buffer-livep session)

Below, (org-babel-comint-buffer-livep session) is nil, which implies
either that session is nil, does not exist, not live, or does not have a
process attached.

> +  (let ((inferior-haskell-buffer session))
> +(when (and (bufferp session) (not (org-babel-comint-buffer-livep 
> session)))

(not (org-babel-comint-buffer-livep session)) is always t here.
Also, session may be a killed buffer object. It is still a buffer, but
not usable. See `buffer-live-p'.

> +  (when (bufferp "*haskell*") (error "Conflicting buffer 
> '*haskell*', rename it or kill it."))
> +  (with-current-buffer session (rename-buffer "*haskell*")))

So, you are now renaming the unique session buffer back to "*haskell*".
And never rename it back to expected :session . Users might be confused.

> +(save-window-excursion
> +  ;; We don't use `run-haskell' to not popup the buffer.
> +  ;; And we protect default-directory.
> +  (let ((default-directory default-directory))
> +(inferior-haskell-start-process))

This is a workaround for a nasty side effect of running
`inferior-haskell-start-process'. We should report this to haskell-mode
developers, leaving appropriate comment in the code.

> +  (sleep-for 0.25)
> +  (setq session inferior-haskell-buffer)
> +  (with-current-buffer session (rename-buffer session-name))

This generally looks like a brittle workaround for inner workings of
haskell-mode. I recommend sending an email to haskell-mode devs,
requesting multiple session support. Otherwise, this whole code
eventually be broken.

> Subject: [PATCH 10/13] * testing/lisp/test-ob-haskell-ghci.el: Test output
>  without EOL
> ...
> +(ert-deftest ob-haskell/output-without-eol-1 ()
> +  "Cannot get output from incomplete lines, when entered line by line."
> +  :expected-result :failed
> +  (should (equal "123"
> + (test-ob-haskell-ghci ":results output" "
> +  putStr(\"1\")
> +  putStr(\"2\")
> +  putStr(\"3\")
> +  putStr(\"\\n\")
> +"

May you explain more about this bug?

> Subject: [PATCH 11/13] lisp/ob-haskell.el: Fix how to use sessions
>
> +  (org-babel-haskell-with-session

This kind of names are usually dedicated to macro calls. But
`org-babel-haskell-with-session' is instead a function. I think a macro
will be better. And you will be able to get rid of unnecessary lambda.

> +   params
> +   (lambda (session)
> + (cl-labels
> + ((csend (txt)
> +  (eom ()
> +  (with-output (todo)

When using `cl-labels', please prefer longer, more descriptive function
names. These functions do not have a docstring and I now am left
guessing and reading the function code repeatedly to understand the
usage.

> +  (full-body (org-babel-expand-body:generic
> +  body params
> +  (org-babel-variable-assignments:haskell params)))

I think we want `org-babel-expand-src-block' here instead of using
semi-internal ob-core.el parts.

> -(let ((buffer 

Re: [PATCH] Add tests for ob-haskell (GHCi)

[Bruno Barbier]

Ruijie Yu  writes:

> Minor remarks below regarding the patchset.
>
> Bruno Barbier  writes:
>
>> +;; Copyright (c) 2023  Free Software Foundation, Inc.
>
> lisp/org.el has only a single space, so probably single space here as well.

Done.

>> +
>> +;; Authors: Bruno BARBIER 
>> +
>> +;; This program is free software; you can redistribute it and/or modify
>> +;; it under the terms of the GNU General Public License as published by
>> +;; the Free Software Foundation, either version 3 of the License, or
>> +;; (at your option) any later version.
>
> Do we need the text for "part of GNU Emacs"?
>

I guess it doesn't harm: I added it, thanks.


>> +
>> +(defun test-ob-haskell-ghci--with-global-session-worker (todo)
>> +  "See `test-ob-haskell-ghci--with-global-session-worker'."
>
> This docstring doesn't say much and only refers to itself.  Maybe
> explain what it does?  (Or now that I look at it, potentially you wanted
> to refer to the macro `test-ob-haskell-ghci-with-global-session'
> instead.)

I've rewritten that function later ... which made the documentation even worse 
:-)
I've fixed it, thanks.


>
>> +(defun test-ob-haskell-ghci (args content  preamble unprotected)
>> +  "Execute the code block CONTENT in a new GHCi session; return the result.
>> +Add ARGS to the code block argument line.  Insert PREAMBLE
>> +before the code block.  When UNPROTECTED is non-nil, don't control
>> +which session is used (i.e. don't call
>> +`test-ob-haskell-ghci--with-global-session-worker')."
>> +  (when (listp content)
>> +(setq content (string-join content "\n")))
>> +  (unless preamble
>> +(setq preamble ""))
>> +  (let ((todo  (lambda ()
>
> One space.

AFAICS, the last version has only one space here.


>> +
>> + Not define  errors
>> +;;

> Single space?

It was an invisible 'd' actually; I repainted in black :-)
Thanks.


Thank you for your review,

Bruno

>From 136878a096eb9f459e97da6617f94ba84085db9b Mon Sep 17 00:00:00 2001
From: Bruno BARBIER 
Date: Fri, 18 Nov 2022 20:14:20 +0100
Subject: [PATCH 01/13] ob-haskell: Add tests for GHCi

testing/lisp/test-ob-haskell-ghci.el: New file.
---
 testing/lisp/test-ob-haskell-ghci.el | 454 +++
 1 file changed, 454 insertions(+)
 create mode 100644 testing/lisp/test-ob-haskell-ghci.el

diff --git a/testing/lisp/test-ob-haskell-ghci.el b/testing/lisp/test-ob-haskell-ghci.el
new file mode 100644
index 0..4023873de
--- /dev/null
+++ b/testing/lisp/test-ob-haskell-ghci.el
@@ -0,0 +1,454 @@
+;;; test-ob-haskell-ghci.el --- tests for ob-haskell.el GHCi  -*- lexical-binding: t; -*-
+
+;; Copyright (c) 2023 Free Software Foundation, Inc.
+;; Authors: Bruno BARBIER 
+
+;; This file is part of GNU Emacs.
+
+;; This program is free software; you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; This program is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with this program.  If not, see .
+
+;;; Commentary:
+;;
+
+ Useful references
+;;
+;;  - https://orgmode.org/worg/org-contrib/babel/languages/lang-compat.html
+;;  - GHCi manual: https://downloads.haskell.org/ghc/latest/docs/users_guide/ghci.html
+ FIXME: Random failures
+;;
+;; To increase the chances of failure when running tests, you can use this command line:
+;;
+;;(for I in 0 1 2 3 4 5 6 7 8 9 10 0 1 2 3 4 5 6 7 8 9 10 0 1 2 3 4 5 6 7 8 9 10; do make 'BTEST_OB_LANGUAGES=haskell' BTEST_RE='haskell' test-dirty & done) 2>&1 | grep FAILED
+;;
+
+ Status
+;;
+;; All the tests should succeed (except for random failures); those
+;; flagged with ":expected-result :failed" are known
+;; limitations/bugs.  Tested with (2023-03-18):
+;;
+;; | emacs-version |  29.0.60 |
+;; | org-version   | main@4cad6c8ea (Mar 16 2023) |
+;; | haskell-mode  | master@20d4e23 (Mar 4  2023) |
+;; | ghci  |9.0.2 |
+
+
+;;; Code:
+;;
+
+(require 'org-test "../testing/org-test")
+(org-test-for-executable "ghci")
+(unless (featurep 'haskell-mode)
+  (signal 'missing-test-dependency "haskell-mode"))
+
+
+;;; Helpers
+;;
+
+(defun test-ob-haskell-ghci--with-global-session-worker (todo)
+  "See `test-ob-haskell-ghci--with-global-session-worker'."
+  (when (get-buffer "*haskell*")
+(error "A buffer named '*haskell*' exists.  Can't safely test haskell blocks"))
+  (unwind-protect (funcall todo)
+;; Kill the "*haskell*" buffer to not pollute other tests.
+(when-let ((hb (get-buffer "*haskell*")))
+  (with-current-buffer hb
+(let 

Re: [PATCH] Add tests for ob-haskell (GHCi)

[General discussions about Org-mode.]

Minor remarks below regarding the patchset.

Bruno Barbier  writes:

> From 9ef867cd2cf89e77b5c5a5a7090fd37b1702e06a Mon Sep 17 00:00:00 2001
> From: Bruno BARBIER 
> Date: Fri, 18 Nov 2022 20:14:20 +0100
> Subject: [PATCH 01/13] ob-haskell: Add tests for GHCi
>
> testing/lisp/test-ob-haskell-ghci.el: New file.
> ---
>  testing/lisp/test-ob-haskell-ghci.el | 453 +++
>  1 file changed, 453 insertions(+)
>  create mode 100644 testing/lisp/test-ob-haskell-ghci.el
>
> diff --git a/testing/lisp/test-ob-haskell-ghci.el 
> b/testing/lisp/test-ob-haskell-ghci.el
> new file mode 100644
> index 0..aba94d73f
> --- /dev/null
> +++ b/testing/lisp/test-ob-haskell-ghci.el
> @@ -0,0 +1,453 @@
> +;;; test-ob-haskell-ghci.el --- tests for ob-haskell.el GHCi  -*- 
> lexical-binding: t; -*-
> +
> +;; Copyright (c) 2023  Free Software Foundation, Inc.

lisp/org.el has only a single space, so probably single space here as well.

> +
> +;; Authors: Bruno BARBIER 
> +
> +;; This program is free software; you can redistribute it and/or modify
> +;; it under the terms of the GNU General Public License as published by
> +;; the Free Software Foundation, either version 3 of the License, or
> +;; (at your option) any later version.

Do we need the text for "part of GNU Emacs"?

> +
> +;; This program is distributed in the hope that it will be useful,
> +;; but WITHOUT ANY WARRANTY; without even the implied warranty of
> +;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +;; GNU General Public License for more details.
> +
> +;; You should have received a copy of the GNU General Public License
> +;; along with this program.  If not, see .
> +
> +;;; Commentary:
> +;;
> +
> + Useful references
> +;;
> +;;  - https://orgmode.org/worg/org-contrib/babel/languages/lang-compat.html
> +;;  - GHCi manual: 
> https://downloads.haskell.org/ghc/latest/docs/users_guide/ghci.html
> + FIXME: Random failures
> +;;
> +;; To increase the chances of failure when running tests, you can use this 
> command line:
> +;;
> +;;(for I in 0 1 2 3 4 5 6 7 8 9 10 0 1 2 3 4 5 6 7 8 9 10 0 1 2 3 4 5 6 
> 7 8 9 10; do make 'BTEST_OB_LANGUAGES=haskell' BTEST_RE='haskell' test-dirty 
> & done) 2>&1 | grep FAILED
> +;;

Wanted to say something here, but then realized that you deleted this
portion in a later commit.

> +
> + Status
> +;;
> +;; All the tests should succeed (except for random failures); those
> +;; flagged with ":expected-result :failed" are known
> +;; limitations/bugs.  Tested with (2023-03-18):
> +;;
> +;; | emacs-version |  29.0.60 |
> +;; | org-version   | main@4cad6c8ea (Mar 16 2023) |
> +;; | haskell-mode  | master@20d4e23 (Mar 4  2023) |
> +;; | ghci  |9.0.2 |
> +
> +
> +;;; Code:
> +;;
> +
> +(require 'org-test "../testing/org-test")
> +(org-test-for-executable "ghci")
> +(unless (featurep 'haskell-mode)
> +  (signal 'missing-test-dependency "haskell-mode"))
> +
> +
> +;;; Helpers
> +;;
> +
> +(defun test-ob-haskell-ghci--with-global-session-worker (todo)
> +  "See `test-ob-haskell-ghci--with-global-session-worker'."

This docstring doesn't say much and only refers to itself.  Maybe
explain what it does?  (Or now that I look at it, potentially you wanted
to refer to the macro `test-ob-haskell-ghci-with-global-session'
instead.)

> +(defun test-ob-haskell-ghci (args content  preamble unprotected)
> +  "Execute the code block CONTENT in a new GHCi session; return the result.
> +Add ARGS to the code block argument line.  Insert PREAMBLE
> +before the code block.  When UNPROTECTED is non-nil, don't control
> +which session is used (i.e. don't call
> +`test-ob-haskell-ghci--with-global-session-worker')."
> +  (when (listp content)
> +(setq content (string-join content "\n")))
> +  (unless preamble
> +(setq preamble ""))
> +  (let ((todo  (lambda ()

One space.

> + Data tables
> +;;
> +
> +(ert-deftest ob-haskell/int-table-data ()
> +  "From worg: int-table-data."
> +  (should (equal 10 (test-ob-haskell-ghci ":var t=int-table-data"
> +  "sum [sum r | r <- t]"
> +  "#+name: int-table-data
> +| 1 | 2 |
> +| 3 | 4 |"
> +
> +(ert-deftest ob-haskell/float-table-data ()
> +  "From worg: float-table-data."
> +  (should (equal 11.0 (test-ob-haskell-ghci ":var t=float-table-data"
> +"sum [sum r | r <- t]"
> +"#+name: float-table-data
> +| 1.1 | 2.2 |
> +| 3.3 | 4.4 |"
> +
> +(ert-deftest ob-haskell/string-table-data ()
> +  "From worg: string-table-data."
> +  (should (equal "abcd" (test-ob-haskell-ghci ":var t=string-table-data"
> +  "concat [concat r | r <- t]"
> +  "#+name: string-table-data
> +| a | b |

Re: [PATCH] Add tests for ob-haskell (GHCi)

[Bruno Barbier]

Hi Ihor,

Sorry for the delay.

Bruno Barbier  writes:

> Ihor Radchenko  writes:
>
>> Bruno Barbier  writes:
>>
>>> Note that I've changed the tests about errors; I'm now expecting
>>> ob-haskell to raise errors. I'm not sure what we should expect to be
>>> consistent with other org babel backends.
>>
>> Errors are usually displayed separately, using
>> `org-babel-eval-error-notify'.
>
> I'll see what can be done with GHCi and use this if possible.

Unfortunately, no progress on this.  And I don't really have time to
work on this.



>>> +;; To increase the chances of failure when running tests, you can use this 
>>> command line:
>>> +;;
..
>>> +;; | ghci  |9.0.2 |
>>
>> You can probably remove this.
>
> Definitely. I'll do. Thanks.

Done.



>>> +  (when (and session-name (string= session-name "none"))
>>> +(setq session-name nil))
>>> +  (unless session-name
>>> +;; As haskell-mode is using the buffer name "*haskell*", we stay
>>> +;; away from it.
>>> +(setq session-name (generate-new-buffer-name "*ob-haskell*")))
>>
>> This will make ob-haskell spawn a separate ghci process buffer every
>> single time a user runs non-session src block. And the buffer is not
>> closed after getting the result.

> Very good point!
>
> I will update this to use the same buffer named "*ob-haskell*" when the
> user doesn't set the session name.  I guess it's consistent with other
> org-babel backends.

I've changed the way session works. 'ob-haskell' should now destroy
temporary sessions.  Thanks again for spotting that mistake.


Let me know if you see further improvement before pushing this.

Thanks,

Bruno


>From 9ef867cd2cf89e77b5c5a5a7090fd37b1702e06a Mon Sep 17 00:00:00 2001
From: Bruno BARBIER 
Date: Fri, 18 Nov 2022 20:14:20 +0100
Subject: [PATCH 01/13] ob-haskell: Add tests for GHCi

testing/lisp/test-ob-haskell-ghci.el: New file.
---
 testing/lisp/test-ob-haskell-ghci.el | 453 +++
 1 file changed, 453 insertions(+)
 create mode 100644 testing/lisp/test-ob-haskell-ghci.el

diff --git a/testing/lisp/test-ob-haskell-ghci.el b/testing/lisp/test-ob-haskell-ghci.el
new file mode 100644
index 0..aba94d73f
--- /dev/null
+++ b/testing/lisp/test-ob-haskell-ghci.el
@@ -0,0 +1,453 @@
+;;; test-ob-haskell-ghci.el --- tests for ob-haskell.el GHCi  -*- lexical-binding: t; -*-
+
+;; Copyright (c) 2023  Free Software Foundation, Inc.
+
+;; Authors: Bruno BARBIER 
+
+;; This program is free software; you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; This program is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with this program.  If not, see .
+
+;;; Commentary:
+;;
+
+ Useful references
+;;
+;;  - https://orgmode.org/worg/org-contrib/babel/languages/lang-compat.html
+;;  - GHCi manual: https://downloads.haskell.org/ghc/latest/docs/users_guide/ghci.html
+ FIXME: Random failures
+;;
+;; To increase the chances of failure when running tests, you can use this command line:
+;;
+;;(for I in 0 1 2 3 4 5 6 7 8 9 10 0 1 2 3 4 5 6 7 8 9 10 0 1 2 3 4 5 6 7 8 9 10; do make 'BTEST_OB_LANGUAGES=haskell' BTEST_RE='haskell' test-dirty & done) 2>&1 | grep FAILED
+;;
+
+ Status
+;;
+;; All the tests should succeed (except for random failures); those
+;; flagged with ":expected-result :failed" are known
+;; limitations/bugs.  Tested with (2023-03-18):
+;;
+;; | emacs-version |  29.0.60 |
+;; | org-version   | main@4cad6c8ea (Mar 16 2023) |
+;; | haskell-mode  | master@20d4e23 (Mar 4  2023) |
+;; | ghci  |9.0.2 |
+
+
+;;; Code:
+;;
+
+(require 'org-test "../testing/org-test")
+(org-test-for-executable "ghci")
+(unless (featurep 'haskell-mode)
+  (signal 'missing-test-dependency "haskell-mode"))
+
+
+;;; Helpers
+;;
+
+(defun test-ob-haskell-ghci--with-global-session-worker (todo)
+  "See `test-ob-haskell-ghci--with-global-session-worker'."
+  (when (get-buffer "*haskell*")
+(error "A buffer named '*haskell*' exists.  Can't safely test haskell blocks"))
+  (unwind-protect (funcall todo)
+;; Kill the "*haskell*" buffer to not pollute other tests.
+(when-let ((hb (get-buffer "*haskell*")))
+  (with-current-buffer hb
+(let ((kill-buffer-query-functions nil)
+  (kill-buffer-hook nil))
+  (kill-buffer hb))
+
+(defmacro test-ob-haskell-ghci-with-global-session ( body)
+  "Eval BODY in a new session, then destroy the session.
+The library ob-haskell doesn't implement session 

Re: [PATCH] Add tests for ob-haskell (GHCi)

[Ihor Radchenko]

Bruno Barbier  writes:

>> Why not simply putStrLn ("\"" ++ show it ++ "\"") ?
>>
>
> I'm not sure I understand. I'm using the first
> 'org-babel-comint-with-output' to execute the source block and save the
> last value (the "it" variable). Then, I'm using a second
> 'org-babel-comint-with-output' to make sure the output of this one
> contains only the last value. If I display "it" in the first block, I
> will not be able to differentiate between some output, previous values
> and the last value. And, printing "org-babel-haskell-eoe" updates the
> "it" variable (the last value becoming "()"), so I have to store the
> real "it" somewhere.

I see. I missed that `it' gets updated because of eoe indicator.

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

Re: [PATCH] Add tests for ob-haskell (GHCi)

[Bruno Barbier]

Ihor Radchenko  writes:

> Bruno Barbier  writes:
>
>> Note that I've changed the tests about errors; I'm now expecting
>> ob-haskell to raise errors. I'm not sure what we should expect to be
>> consistent with other org babel backends.
>
> Errors are usually displayed separately, using
> `org-babel-eval-error-notify'.

I'll see what can be done with GHCi and use this if possible.
Thanks.


>> +;; To increase the chances of failure when running tests, you can use this 
>> command line:
>> +;;
>> +;;(for I in 0 1 2 3 4 5 6 7 8 9 10 0 1 2 3 4 5 6 7 8 9 10 0 1 2 3 4 5 6 
>> 7 8 9 10; do make 'BTEST_OB_LANGUAGES=haskell' BTEST_RE='haskell' test-dirty 
>> & done) 2>&1 | grep FAILED
>> +;;
>> +
>> + Status
>> +;;
>> +;; All the tests should succeed (except for random failures); those
>> +;; flagged with ":expected-result :failed" are known
>> +;; limitations/bugs.  Tested with (2023-03-18):
>> +;;
>> +;; | emacs-version |  29.0.60 |
>> +;; | org-version   | main@4cad6c8ea (Mar 16 2023) |
>> +;; | haskell-mode  | master@20d4e23 (Mar 4  2023) |
>> +;; | ghci  |9.0.2 |
>
> You can probably remove this.

Definitely. I'll do. Thanks.



>> +(`value (org-babel-comint-with-output
>> +(session org-babel-haskell-eoe nil full-body)
>> +  (insert "__LAST_VALUE_IMPROBABLE_NAME__=()::()\n")
>> +  (comint-send-input nil t)
>> +  (insert full-body)
>> +  (comint-send-input nil t)
>> +  (insert "__LAST_VALUE_IMPROBABLE_NAME__=it\n")
>> +  (comint-send-input nil t)
>> +  (insert (concat "putStrLn (\"\\\"\" ++ " 
>> org-babel-haskell-eoe " ++ \"\\\"\")\n"))
>
> Why not simply putStrLn ("\"" ++ show it ++ "\"") ?
>

I'm not sure I understand. I'm using the first
'org-babel-comint-with-output' to execute the source block and save the
last value (the "it" variable). Then, I'm using a second
'org-babel-comint-with-output' to make sure the output of this one
contains only the last value. If I display "it" in the first block, I
will not be able to differentiate between some output, previous values
and the last value. And, printing "org-babel-haskell-eoe" updates the
"it" variable (the last value becoming "()"), so I have to store the
real "it" somewhere.

Am I missing something ?

>> +  (when (and session-name (string= session-name "none"))
>> +(setq session-name nil))
>> +  (unless session-name
>> +;; As haskell-mode is using the buffer name "*haskell*", we stay
>> +;; away from it.
>> +(setq session-name (generate-new-buffer-name "*ob-haskell*")))
>
> This will make ob-haskell spawn a separate ghci process buffer every
> single time a user runs non-session src block. And the buffer is not
> closed after getting the result.

Very good point!

I will update this to use the same buffer named "*ob-haskell*" when the
user doesn't set the session name.  I guess it's consistent with other
org-babel backends.



Thank you very much for the review and you help!

Bruno

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

Re: [PATCH] Add tests for ob-haskell (GHCi)

[Ihor Radchenko]

Bruno Barbier  writes:

> About :results output vs :results value, I decided to use the "it"
> variable, i.e. to ask GHCi what was the last value (see [1]).
>
> About sessions, I decided to rename the buffer as you suggested. That's
> a partial workaround that should work well enough, until haskell-mode
> provides a way to choose the buffer name.
>
> I've updated the tests and the expected results.

Thanks!
I will provide comments inline.

> Note that I've changed the tests about errors; I'm now expecting
> ob-haskell to raise errors. I'm not sure what we should expect to be
> consistent with other org babel backends.

Errors are usually displayed separately, using
`org-babel-eval-error-notify'.

> +;; To increase the chances of failure when running tests, you can use this 
> command line:
> +;;
> +;;(for I in 0 1 2 3 4 5 6 7 8 9 10 0 1 2 3 4 5 6 7 8 9 10 0 1 2 3 4 5 6 
> 7 8 9 10; do make 'BTEST_OB_LANGUAGES=haskell' BTEST_RE='haskell' test-dirty 
> & done) 2>&1 | grep FAILED
> +;;
> +
> + Status
> +;;
> +;; All the tests should succeed (except for random failures); those
> +;; flagged with ":expected-result :failed" are known
> +;; limitations/bugs.  Tested with (2023-03-18):
> +;;
> +;; | emacs-version |  29.0.60 |
> +;; | org-version   | main@4cad6c8ea (Mar 16 2023) |
> +;; | haskell-mode  | master@20d4e23 (Mar 4  2023) |
> +;; | ghci  |9.0.2 |

You can probably remove this.

> +(`value (org-babel-comint-with-output
> + (session org-babel-haskell-eoe nil full-body)
> +  (insert "__LAST_VALUE_IMPROBABLE_NAME__=()::()\n")
> +  (comint-send-input nil t)
> +  (insert full-body)
> +  (comint-send-input nil t)
> +  (insert "__LAST_VALUE_IMPROBABLE_NAME__=it\n")
> +  (comint-send-input nil t)
> +  (insert (concat "putStrLn (\"\\\"\" ++ " 
> org-babel-haskell-eoe " ++ \"\\\"\")\n"))

Why not simply putStrLn ("\"" ++ show it ++ "\"") ?

> +  (when (and session-name (string= session-name "none"))
> +(setq session-name nil))
> +  (unless session-name
> +;; As haskell-mode is using the buffer name "*haskell*", we stay
> +;; away from it.
> +(setq session-name (generate-new-buffer-name "*ob-haskell*")))

This will make ob-haskell spawn a separate ghci process buffer every
single time a user runs non-session src block. And the buffer is not
closed after getting the result.

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

Re: [PATCH] Add tests for ob-haskell (GHCi)

[ParetoOptimalDev]

Ihor Radchenko  writes:

> Note that setting the prompt to non-"", makes little sense - we want to
> remove it anyway. Why do extra work?

Oh, I didn't notice you set the prompt to "". I thought the extra work
you were being done was *because* somewhere else the default was set to
"λ ".

I agree, do whatever is less work and more normalized here.

Re: [PATCH] Add tests for ob-haskell (GHCi)

[Bruno Barbier]

Ihor Radchenko  writes:

> I investigated further and now applied a set of patches that improves
> prompt filtering in org-comint.
> ..

Thanks Ihor.

I don't get random failures anymore.  And thanks for fixing the tests
that were incorrect.

Thanks Pareto for mentioning the possibiliy to customize GHCi prompts.



> The remaining tests fall into two categories:
> 1. Tests trying to test :results value vs. :results output in sessions.
> 2. Tests trying to test for multiple sessions not interfering each
>other.

About :results output vs :results value, I decided to use the "it"
variable, i.e. to ask GHCi what was the last value (see [1]).

About sessions, I decided to rename the buffer as you suggested. That's
a partial workaround that should work well enough, until haskell-mode
provides a way to choose the buffer name.

I've updated the tests and the expected results.

Note that I've changed the tests about errors; I'm now expecting
ob-haskell to raise errors. I'm not sure what we should expect to be
consistent with other org babel backends.

I also did some modifications in my initial patch in-place.

Bruno


[1]:https://downloads.haskell.org/ghc/latest/docs/users_guide/ghci.html#the-it-variable

>From 46e8fa78574908a15fe6eb82a2cca5d2f537c78e Mon Sep 17 00:00:00 2001
From: Bruno BARBIER 
Date: Fri, 18 Nov 2022 20:14:20 +0100
Subject: [PATCH 1/7] ob-haskell: Add tests for GHCi

testing/lisp/test-ob-haskell-ghci.el: New file.
---
 testing/lisp/test-ob-haskell-ghci.el | 453 +++
 1 file changed, 453 insertions(+)
 create mode 100644 testing/lisp/test-ob-haskell-ghci.el

diff --git a/testing/lisp/test-ob-haskell-ghci.el b/testing/lisp/test-ob-haskell-ghci.el
new file mode 100644
index 0..aba94d73f
--- /dev/null
+++ b/testing/lisp/test-ob-haskell-ghci.el
@@ -0,0 +1,453 @@
+;;; test-ob-haskell-ghci.el --- tests for ob-haskell.el GHCi  -*- lexical-binding: t; -*-
+
+;; Copyright (c) 2023  Free Software Foundation, Inc.
+
+;; Authors: Bruno BARBIER 
+
+;; This program is free software; you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; This program is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with this program.  If not, see .
+
+;;; Commentary:
+;;
+
+ Useful references
+;;
+;;  - https://orgmode.org/worg/org-contrib/babel/languages/lang-compat.html
+;;  - GHCi manual: https://downloads.haskell.org/ghc/latest/docs/users_guide/ghci.html
+ FIXME: Random failures
+;;
+;; To increase the chances of failure when running tests, you can use this command line:
+;;
+;;(for I in 0 1 2 3 4 5 6 7 8 9 10 0 1 2 3 4 5 6 7 8 9 10 0 1 2 3 4 5 6 7 8 9 10; do make 'BTEST_OB_LANGUAGES=haskell' BTEST_RE='haskell' test-dirty & done) 2>&1 | grep FAILED
+;;
+
+ Status
+;;
+;; All the tests should succeed (except for random failures); those
+;; flagged with ":expected-result :failed" are known
+;; limitations/bugs.  Tested with (2023-03-18):
+;;
+;; | emacs-version |  29.0.60 |
+;; | org-version   | main@4cad6c8ea (Mar 16 2023) |
+;; | haskell-mode  | master@20d4e23 (Mar 4  2023) |
+;; | ghci  |9.0.2 |
+
+
+;;; Code:
+;;
+
+(require 'org-test "../testing/org-test")
+(org-test-for-executable "ghci")
+(unless (featurep 'haskell-mode)
+  (signal 'missing-test-dependency "haskell-mode"))
+
+
+;;; Helpers
+;;
+
+(defun test-ob-haskell-ghci--with-global-session-worker (todo)
+  "See `test-ob-haskell-ghci--with-global-session-worker'."
+  (when (get-buffer "*haskell*")
+(error "A buffer named '*haskell*' exists.  Can't safely test haskell blocks"))
+  (unwind-protect (funcall todo)
+;; Kill the "*haskell*" buffer to not pollute other tests.
+(when-let ((hb (get-buffer "*haskell*")))
+  (with-current-buffer hb
+(let ((kill-buffer-query-functions nil)
+  (kill-buffer-hook nil))
+  (kill-buffer hb))
+
+(defmacro test-ob-haskell-ghci-with-global-session ( body)
+  "Eval BODY in a new session, then destroy the session.
+The library ob-haskell doesn't implement session yet.  It will
+always use a buffer named \"*haskell*\".  We kill that buffer
+after the source block execution.  To be safe, we fail if such a
+buffer already exists."
+  `(test-ob-haskell-ghci--with-global-session-worker (lambda () ,@body)))
+
+(defun test-ob-haskell-ghci (args content  preamble unprotected)
+  "Execute the code block CONTENT in a new GHCi session; return the result.
+Add ARGS to the code block argument line.  Insert PREAMBLE
+before 

Re: [PATCH] Add tests for ob-haskell (GHCi)

[Ihor Radchenko]

Ihor Radchenko  writes:

> We can try the attached patch. Yet another edge case in comint, it
> appears.

I investigated further and now applied a set of patches that improves
prompt filtering in org-comint.

https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=a8a516ba330fe8c435334030ffbe371b8c80c877
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=866ed1a3c5c37cad243085f9a8fa904970e4d614

Further, I tried to incorporate Pareto's suggestion about secondary
prompt and cleaned up the original GHCi tests.

With the attached set of patches (first one is Bruno's original patch),
I get all the tests reliably passing, except the tests left as
:expected-result failed.

The remaining tests fall into two categories:
1. Tests trying to test :results value vs. :results output in sessions.
2. Tests trying to test for multiple sessions not interfering each
   other.

The first category of tests fails because `org-babel-interpret-haskell'
simply cannot distinguish between :results value and :results output.
Someone familiar with GHC should handle this problem. For example, by
wrapping :results output code in such a way that output is discarded in
the GHCi comint buffer.

The second category of tests fails because haskell-mode uses a fixed
"*haskell*" comint buffer name. As I suggested in another message, a
viable approach could be renaming the original "*haskell*" buffer to
something else in `org-babel-haskell-initiate-session'. Again, someone
more familiar with haskell-mode should judge if such option is truly
viable.

>From 1273b5b415fa420e718645452bc75bc7a9407af5 Mon Sep 17 00:00:00 2001
Message-Id: <1273b5b415fa420e718645452bc75bc7a9407af5.1679653723.git.yanta...@posteo.net>
From: Bruno BARBIER 
Date: Fri, 18 Nov 2022 20:14:20 +0100
Subject: [PATCH 1/4] ob-haskell: Add tests for GHCi

testing/lisp/test-ob-haskell-ghci.el: New file.
---
 testing/lisp/test-ob-haskell-ghci.el | 428 +++
 1 file changed, 428 insertions(+)
 create mode 100644 testing/lisp/test-ob-haskell-ghci.el

diff --git a/testing/lisp/test-ob-haskell-ghci.el b/testing/lisp/test-ob-haskell-ghci.el
new file mode 100644
index 0..4b1e4669b
--- /dev/null
+++ b/testing/lisp/test-ob-haskell-ghci.el
@@ -0,0 +1,428 @@
+;;; test-ob-haskell-ghci.el --- tests for ob-haskell.el GHCi  -*- lexical-binding: t; -*-
+
+;; Copyright (c) 2023  Free Software Foundation, Inc.
+
+;; Authors: Bruno BARBIER 
+
+;; This program is free software; you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; This program is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with this program.  If not, see .
+
+;;; Commentary:
+;;
+
+ Useful references
+;;
+;;  - https://orgmode.org/worg/org-contrib/babel/languages/lang-compat.html
+
+ FIXME: Random failures
+;;
+;; To increase the chances of failure when running tests, you can use this command line:
+;;
+;;(for I in 0 1 2 3 4 5 6 7 8 9 10 0 1 2 3 4 5 6 7 8 9 10 0 1 2 3 4 5 6 7 8 9 10; do make 'BTEST_OB_LANGUAGES=haskell' BTEST_RE='haskell' test-dirty & done) 2>&1 | grep FAILED
+;;
+
+ Status
+;;
+;; All the tests should succeed (except for random failures); those
+;; flagged with ":expected-result :failed" are known
+;; limitations/bugs.  Tested with (2023-03-18):
+;;
+;; | emacs-version |  29.0.60 |
+;; | org-version   | main@4cad6c8ea (Mar 16 2023) |
+;; | haskell-mode  | master@20d4e23 (Mar 4  2023) |
+;; | ghci  |9.0.2 |
+
+
+;;; Code:
+;;
+
+(require 'org-test "../testing/org-test")
+(org-test-for-executable "ghci")
+(unless (featurep 'haskell-mode)
+  (signal 'missing-test-dependency "haskell-mode"))
+
+
+;;; Helpers
+;;
+
+(defun test-ob-haskell-ghci--with-global-session-worker (todo)
+  "See `test-ob-haskell-ghci--with-global-session-worker'."
+  (when (get-buffer "*haskell*")
+(error "A buffer named '*haskell*' exists.  Can't safely test haskell blocks"))
+  (unwind-protect (funcall todo)
+;; Kill the "*haskell*" buffer to not pollute other tests.
+(when-let ((hb (get-buffer "*haskell*")))
+  (with-current-buffer hb
+(let ((kill-buffer-query-functions nil)
+  (kill-buffer-hook nil))
+  (kill-buffer hb))
+
+(defmacro test-ob-haskell-ghci-with-global-session ( body)
+  "Eval BODY in a new session, then destroy the session.
+The library ob-haskell doesn't implement session yet.  It will
+always use a buffer named \"*haskell*\".  We kill that buffer
+after the source block execution.  To 

Re: [PATCH] Add tests for ob-haskell (GHCi)

[Ihor Radchenko]

ParetoOptimalDev  writes:

>> comint-prompt-regexp: ^[[:alnum:].*_() |λ]*> \|^λ?>
>> Output: ghci| ghci| ghci| ghci| ghci| 6
>>
>> Note "|".
>
> You can fix this by sending `set :prompt-cont ""`. Here is a
> demonstration:

I think you meant `:set prompt-cont ""`.

It is a good idea, thanks!
We use similar approach in ob-shell.

Note that setting the prompt to non-"", makes little sense - we want to
remove it anyway. Why do extra work?

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

Re: [PATCH] Add tests for ob-haskell (GHCi)

[ParetoOptimalDev]

I forgot to mention you can also set the prompt as well to something
simple like `:set prompt "> "`.

> Ihor Radchenko  writes:
>
>> Part of the problem is that haskell-mode appears to use non-standard
>> prompt:
>>
>> comint-prompt-regexp: ^[[:alnum:].*_() |λ]*> \|^λ?>
>> Output: ghci| ghci| ghci| ghci| ghci| 6
>>
>> Note "|".
>
> You can fix this by sending `set :prompt-cont ""`. Here is a
> demonstration:
>
> GHCi, version 9.0.2: https://www.haskell.org/ghc/  :? for help
> ghci> ghci> :set prompt-cont ""
> ghci> :{
> let { x = 2
> ; y = 3
> }
> in x*y
> :}
> 6
> ghci> 

Re: [PATCH] Add tests for ob-haskell (GHCi)

[ParetoOptimalDev]

Ihor Radchenko  writes:

> Part of the problem is that haskell-mode appears to use non-standard
> prompt:
>
> comint-prompt-regexp: ^[[:alnum:].*_() |λ]*> \|^λ?>
> Output: ghci| ghci| ghci| ghci| ghci| 6
>
> Note "|".

You can fix this by sending `set :prompt-cont ""`. Here is a
demonstration:

GHCi, version 9.0.2: https://www.haskell.org/ghc/  :? for help
ghci> ghci> :set prompt-cont ""
ghci> :{
let { x = 2
; y = 3
}
in x*y
:}
6
ghci> 

Re: [PATCH] Add tests for ob-haskell (GHCi)

[Ihor Radchenko]

Bruno Barbier  writes:

> +(ert-deftest ob-haskell/let-multilines-1 ()
> +  "Local definitions on multiple lines."
> +  :expected-result :failed
> +  (should (equal 6 (test-ob-haskell-ghci "" "
> +:{
> + let { x=2
> + ; y=3
> + }
> + in x*y
> +:}
> +"

Part of the problem is that haskell-mode appears to use non-standard
prompt:

comint-prompt-regexp: ^[[:alnum:].*_() |λ]*> \|^λ?>
Output: ghci| ghci| ghci| ghci| ghci| 6

Note "|".

We may want to either report this upstream or modify
`org-babel-interpret-haskell' further where it extends
`comint-prompt-regexp'.

> +(ert-deftest ob-haskell/ghci-info ()
> +  "The ghci meta command ':info' ."
> +  (should (equal "repeat :: a -> [a]-- Defined in ‘GHC.List’"
> + (test-ob-haskell-ghci ":results output" ":info repeat"

On my system, the output contains different number of spaces in
"[a]-- ". It appears to be system-dependent. Looks like test's
fault.

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

Re: [PATCH] Add tests for ob-haskell (GHCi)

[Ihor Radchenko]

Bruno Barbier  writes:

> Ihor Radchenko  writes:
>
>> From a first look, random failures appear to be related to session
>> initialization.
>
> My function 'test-ob-haskell-ghci' should protect against that; it
> ensures the "*haskell*" buffer is always new.  From what I understand,
> this is an input buffering problem: inputs are not full lines.

I see.

We can try the attached patch. Yet another edge case in comint, it
appears.

>From 2bdcec1e1f9a3ce0314d182a4c50887e8ec80a17 Mon Sep 17 00:00:00 2001
Message-Id: <2bdcec1e1f9a3ce0314d182a4c50887e8ec80a17.1679480064.git.yanta...@posteo.net>
From: Ihor Radchenko 
Date: Wed, 22 Mar 2023 11:12:30 +0100
Subject: [PATCH] org-babel-comint-with-output: Handle agglomerated prompts
 better

* lisp/ob-comint.el (org-babel-comint-with-output): Consider that
comint can sometimes agglomerate multiple prompts together even within
a single output increment as passed to
`comint-output-filter-functions'.

Example in GHC comint buffer:

GHCi, version 9.0.2: https://www.haskell.org/ghc/  :? for help
ghci> ghci> :{
main :: IO ()
main = putStrLn "Hello World!"
:}

main
"org-babel-haskell-eoe"
ghci| ghci| ghci| ghci> ghci> Hello World!
ghci> "org-babel-haskell-eoe"
ghci>
---
 lisp/ob-comint.el | 11 ++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/lisp/ob-comint.el b/lisp/ob-comint.el
index 54bf5127e..e167d911b 100644
--- a/lisp/ob-comint.el
+++ b/lisp/ob-comint.el
@@ -91,7 +91,16 @@ (defmacro org-babel-comint-with-output (meta  body)
   ;; trailing newline.  Use more reliable
   ;; match to split the output later.
   (replace-regexp-in-string
-   comint-prompt-regexp
+   ;; Sometimes, we get multiple agglomerated
+   ;; prompts together in a single output:
+   ;; "prompt prompt prompt output"
+   ;; Remove them progressively, so that
+   ;; possible "^" in the prompt regexp gets to
+   ;; work as we remove the heading prompt
+   ;; instance.
+   (if (string-prefix-p "^" comint-prompt-regexp)
+   (format "^\\(%s\\)+" (substring comint-prompt-regexp 1))
+ comint-prompt-regexp)
,org-babel-comint-prompt-separator
text
 		 comint-output-filter-functions))
-- 
2.39.1


>> It appears that ob-haskell is re-using sessions even
>> if :session is nil (default). And multiple session not allowed?? (I am
>> looking at `org-babel-haskell-initiate-session', which ignores session
>> parameter completely).
>
> From what I understood, the buffer "*haskell*" is coming from
> haskell-mode. And, to make it work might require some changes there too.

>From my reading of haskell-mode, it seems to be safe if we simply rename
Haskell process buffer upon session initialization.

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

Re: [PATCH] Add tests for ob-haskell (GHCi)

[Bruno Barbier]

Hi Ihor,

Ihor Radchenko  writes:

> From a first look, random failures appear to be related to session
> initialization.

My function 'test-ob-haskell-ghci' should protect against that; it
ensures the "*haskell*" buffer is always new.  From what I understand,
this is an input buffering problem: inputs are not full lines.

> It appears that ob-haskell is re-using sessions even
> if :session is nil (default). And multiple session not allowed?? (I am
> looking at `org-babel-haskell-initiate-session', which ignores session
> parameter completely).

>From what I understood, the buffer "*haskell*" is coming from
haskell-mode. And, to make it work might require some changes there too.

Thanks for having a look,
Let me know,


Bruno


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

Re: [PATCH] Add tests for ob-haskell (GHCi)

[Ihor Radchenko]

Ihor Radchenko  writes:

> From a first look, random failures appear to be related to session
> initialization. It appears that ob-haskell is re-using sessions even
> if :session is nil (default). And multiple session not allowed?? (I am
> looking at `org-babel-haskell-initiate-session', which ignores session
> parameter completely).

Lawrence, may you please take a look at this thread?

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

Re: [PATCH] Add tests for ob-haskell (GHCi)

[Ihor Radchenko]

Bruno Barbier  writes:

> All the tests will randomly fail; that is an ob-haskell bug (see
> the command 'test-ob-haskell-ghci.el' to make them fail).

First of all, thanks for the patch! Tests are especially welcome.

>From a first look, random failures appear to be related to session
initialization. It appears that ob-haskell is re-using sessions even
if :session is nil (default). And multiple session not allowed?? (I am
looking at `org-babel-haskell-initiate-session', which ignores session
parameter completely).

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

[PATCH] Add tests for ob-haskell (GHCi)

[Bruno Barbier]

Hi all,

I've collected some tests about ob-haskell, using GHCi.

This patch is not ready for final review.  It's a preliminary version.

These tests are about GHCi, and only about GHCi (that's why I named
the file 'test-ob-haskell-ghci.el').


Some tests are revealing bugs; they are flagged with:

:expected-result :failed


All the tests will randomly fail; that is an ob-haskell bug (see
the command 'test-ob-haskell-ghci.el' to make them fail).


Some tests might be questionable, revealing more design choices and/or
backend capabilities than the expected behavior.


Solving the random failure bug should be a priority, in my opinion, as
it makes the testing process unreliable.  Adding support for sessions
would also be a good thing, so that one test doesn't contaminate others.


FTR, here are the tools that I have used:

| emacs-version |  29.0.60 |
| org-version   | main@4cad6c8ea (Mar 16 2023) |
| haskell-mode  | master@20d4e23 (Mar 4  2023) |
| ghci  |9.0.2 |



Bruno

>From c085fac2fcb429f7e643df8e4fff3a4ae1665d07 Mon Sep 17 00:00:00 2001
From: Bruno BARBIER 
Date: Fri, 18 Nov 2022 20:14:20 +0100
Subject: [PATCH] ob-haskell: Add tests for GHCi

testing/lisp/test-ob-haskell-ghci.el: New file.
---
 testing/lisp/test-ob-haskell-ghci.el | 428 +++
 1 file changed, 428 insertions(+)
 create mode 100644 testing/lisp/test-ob-haskell-ghci.el

diff --git a/testing/lisp/test-ob-haskell-ghci.el b/testing/lisp/test-ob-haskell-ghci.el
new file mode 100644
index 0..4b1e4669b
--- /dev/null
+++ b/testing/lisp/test-ob-haskell-ghci.el
@@ -0,0 +1,428 @@
+;;; test-ob-haskell-ghci.el --- tests for ob-haskell.el GHCi  -*- lexical-binding: t; -*-
+
+;; Copyright (c) 2023  Free Software Foundation, Inc.
+
+;; Authors: Bruno BARBIER 
+
+;; This program is free software; you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; This program is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with this program.  If not, see <https://www.gnu.org/licenses/>.
+
+;;; Commentary:
+;;
+
+ Useful references
+;;
+;;  - https://orgmode.org/worg/org-contrib/babel/languages/lang-compat.html
+
+ FIXME: Random failures
+;;
+;; To increase the chances of failure when running tests, you can use this command line:
+;;
+;;(for I in 0 1 2 3 4 5 6 7 8 9 10 0 1 2 3 4 5 6 7 8 9 10 0 1 2 3 4 5 6 7 8 9 10; do make 'BTEST_OB_LANGUAGES=haskell' BTEST_RE='haskell' test-dirty & done) 2>&1 | grep FAILED
+;;
+
+ Status
+;;
+;; All the tests should succeed (except for random failures); those
+;; flagged with ":expected-result :failed" are known
+;; limitations/bugs.  Tested with (2023-03-18):
+;;
+;; | emacs-version |  29.0.60 |
+;; | org-version   | main@4cad6c8ea (Mar 16 2023) |
+;; | haskell-mode  | master@20d4e23 (Mar 4  2023) |
+;; | ghci  |9.0.2 |
+
+
+;;; Code:
+;;
+
+(require 'org-test "../testing/org-test")
+(org-test-for-executable "ghci")
+(unless (featurep 'haskell-mode)
+  (signal 'missing-test-dependency "haskell-mode"))
+
+
+;;; Helpers
+;;
+
+(defun test-ob-haskell-ghci--with-global-session-worker (todo)
+  "See `test-ob-haskell-ghci--with-global-session-worker'."
+  (when (get-buffer "*haskell*")
+(error "A buffer named '*haskell*' exists.  Can't safely test haskell blocks"))
+  (unwind-protect (funcall todo)
+;; Kill the "*haskell*" buffer to not pollute other tests.
+(when-let ((hb (get-buffer "*haskell*")))
+  (with-current-buffer hb
+(let ((kill-buffer-query-functions nil)
+  (kill-buffer-hook nil))
+  (kill-buffer hb))
+
+(defmacro test-ob-haskell-ghci-with-global-session ( body)
+  "Eval BODY in a new session, then destroy the session.
+The library ob-haskell doesn't implement session yet.  It will
+always use a buffer named \"*haskell*\".  We kill that buffer
+after the source block execution.  To be safe, we fail if such a
+buffer already exists."
+  `(test-ob-haskell-ghci--with-global-session-worker (lambda () ,@body)))
+
+(defun test-ob-haskell-ghci (args content  preamble unprotected)
+  "Execute the code block CONTENT in a new GHCi session; return the result.
+Add ARGS to the code block argument line.  Insert PREAMBLE
+before the code block.  When UNPROTECTED is non-nil, don't control
+which session is used (i.e. don't call
+`test-ob-ha