Re: [O] ob-lisp.el patch to choose to use SLIME or SLY

[Nicolas Goaziou]

Hello,

"address@hidden" <address@hidden> writes:

> I original created an package ob-lisp on here:
> https://github.com/stardiviner/ob-lisp
> and add it to MELPA recipe.
> https://github.com/melpa/melpa/pull/3682
>
> The author suggest me to merge this change to Org-mode.
>
> I attached patch in attachment file.
> Hope someone can merge this patch.

Thank you. I have some comments about it.

> [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/
>
> diff --git a/ob-lisp.el b/ob-lisp.el
> index d2cac3d..04df7fb 100644
> --- a/ob-lisp.el
> +++ b/ob-lisp.el
> @@ -1,14 +1,12 @@
> -;;; ob-lisp.el --- org-babel functions for common lisp evaluation with SLY 
> or SLIME.
> +;;; ob-lisp.el --- org-babel functions for common lisp evaluation

You are not using an up-to-date version of the file, since this header
changed in development version. Could you rebase your patch on top of
latest development release?
>  
> -;; Copyright (C) 2016-2020 Free Software Foundation, Inc.
> +;; Copyright (C) 2009-2016 Free Software Foundation, Inc.

Ditto.

> -;; Authors: stardiviner <address@hidden>
> -;; Maintainer: stardiviner <address@hidden>
> -;; 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 <address@hidden>
> +;; Keywords: literate programming, reproducible research
> +;; Homepage: http://orgmode.org

Ditto. These changes should not appear in the final patch.
>  
>  ;; This file is part of GNU Emacs.
>  
> @@ -27,27 +25,17 @@
>  
>  ;;; Commentary:
>  
> -;;; Support for evaluating Common Lisp code, relies on SLY or SLIME for all 
> eval.
> +;;; support for evaluating common lisp code, relies on slime for all eval

s/slime/SLIME/

>  
>  ;;; Requirements:
>  
> -;; 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/
>  
>  ;;; 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)

Could you mention this removal in etc/ORG-NEWS? This is a user-visible
change.

Also, could you provide an appropriate commit message, e.g.:

--8<---------------cut here---------------start------------->8---
  ob-lisp: Drop SLY in favor of SLIME

  * lisp/ob-lisp.el (org-babel-expand-body:lisp,
    org-babel-execute:lisp): Use SLIME instead of SLY.
  (org-babel-lisp-eval-fn): Remove variable.
--8<---------------cut here---------------end--------------->8---

Feel free to add justifications about this change at the end of the
commit message.

Regards,

-- 
Nicolas Goaziou