contribution: ob-php

contribution: ob-php

[Hex]

[-- 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)))))
+
+

Re: contribution: ob-php

[Nicolas Goaziou]

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