all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: Nicolas Goaziou <mail@nicolasgoaziou.fr>
To: Roland Coeurjoly <rolandcoeurjoly@gmail.com>
Cc: emacs-orgmode@gnu.org
Subject: Re: Fwd: Support compilation of Haskell in org mode babel blocks.
Date: Mon, 04 May 2020 17:26:00 +0200	[thread overview]
Message-ID: <87v9lb3enr.fsf@nicolasgoaziou.fr> (raw)
In-Reply-To: <CAMdauZop0iFB7Dw=bxfsfeX7JXdF4dAYj4KnODBBCmTuxin55Q@mail.gmail.com> (Roland Coeurjoly's message of "Tue, 28 Apr 2020 16:56:37 +0200")

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


  reply	other threads:[~2020-05-04 15:27 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 [this message]
2020-05-04 16:39     ` Roland Coeurjoly
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=87v9lb3enr.fsf@nicolasgoaziou.fr \
    --to=mail@nicolasgoaziou.fr \
    --cc=emacs-orgmode@gnu.org \
    --cc=rolandcoeurjoly@gmail.com \
    /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.