all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
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 --]

  reply	other threads:[~2020-05-04 16:52 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-25 19:04 bug#40852: Support compilation of Haskell in org mode babel blocks Roland Coeurjoly
2020-04-27 18:20 ` bug#40852: Fix patch Roland Coeurjoly
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
2020-09-30  3:35 ` bug#40852: " Lars Ingebrigtsen
2020-10-26 20:18   ` Roland Coeurjoly
2020-10-26 20:26     ` bug#40852: (no subject) Lars Ingebrigtsen

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

* 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 external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.