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