* contribution: ob-php
@ 2015-01-12 20:10 Hex
2015-01-13 11:00 ` Nicolas Goaziou
0 siblings, 1 reply; 2+ messages in thread
From: Hex @ 2015-01-12 20:10 UTC (permalink / raw)
To: Emacs-orgmode
[-- Attachment #1: Type: text/plain, Size: 981 bytes --]
I use org mode for code review quite a bit. It's awesome to be able to
throw snippets of code in a doc and be able to verify fixes within the
doc. I noticed there's no PHP mode (and totally understand why seeing
as how "literate programming" is a keyword ;) so I wrote one.
Elsip isn't exactly a language I hit a lot, so it might be pretty
rough. I added the following files:
lisp/ob-php.el
testing/examples/ob-php-test.org
testing/lisp/test-ob-php.el
Unfortunately I wasn't able to run the test suite even against the
clean pull...
> Eager macro-expansion failure: (void-variable test-line)
> Symbol's value as variable is void: test-line
... but I ran all tests by hand and they passed, so I'd be surprised
if they didn't run in the suite.
I also submitted an assignment contract request to the FSF. The patch
is attached. It's against rev 2668d9e9cecce37d5598275fce380842f5a4a28c.
Is there anything else I need to do to get this code included? Can
someone review this?
[-- Attachment #2: patch.diff --]
[-- Type: text/x-diff, Size: 9286 bytes --]
diff --git a/lisp/ob-php.el b/lisp/ob-php.el
new file mode 100644
index 0000000..daaad07
--- /dev/null
+++ b/lisp/ob-php.el
@@ -0,0 +1,119 @@
+;;; ob-php.el --- org-babel functions for php evaluation
+
+;; Copyright (C) 2014 Josh Dukes
+
+;; Author: Josh Dukes
+;; Keywords: literate programming, reproducible research
+;; Homepage: http://orgmode.org
+;; Version: 0.01
+
+;; This file is part of GNU Emacs.
+
+;; GNU Emacs is free software: you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; GNU Emacs is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with GNU Emacs. If not, see <http://www.gnu.org/licenses/>.
+
+;;; Commentary:
+
+;; Org-Babel support for evaluating php source code.
+
+;;; Requirements:
+
+;; php and php-mode
+
+;;; Code:
+(require 'ob)
+
+;; optionally define a file extension for this language
+(add-to-list 'org-babel-tangle-lang-exts '("php" . "php"))
+
+(defvar org-babel-php-command "php"
+"Name of command for executing PHP code.")
+
+;; This function expands the body of a source code block by doing
+;; things like prepending argument definitions to the body, it is
+;; be called by the `org-babel-execute:php' function below.
+(defun org-babel-expand-body:php (body params &optional processed-params)
+ "Expand BODY according to PARAMS, return the expanded body."
+ (if (or (car (assoc :expand params)) nil)
+ (let ((full-body
+ (org-babel-expand-body:generic
+ body
+ params
+ (org-babel-variable-assignments:php params))))
+ (concat "<?php\n"
+ full-body
+ "\n?>"))
+ (let ((vars
+ (concat "<?php\n"
+ (mapconcat
+ #'identity
+ (org-babel-variable-assignments:php params)
+ "\n")
+ "\n?>")))
+ (concat vars body))))
+
+(defun org-babel-prep-session:php (session params)
+ "Prepare SESSION according to the header arguments in PARAMS."
+ (error "Sessions are not supported for PHP"))
+
+(defun org-babel-php-initiate-session (&optional session params)
+ "Return nil because sessions are not supported by PHP."
+ nil)
+
+(defun org-babel-execute:php (body params)
+ (let* ((session (cdr (assoc :session params)))
+ (flags (or (cdr (assoc :flags params)) ""))
+ (src-file (org-babel-temp-file "php-src-"))
+ (full-body (org-babel-expand-body:php body params))
+ (session (org-babel-php-initiate-session session))
+ (results
+ (progn
+ (with-temp-file src-file (insert full-body))
+ (org-babel-eval
+ (concat org-babel-php-command
+ " " flags " " src-file) ""))))
+ (progn
+ (org-babel-reassemble-table
+ (org-babel-result-cond (cdr (assoc :result-params params))
+ (org-babel-read results)
+ (let ((tmp-file (org-babel-temp-file "c-")))
+ (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)))))))
+
+(defun org-babel-variable-assignments:php (params)
+ "Return a list of PHP statements assigning the block's variables."
+ (mapcar
+ (lambda (pair)
+ (format "$%s=%s;"
+ (car pair)
+ (org-babel-php-var-to-php (cdr pair))))
+ (mapcar #'cdr (org-babel-get-header params :var))))
+
+(defun org-babel-php-var-to-php (var)
+ "Convert an elisp var into a string of php source code
+specifying a var of the same value."
+ (if (listp var)
+ (concat "Array(" (mapconcat #'org-babel-php-var-to-php var ",") ")")
+ (format "%S" var)))
+
+(defun org-babel-php-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."
+ (org-babel-script-escape results))
+
+(provide 'ob-php)
+;;; ob-php.el ends here
diff --git a/testing/examples/ob-php-test.org b/testing/examples/ob-php-test.org
new file mode 100644
index 0000000..f1c901c
--- /dev/null
+++ b/testing/examples/ob-php-test.org
@@ -0,0 +1,98 @@
+* php test expand test
+ :PROPERTIES:
+ :ID: 39b75bdf-0f2a-4e7a-a03c-4c2bfa96bf60
+ :END:
+#+source: hello
+#+begin_src php :expand true
+ echo "hello world";
+#+end_src
+
+#+RESULTS:
+: hello world
+
+#+source: intreturn
+#+begin_src php :expand true
+ echo 42;
+#+end_src
+
+#+RESULTS:
+: 42
+
+#+source: expandvar
+#+begin_src php :var foo="bar" :var baz=1 :expand true
+ echo "$baz $foo";
+#+end_src
+
+#+RESULTS:
+: 1 bar
+
+#+source: multiline
+#+begin_src php :expand true
+ echo "php!\n";
+ echo "sucks!";
+#+end_src
+
+#+RESULTS:
+| php! |
+| sucks! |
+
+* php noexpand test
+ :PROPERTIES:
+ :ID: 47f2043a-abc9-4059-92bf-5df939e6881b
+ :END:
+#+source: externalstuff
+#+begin_src php
+ outside php
+ <?php echo "inside php\n"?>
+#+end_src
+
+#+RESULTS:
+| outside | php |
+| inside | php |
+
+* php var array test
+ :PROPERTIES:
+ :ID: 2472b4b4-6582-44ca-844d-6a8299dd728b
+ :END:
+#+name: atable
+| a | b |
+| c | duck |
+
+#+source: fromvar
+#+begin_src php :var foo='("baz" "bar")
+ <?php
+ foreach($foo as $val){
+ echo "value: $val\n";
+ }
+ ?>
+#+end_src
+
+#+RESULTS:
+| value: | baz |
+| value: | bar |
+
+#+source: fromvarexpand
+#+begin_src php :var foo='("baz" "bar") :expand true
+ foreach($foo as $val){
+ echo "value: $val\n";
+ }
+#+end_src
+
+#+RESULTS:
+| value: | baz |
+| value: | bar |
+
+#+source: fromtableexpand
+#+begin_src php :var foo=atable :expand true
+ foreach($foo as $row){
+ foreach($row as $val)
+ echo "value: $val\n";
+ }
+#+end_src
+
+#+RESULTS:
+| value: | a |
+| value: | b |
+| value: | c |
+| value: | duck |
+
diff --git a/testing/lisp/test-ob-php.el b/testing/lisp/test-ob-php.el
new file mode 100644
index 0000000..47a1169
--- /dev/null
+++ b/testing/lisp/test-ob-php.el
@@ -0,0 +1,82 @@
+;;; test-ob-php.el --- tests for ob-php.el
+
+;; Copyright (c) 2014 Josh Dukes
+;; Authors: Josh Dukes
+
+;; This file is not part of GNU Emacs.
+
+;; This program is free software; you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; This program is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+;;; Code:
+(unless (featurep 'ob-php)
+ (signal 'missing-test-dependency "Support for PHP code blocks"))
+
+(ert-deftest ob-php/assert ()
+ (should t))
+
+;;expand tests
+
+(ert-deftest ob-php/hello-world ()
+ "Hello world program."
+ (org-test-at-id "39b75bdf-0f2a-4e7a-a03c-4c2bfa96bf60"
+ (org-babel-next-src-block)
+ (should (equal "hello world" (org-babel-execute-src-block)))))
+
+(ert-deftest ob-php/intreturn ()
+ "return an int."
+ (org-test-at-id "39b75bdf-0f2a-4e7a-a03c-4c2bfa96bf60"
+ (org-babel-next-src-block 2)
+ (should (= 42 (org-babel-execute-src-block)))))
+
+(ert-deftest ob-php/expandvar ()
+ "expand with variables."
+ (org-test-at-id "39b75bdf-0f2a-4e7a-a03c-4c2bfa96bf60"
+ (org-babel-next-src-block 3)
+ (should (equal "1 bar" (org-babel-execute-src-block)))))
+
+(ert-deftest ob-php/multiline ()
+ "expand with variables."
+ (org-test-at-id "39b75bdf-0f2a-4e7a-a03c-4c2bfa96bf60"
+ (org-babel-next-src-block 4)
+ (should (equal '(("php!")("sucks!")) (org-babel-execute-src-block)))))
+
+;; noexpand tests
+
+(ert-deftest ob-php/external-block ()
+ "outside php block, inside php block."
+ (org-test-at-id "47f2043a-abc9-4059-92bf-5df939e6881b"
+ (org-babel-next-src-block)
+ (should (equal '(("outside" "php")("inside" "php")) (org-babel-execute-src-block)))))
+
+;;array tests
+
+(ert-deftest ob-php/fromvar ()
+ "get value from variable in an unexpanded block."
+ (org-test-at-id "2472b4b4-6582-44ca-844d-6a8299dd728b"
+ (org-babel-next-src-block)
+ (should (equal '(("value:" "baz")("value:" "bar")) (org-babel-execute-src-block)))))
+
+(ert-deftest ob-php/fromvarexpand ()
+ "get value from variable in an expanded block."
+ (org-test-at-id "2472b4b4-6582-44ca-844d-6a8299dd728b"
+ (org-babel-next-src-block 2)
+ (should (equal '(("value:" "baz")("value:" "bar")) (org-babel-execute-src-block)))))
+
+(ert-deftest ob-php/fromtableexpand ()
+ "get value from table in an expanded block."
+ (org-test-at-id "2472b4b4-6582-44ca-844d-6a8299dd728b"
+ (org-babel-next-src-block 3)
+ (should (equal '(("value:" "a")("value:" "b")("value:" "c")("value:" "duck")) (org-babel-execute-src-block)))))
+
+
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: contribution: ob-php
2015-01-12 20:10 contribution: ob-php Hex
@ 2015-01-13 11:00 ` Nicolas Goaziou
0 siblings, 0 replies; 2+ messages in thread
From: Nicolas Goaziou @ 2015-01-13 11:00 UTC (permalink / raw)
To: Hex; +Cc: Emacs-orgmode
Hello,
Hex <hex@neg9.org> writes:
> I use org mode for code review quite a bit. It's awesome to be able to
> throw snippets of code in a doc and be able to verify fixes within the
> doc. I noticed there's no PHP mode (and totally understand why seeing
> as how "literate programming" is a keyword ;) so I wrote one.
>
> Elsip isn't exactly a language I hit a lot, so it might be pretty
> rough. I added the following files:
>
> lisp/ob-php.el
> testing/examples/ob-php-test.org
> testing/lisp/test-ob-php.el
Thanks for your work.
However, we can only apply it on master branch once FSF assignment is
completed. It can take a couple of weeks, depending on your location.
Also, please use git format-patch with a proper commit message when
providing a patch.
Some comments follow.
> +;; Copyright (C) 2014 Josh Dukes
This will need to be the "Free Software Foundation, Inc."
> +;; This function expands the body of a source code block by doing
> +;; things like prepending argument definitions to the body, it is
> +;; be called by the `org-babel-execute:php' function below.
> +(defun org-babel-expand-body:php (body params &optional processed-params)
> + "Expand BODY according to PARAMS, return the expanded body."
You should give the type of BODY (probably a string) and PARAMS (a
plist), and describe PROCESSED-PARAMS.
> + (if (or (car (assoc :expand params)) nil)
(if (car (assq :expand params)) ...)
Also, it is probably `cdr' instead of `car'.
> + (let ((full-body
> + (org-babel-expand-body:generic
> + body
> + params
> + (org-babel-variable-assignments:php params))))
> + (concat "<?php\n"
> + full-body
> + "\n?>"))
IMO, this is unnecessarily convoluted
(format "<?php\n%s\n?>"
(org-babel-expand-body:generic
...))
> + (let ((vars
> + (concat "<?php\n"
> + (mapconcat
> + #'identity
> + (org-babel-variable-assignments:php params)
> + "\n")
> + "\n?>")))
> + (concat vars body))))
Ditto.
(concat "<?php\n"
(mapconcat ...)
"\n?>"
body)
> +(defun org-babel-execute:php (body params)
Missing docstring.
> + (let* ((session (cdr (assoc :session params)))
> + (flags (or (cdr (assoc :flags params)) ""))
`assoc' -> `assq'
> + (src-file (org-babel-temp-file "php-src-"))
> + (full-body (org-babel-expand-body:php body params))
> + (session (org-babel-php-initiate-session session))
> + (results
> + (progn
> + (with-temp-file src-file (insert full-body))
> + (org-babel-eval
> + (concat org-babel-php-command
> + " " flags " " src-file) ""))))
> + (progn
`progn' is implicit around the body of `let' (or `let*'). You can remove
it.
> + (org-babel-reassemble-table
> + (org-babel-result-cond (cdr (assoc :result-params params))
> + (org-babel-read results)
> + (let ((tmp-file (org-babel-temp-file "c-")))
> + (with-temp-file tmp-file (insert results))
> + (org-babel-import-elisp-from-file tmp-file)))
Is the temporary file necessary? `with-temp-buffer' is cheaper. Use it
if possible.
> + (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)))))))
`assoc' -> `assq'
> +
> +(defun org-babel-variable-assignments:php (params)
> + "Return a list of PHP statements assigning the block's variables."
You need to describe PARAMS.
> + (mapcar
> + (lambda (pair)
> + (format "$%s=%s;"
> + (car pair)
> + (org-babel-php-var-to-php (cdr pair))))
> + (mapcar #'cdr (org-babel-get-header params :var))))
Nitpick: can't you do it with a single `mapcar'?
> +(defun org-babel-php-var-to-php (var)
> + "Convert an elisp var into a string of php source code
> +specifying a var of the same value."
First sentence of docstring has to fit on a single line.
> + (if (listp var)
> + (concat "Array(" (mapconcat #'org-babel-php-var-to-php var ",") ")")
> + (format "%S" var)))
> +
> +(defun org-babel-php-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."
See above.
> +++ b/testing/examples/ob-php-test.org
I know Babel usually relies on an external Org file for its tests, but
this is a pain to debug when one fails.
It is not terribly important, but write self-contained tests if you can,
i.e., using `org-test-with-temp-text'.
> diff --git a/testing/lisp/test-ob-php.el b/testing/lisp/test-ob-php.el
> new file mode 100644
> index 0000000..47a1169
> --- /dev/null
> +++ b/testing/lisp/test-ob-php.el
[...]
> +(ert-deftest ob-php/assert ()
> + (should t))
You can remove that.
Regards,
--
Nicolas Goaziou
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2015-01-13 10:59 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-12 20:10 contribution: ob-php Hex
2015-01-13 11:00 ` Nicolas Goaziou
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).