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 &rest 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