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

[Roland Coeurjoly <rolandcoeurjoly@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 5857 bytes --]

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
<https://emacs.stackexchange.com/questions/21245/dealing-with-warning-assignment-to-free-variable-when-certain-libraries-can-b>,
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 <mail@nicolasgoaziou.fr>
wrote:

> Hello,
>
> Roland Coeurjoly <rolandcoeurjoly@gmail.com> writes:
>
> > From 091f470a278561a60fac1ee3ee658f6823bc2503 Mon Sep 17 00:00:00 2001
> > From: Roland Coeurjoly <rolandcoeurjoly@gmail.com>
> > 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
>

[-- Attachment #2: Type: text/html, Size: 7627 bytes --]