Re: Fwd: Support compilation of Haskell in org mode babel blocks.

[Roland Coeurjoly]

I am confused about the last point:

Use a let-binding instead of setq.

If I use:

(let* compile (string= (cdr (assq :compile params)) "yes"))

I get the following error:

ob-haskell.el:158:1: Error: Wrong type argument: listp, compile

If I leave with set:

(setq compile (string= (cdr (assq :compile params)) "yes"))

I get the following warnings:

ob-haskell.el:161:12: Warning: assignment to free variable ‘compile’
ob-haskell.el:161:12: Warning: reference to free variable ‘compile’

Upon searching the web, I find a relevant this relevant post, whose conclusion (if I understand it correctly) is that we could live with those warning raised by setq.

Apart from this issue I am ready to submit a fixed patch.

What do you recommend?

On Mon, May 4, 2020 at 5:26 PM Nicolas Goaziou <address@hidden> wrote:

Hello,

Roland Coeurjoly <address@hidden> writes:

> From 091f470a278561a60fac1ee3ee658f6823bc2503 Mon Sep 17 00:00:00 2001
> From: Roland Coeurjoly <address@hidden>
> Date: Sat, 25 Apr 2020 20:35:22 +0200
> Subject: [PATCH] Add Haskell specific header argument compile, to compile
>  instead of interpret the body of source block

Thank you!

Could you rewrite the commit message so that it is on par with our
conventions?

It could be something like:

  ob-haskell: introduce :compile header argument

  * lisp/ob-haskell (org-babel-haskell-compiler):
  (org-babel-header-args:haskell): new variables.
  (org-babel-haskell-execute):
  (org-babel-haskell-interpret): new functions.
  (org-babel-execute:haskell): use new functions.

> -;; Org-Babel support for evaluating haskell source code.  This one will
> -;; be sort of tricky because haskell programs must be compiled before
> +;; Org-Babel support for evaluating Haskell source code.
> +;; Haskell programs must be compiled before

"Org Babel" would be even better, while you're changing this line.

> +(defcustom org-babel-Haskell-compiler "ghc"

No need to capitalize Haskell here.

> +  "Command used to compile a Haskell source code file into an executable.
> +May be either a command in the path, like ghc

like "ghc"

> +or an absolute path name, like /usr/local/bin/ghc

like "/usr/local/bin/ghc".

> +parameter may be used, like ghc -v"

The command can include a parameter, such as "ghc -v".

> +  :group 'org-babel
> +  :version "27.0"

It should be :package-version '(Org "9.4") instead.

> +  :type 'string)
> +
> +(defconst org-babel-header-args:haskell '((compile . :any))
> +  "Haskell-specific header arguments.")
> +
> +(defun org-babel-Haskell-execute (body params)
> +  "This function should only be called by `org-babel-execute:haskell'"
> +  (let* ((tmp-src-file (org-babel-temp-file
> +                     "Haskell-src-"
> +                        ".hs"))

Indentation seems a bit off.

> +         (tmp-bin-file
> +          (org-babel-process-file-name
> +           (org-babel-temp-file "Haskell-bin-" org-babel-exeext)))
> +         (cmdline (cdr (assq :cmdline params)))
> +         (cmdline (if cmdline (concat " " cmdline) ""))
> +         (flags (cdr (assq :flags params)))
> +         (flags (mapconcat 'identity

Nitpick: #'identity

> +                        (if (listp flags) flags (list flags)) " "))
> +         (libs (org-babel-read
> +             (or (cdr (assq :libs params))
> +                 (org-entry-get nil "libs" t))
> +             nil))

Ditto, mind the indentation.

> +         (libs (mapconcat #'identity
> +                       (if (listp libs) libs (list libs))
> +                       " ")))
> +    (with-temp-file tmp-src-file (insert body))
> +    (org-babel-eval
> +     (format "%s -o %s %s %s %s"
> +             org-babel-Haskell-compiler
> +          tmp-bin-file
> +          flags
> +          (org-babel-process-file-name tmp-src-file)
> +          libs) "")

Please move the empty string below.

> +    (let ((results
> +        (org-babel-eval
> +         (concat tmp-bin-file cmdline) "")))
> +      (when results
> +        (setq results (org-trim (org-remove-indentation results)))
> +        (org-babel-reassemble-table
> +         (org-babel-result-cond (cdr (assq :result-params params))
> +        (org-babel-read results t)
> +        (let ((tmp-file (org-babel-temp-file "Haskell-")))
> +          (with-temp-file tmp-file (insert results))
> +          (org-babel-import-elisp-from-file tmp-file)))
> +         (org-babel-pick-name
> +       (cdr (assq :colname-names params)) (cdr (assq :colnames params)))
> +         (org-babel-pick-name
> +       (cdr (assq :rowname-names params)) (cdr (assq :rownames params)))))
> +      )))

Please move the closing parens on the line above.
> +
> +(defun org-babel-interpret-Haskell (body params)

Why capitalization in function names?

>    (require 'inf-haskell)
>    (add-hook 'inferior-haskell-hook
>              (lambda ()
> @@ -96,6 +154,14 @@ org-babel-execute:haskell
>       (org-babel-pick-name (cdr (assq :rowname-names params))
>                         (cdr (assq :rowname-names params))))))
> 
> +
> +(defun org-babel-execute:haskell (body params)
> +  "Execute a block of Haskell code."
> +  (setq compile (string= (cdr (assq :compile params)) "yes"))

Use a let-binding instead of setq.


Could you send an updated patch?

Regards,

--
Nicolas Goaziou