From: Nicolas Goaziou <mail@nicolasgoaziou.fr>
To: Christian Garbs <mitch@mitch.h.shuttle.de>
Cc: Christian Garbs <mitch@cgarbs.de>, emacs-orgmode@gnu.org
Subject: Re: [PATCH] ob-vala.el: Add Vala support to Babel
Date: Sun, 23 Jul 2017 11:05:02 +0200 [thread overview]
Message-ID: <87379no75t.fsf@nicolasgoaziou.fr> (raw)
In-Reply-To: <1500061387-3353-1-git-send-email-mitch@cgarbs.de> (Christian Garbs's message of "Fri, 14 Jul 2017 21:43:07 +0200")
Hello,
Christian Garbs <mitch@mitch.h.shuttle.de> writes:
> * ob-vala.el: Add support for the Vala language to Babel.
Thank you. Comments follow.
> +;; file extension
;; File extension.
> +;; header arguments empty by default
;; Header argument empty by default.
> +(defvar org-babel-vala-compiler "valac"
> + "Command used to compile a Vala source code file into an
> +executable.")
The first line of a docstring should be a full sentence.
Also, shouldn't this be a defcustom?
> +(defun org-babel-expand-body:vala (body params &optional processed-params)
> + "Expand BODY: does nothing, returns original BODY while ignoring PARAMS."
> + body ;; TODO: expand params?
> + )
> +
> +;; This is the main function which is called to evaluate a code
> +;; block.
> +;;
> +;; - run Vala compiler and create a binary in a temporary file
> +;; - compiler/linker flags can be set via :flags header argument
> +;; - if compilation succeeded, run the binary
> +;; - commandline parameters to the binary can be set via :cmdline
> +;; header argument
> +;; - stdout will be parsed as RESULT (control via :result-params
> +;; header argument)
> +;;
> +;; There is no session support because Vala is a compiled language.
> +;;
> +;; This function is heavily based on ob-C.el
> +(defun org-babel-execute:vala (body params)
> + "Execute a block of Vala code with org-babel.
"org-babel" -> "Babel"
> +This function is called by `org-babel-execute-src-block'"
> + (message "executing Vala source code block")
> + (let* ((tmp-src-file (org-babel-temp-file
> + "vala-src-"
> + ".vala"))
> + (tmp-bin-file (org-babel-temp-file "vala-bin-" org-babel-exeext))
> + (cmdline (cdr (assoc :cmdline params)))
> + (flags (cdr (assoc :flags params)))
`assoc' -> `assq'
> + (full-body (org-babel-expand-body:vala body params))
> + (compile
> + (progn
> + (with-temp-file tmp-src-file (insert full-body))
> + (org-babel-eval
> + (format "%s %s -o %s %s"
> + org-babel-vala-compiler
> + (mapconcat 'identity
'identity -> #'identity
> + (if (listp flags) flags (list flags)) " ")
> + (org-babel-process-file-name tmp-bin-file)
> + (org-babel-process-file-name tmp-src-file)) "")))
> + )
There should be no dangling parenthesis. Please move it at the end of
the line above.
> + (if (file-executable-p tmp-bin-file)
> + (let ((results
> + (org-trim
> + (org-babel-eval
> + (concat tmp-bin-file (if cmdline (concat " " cmdline) "")) ""))))
> + (org-babel-reassemble-table
> + (org-babel-result-cond (cdr (assoc :result-params params))
> + (org-babel-read results)
> + (let ((tmp-file (org-babel-temp-file "vala-")))
> + (with-temp-file tmp-file (insert results))
> + (org-babel-import-elisp-from-file tmp-file)))
> + (org-babel-pick-name
> + (cdr (assoc :colname-names params)) (cdr (assoc :colnames params)))
> + (org-babel-pick-name
> + (cdr (assoc :rowname-names params)) (cdr (assoc :rownames params)))
> + ))
> + )))
See above.
Also, it looks like you are using a one-armed `if'. In this case,
I suggest using (when ...) instead.
> +(defun org-babel-prep-session:vala (session params)
> + "This function does nothing as Vala is a compiled language with no
> +support for sessions"
> + (error "Vala is a compiled language -- no support for sessions"))
See remark about first line in a docstirng.
> +(defun org-babel-vala-var-to-vala (var)
> + "Convert an elisp var into a string of vala source code
> +specifying a var of the same value."
> + (format "%S" var))
Ditto.
> +(defun org-babel-vala-table-or-string (results)
> + "If the results look like a table, then convert them into an
> +Emacs-lisp table, otherwise return the results as a string."
> + )
The body of the function is missing.
Could you send an updated patch?
Regards,
--
Nicolas Goaziou
next prev parent reply other threads:[~2017-07-23 9:05 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-07-09 20:09 Babel support for Vala Christian Garbs
2017-07-10 9:03 ` Nicolas Goaziou
2017-07-10 17:54 ` Christian Garbs
2017-07-11 8:10 ` Nicolas Goaziou
2017-07-13 21:42 ` Christian Garbs
2017-07-14 8:30 ` Nicolas Goaziou
2017-07-14 19:43 ` [PATCH] ob-vala.el: Add Vala support to Babel Christian Garbs
2017-07-23 9:05 ` Nicolas Goaziou [this message]
2017-07-31 22:04 ` [PATCH v2] " Christian Garbs
2017-08-01 21:59 ` Nicolas Goaziou
2017-07-31 22:04 ` [PATCH v3] " Christian Garbs
2017-08-07 9:41 ` Nicolas Goaziou
2017-08-01 21:20 ` [PATCH] " Christian Garbs
2017-08-01 22:03 ` Nicolas Goaziou
2017-08-02 20:24 ` Christian Garbs
2017-08-03 10:49 ` Nicolas Goaziou
2017-08-05 19:53 ` Christian Garbs
2017-08-06 0:22 ` Nicolas Goaziou
2017-08-06 10:06 ` Christian Garbs
2017-08-07 9:52 ` Nicolas Goaziou
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=87379no75t.fsf@nicolasgoaziou.fr \
--to=mail@nicolasgoaziou.fr \
--cc=emacs-orgmode@gnu.org \
--cc=mitch@cgarbs.de \
--cc=mitch@mitch.h.shuttle.de \
/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).