Re: a patch to ob-lisp.el

["numbchild@gmail.com" <numbchild@gmail.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2066 bytes --]

I updated all comments which you point out.
I wandering how to sign the FSF papers? Sorry about this, this is my first
time to contribute code in open source.



[stardiviner]           <Hack this world!>      GPG key ID: 47C32433
IRC(freeenode): stardiviner                     Twitter:  @numbchild
Key fingerprint = 9BAA 92BC CDDD B9EF 3B36  CB99 B8C4 B8E5 47C3 2433
Blog: http://stardiviner.github.io/

On Mon, Mar 28, 2016 at 12:01 PM, Nick Dokos <ndokos@gmail.com> wrote:

> stardiviner <numbchild@gmail.com> writes:
>
> > I hope to merge this patch to Org-mode `ob-lisp.el`.
> >
>
> I have only looked at the surface of it, but I see some issues.
>
> > < ;; Copyright (C) 2016-2020 Free Software Foundation, Inc.
> > ---
> > > ;; Copyright (C) 2009-2016 Free Software Foundation, Inc.
>
> Why did you leave out the previous copyright statement?
> Where did 2020 come from?
>
> > < ;; Authors: stardiviner <numbchild@gmail.com>
> > < ;; Maintainer: stardiviner <numbchild@gmail.com>
> > < ;; Keywords: org babel lisp sly slime
> > < ;; URL: https://github.com/stardiviner/ob-lisp
> > < ;; Created: 1th March 2016
> > < ;; Version: 0.0.1
> > < ;; Package-Requires: ((org "8"))
> > ---
> >> ;; Authors: Joel Boehland
> >> ;;    Eric Schulte
> >> ;;    David T. O'Toole <dto@gnu.org>
> >> ;; Keywords: literate programming, reproducible research
> >> ;; Homepage: http://orgmode.org
>
> Why did you erase the previous authors?
>
> Are you volunteering to be the maintainer of ob-lisp.el or are you
> planning to keep your own repo and submit patches? Have you signed FSF
> papers?
>
> > < ;; Requires SLY (Sylvester the Cat's Common Lisp IDE) and SLIME
> > < ;; See:
> > < ;; - https://github.com/capitaomorte/sly
> > < ;; - http://common-lisp.net/project/slime/
> > ---
> >> ;; Requires SLIME (Superior Lisp Interaction Mode for Emacs.)
> >> ;; See http://common-lisp.net/project/slime/
>
> Does it really require both? If so, I suggest you rework it so
> either can be used, but only one is required. If not, I suggest
> you fix the comment.
>
> --
> Nick
>
>
>

[-- Attachment #1.2: Type: text/html, Size: 3892 bytes --]

[-- Attachment #2: ediff-output --]
[-- Type: application/octet-stream, Size: 6334 bytes --]

*** /home/stardiviner/Code/Emacs/ob-lisp/ob-lisp.el	2016-03-30 11:22:10.967967353 +0800
--- /home/stardiviner/.emacs.d/elpa/org-plus-contrib-20160321/ob-lisp.el	2016-03-30 11:21:04.847963494 +0800
***************
*** 1,4 ****
! ;;; ob-lisp.el --- org-babel functions for common lisp evaluation with SLY or SLIME.
  
  ;; Copyright (C) 2009-2016 Free Software Foundation, Inc.
  
--- 1,4 ----
! ;;; ob-lisp.el --- org-babel functions for common lisp evaluation
  
  ;; Copyright (C) 2009-2016 Free Software Foundation, Inc.
  
***************
*** 25,51 ****
  
  ;;; Commentary:
  
! ;;; Support for evaluating Common Lisp code, relies on SLY or SLIME for all eval.
  
  ;;; Requirements:
  
! ;; Requires SLY (Sylvester the Cat's Common Lisp IDE) or SLIME.
! ;; See:
! ;; - https://github.com/capitaomorte/sly
! ;; - http://common-lisp.net/project/slime/
  
  ;;; Code:
  (require 'ob)
  
! (defcustom org-babel-lisp-eval-fn "sly-eval"
!   "The function to be called to evaluate code on the Lisp side."
!   :group 'org-babel
!   :version "24.1"
!   :options '("sly-eval" "slime-eval")
!   :type 'stringp)
! 
! 
! ;; (declare-function sly-eval "ext:sly" (sexp &optional package))
  
  (defvar org-babel-tangle-lang-exts)
  (add-to-list 'org-babel-tangle-lang-exts '("lisp" . "lisp"))
--- 25,41 ----
  
  ;;; Commentary:
  
! ;;; support for evaluating common lisp code, relies on slime for all eval
  
  ;;; Requirements:
  
! ;; Requires SLIME (Superior Lisp Interaction Mode for Emacs.)
! ;; See http://common-lisp.net/project/slime/
  
  ;;; Code:
  (require 'ob)
  
! (declare-function slime-eval "ext:slime" (sexp &optional package))
  
  (defvar org-babel-tangle-lang-exts)
  (add-to-list 'org-babel-tangle-lang-exts '("lisp" . "lisp"))
***************
*** 65,118 ****
  (defun org-babel-expand-body:lisp (body params)
    "Expand BODY according to PARAMS, return the expanded body."
    (let* ((vars (mapcar #'cdr (org-babel-get-header params :var)))
!          (result-params (cdr (assoc :result-params params)))
!          (print-level nil) (print-length nil)
!          (body (org-babel-trim
!                 (if (> (length vars) 0)
!                     (concat "(let ("
!                             (mapconcat
!                              (lambda (var)
!                                (format "(%S (quote %S))" (car var) (cdr var)))
!                              vars "\n      ")
!                             ")\n" body ")")
!                   body))))
      (if (or (member "code" result-params)
!             (member "pp" result-params))
!         (format "(pprint %s)" body)
        body)))
  
- ;;;###autoload
  (defun org-babel-execute:lisp (body params)
!   "Execute a block `BODY' with `PARAMS' of Common Lisp code with Babel."
!   (pcase org-babel-lisp-eval-fn
!     ("slime-eval" (require 'slime))
!     ("sly-eval" (require 'sly)))
    (org-babel-reassemble-table
     (let ((result
!           (funcall (if (member "output" (cdr (assoc :result-params params)))
!                        #'car #'cadr)
!                    (with-temp-buffer
!                      (insert (org-babel-expand-body:lisp body params))
!                      (funcall org-babel-lisp-eval-fn
!                               `(swank:eval-and-grab-output
!                                 ,(let ((dir (if (assoc :dir params)
!                                                 (cdr (assoc :dir params))
!                                               default-directory)))
!                                    (format
!                                     (if dir (format org-babel-lisp-dir-fmt dir)
!                                       "(progn %s\n)")
!                                     (buffer-substring-no-properties
!                                      (point-min) (point-max)))))
!                               (cdr (assoc :package params)))))))
       (org-babel-result-cond (cdr (assoc :result-params params))
         result
         (condition-case nil
             (read (org-babel-lisp-vector-to-list result))
           (error result))))
     (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-lisp-vector-to-list (results)
    ;; TODO: better would be to replace #(...) with [...]
--- 55,104 ----
  (defun org-babel-expand-body:lisp (body params)
    "Expand BODY according to PARAMS, return the expanded body."
    (let* ((vars (mapcar #'cdr (org-babel-get-header params :var)))
! 	 (result-params (cdr (assoc :result-params params)))
! 	 (print-level nil) (print-length nil)
! 	 (body (org-babel-trim
! 		(if (> (length vars) 0)
! 		    (concat "(let ("
! 			    (mapconcat
! 			     (lambda (var)
! 			       (format "(%S (quote %S))" (car var) (cdr var)))
! 			     vars "\n      ")
! 			    ")\n" body ")")
! 		  body))))
      (if (or (member "code" result-params)
! 	    (member "pp" result-params))
! 	(format "(pprint %s)" body)
        body)))
  
  (defun org-babel-execute:lisp (body params)
!   "Execute a block of Common Lisp code with Babel."
!   (require 'slime)
    (org-babel-reassemble-table
     (let ((result
! 	  (funcall (if (member "output" (cdr (assoc :result-params params)))
! 		       #'car #'cadr)
! 		   (with-temp-buffer
! 		     (insert (org-babel-expand-body:lisp body params))
! 		     (slime-eval `(swank:eval-and-grab-output
! 				   ,(let ((dir (if (assoc :dir params)
! 						   (cdr (assoc :dir params))
! 						 default-directory)))
! 				      (format
! 				       (if dir (format org-babel-lisp-dir-fmt dir)
! 					 "(progn %s\n)")
! 				       (buffer-substring-no-properties
! 					(point-min) (point-max)))))
! 				 (cdr (assoc :package params)))))))
       (org-babel-result-cond (cdr (assoc :result-params params))
         result
         (condition-case nil
             (read (org-babel-lisp-vector-to-list result))
           (error result))))
     (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-lisp-vector-to-list (results)
    ;; TODO: better would be to replace #(...) with [...]