From: Roland Coeurjoly <rolandcoeurjoly@gmail.com>
To: Roland Coeurjoly <rolandcoeurjoly@gmail.com>, emacs-orgmode@gnu.org
Subject: Re: Fwd: Support compilation of Haskell in org mode babel blocks.
Date: Mon, 4 May 2020 18:39:27 +0200 [thread overview]
Message-ID: <CAMdauZpCQQdTbVuhEjqnzetLf0EOZ40EpR6RnD_MrWHKXq6uaw@mail.gmail.com> (raw)
In-Reply-To: <87v9lb3enr.fsf@nicolasgoaziou.fr>
[-- 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 --]
next prev parent reply other threads:[~2020-05-04 16:52 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <CAMdauZqv91_bW2aZj=AT32vBdyPia92vjeVFMmsFazsEhJCTHg@mail.gmail.com>
2020-04-28 14:56 ` Fwd: Support compilation of Haskell in org mode babel blocks Roland Coeurjoly
2020-05-04 15:26 ` Nicolas Goaziou
2020-05-04 16:39 ` Roland Coeurjoly [this message]
2020-05-04 21:49 ` Nicolas Goaziou
2020-05-05 21:16 ` Roland Coeurjoly
2020-05-05 21:31 ` Nicolas Goaziou
2020-05-05 21:42 ` Roland Coeurjoly
2020-05-22 7:04 ` Roland Coeurjoly
2020-05-22 15:30 ` Nicolas Goaziou
2020-05-22 16:17 ` Roland Coeurjoly
2020-05-23 5:12 ` Kyle Meyer
2020-05-23 10:30 ` Roland Coeurjoly
2020-05-23 14:02 ` Bastien
2020-05-23 14:51 ` Roland Coeurjoly
2020-05-23 15:21 ` Bastien
2020-05-25 7:19 ` Nicolas Goaziou
2020-05-04 15:29 ` Nicolas Goaziou
2020-05-04 15:42 ` Roland Coeurjoly
2020-05-04 21:52 ` Nicolas Goaziou
2020-05-05 21:23 ` Roland Coeurjoly
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.orgmode.org/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAMdauZpCQQdTbVuhEjqnzetLf0EOZ40EpR6RnD_MrWHKXq6uaw@mail.gmail.com \
--to=rolandcoeurjoly@gmail.com \
--cc=emacs-orgmode@gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).