ob-lilypond: Code cleanup

* lisp/ob-lilypond.el (org-babel-lilypond-OSX-ly-path,
  org-babel-lilypond-OSX-pdf-path, org-babel-lilypond-OSX-midi-path,
  org-babel-lilypond-nix-ly-path, org-babel-lilypond-nix-pdf-path,
  org-babel-lilypond-nix-midi-path, org-babel-lilypond-w32-ly-path,
  org-babel-lilypond-w32-pdf-path, org-babel-lilypond-w32-midi-path,
  org-babel-lilypond-determine-ly-path,
  org-babel-lilypond-determine-pdf-path,
  org-babel-lilypond-determine-midi-path): Remove.
  (org-babel-lilypond-ly-command, org-babel-lilypond-midi-command,
  org-babel-lilypond-pdf-command): Replacement for removed variables
  and functions.  Adapt all calls to the removed functions to use
  these variables instead.
  (org-babel-lilypond-commands): New defcustom for setting up
  the *-command variables.  Keep different defaults for different
  systems as the original code did to avoid tripping up unsuspecting
  users.
  (org-babel-lilypond-execute-tangled-ly,
  org-babel-lilypond-check-for-compile-error): Revert conditions to
  avoid superfluous forms.  Remove unused return values.
* testing/lisp/test-ob-lilypond.el: Do test for new variables and
  replace removed function calls with the appropriate variable
  content.  Exercise the new defcustom thoroughly.

lisp/ob-lilypond.el

@@ -62,18 +62,38 @@ org-babel-lilypond-play-midi-post-tangle determines whether to automate the
 playing of the resultant midi file.  If the value is nil,
 the midi file is not automatically played.  Default value is t")
 
-(defvar org-babel-lilypond-OSX-ly-path
-  "/Applications/lilypond.app/Contents/Resources/bin/lilypond")
-(defvar org-babel-lilypond-OSX-pdf-path "open")
-(defvar org-babel-lilypond-OSX-midi-path "open")
-
-(defvar org-babel-lilypond-nix-ly-path "/usr/bin/lilypond")
-(defvar org-babel-lilypond-nix-pdf-path "evince")
-(defvar org-babel-lilypond-nix-midi-path "timidity")
-
-(defvar org-babel-lilypond-w32-ly-path "lilypond")
-(defvar org-babel-lilypond-w32-pdf-path "")
-(defvar org-babel-lilypond-w32-midi-path "")
+(defconst org-babel-lilypond-ly-command
+  "Command to execute lilypond on your system.")
+(defconst org-babel-lilypond-pdf-command
+  "Command to show a PDF file on your system.")
+(defconst org-babel-lilypond-midi-command
+  "Command to play a MIDI file on your system.")
+(defcustom org-babel-lilypond-commands
+  (cond
+   ((eq system-type 'darwin)
+    '("/Applications/lilypond.app/Contents/Resources/bin/lilypond" "open" "open"))
+   ((eq system-type 'windows-nt)
+    '("lilypond" "" ""))
+   (t
+    '("lilypond" "xdg-open" "xdg-open")))
+  "Commands to run lilypond and view or play the results.
+These should be executables that take a filename as an argument.
+On some system it is possible to specify the filename directly
+and the viewer or player will be determined from the file type;
+you can leave the string empty on this case."
+  :group 'org-babel
+  :type '(list
+	  (string :tag "Lilypond   ")
+	  (string :tag "PDF Viewer ")
+	  (string :tag "MIDI Player"))
+  :version "24.3"
+  :package-version '(Org . "8.2.7")
+  :set
+  (lambda (symbol value)
+    (setq
+     org-babel-lilypond-ly-command   (nth 0 value)
+     org-babel-lilypond-pdf-command  (nth 1 value)
+     org-babel-lilypond-midi-command (nth 2 value))))
 
 (defvar org-babel-lilypond-gen-png nil
   "Image generation (png) can be turned on by default by setting
@@ -150,7 +170,7 @@ specific arguments to =org-babel-tangle="
       (insert (org-babel-expand-body:generic body params)))
     (org-babel-eval
      (concat
-      (org-babel-lilypond-determine-ly-path)
+      org-babel-lilypond-ly-command
       " -dbackend=eps "
       "-dno-gs-load-fonts "
       "-dinclude-eps-fonts "
@@ -177,29 +197,27 @@ If error in compilation, attempt to mark the error in lilypond org file"
                             (buffer-file-name) ".lilypond"))
           (org-babel-lilypond-temp-file (org-babel-lilypond-switch-extension
                          (buffer-file-name) ".ly")))
-      (if (file-exists-p org-babel-lilypond-tangled-file)
-          (progn
-            (when (file-exists-p org-babel-lilypond-temp-file)
-              (delete-file org-babel-lilypond-temp-file))
-            (rename-file org-babel-lilypond-tangled-file
-                         org-babel-lilypond-temp-file))
-        (error "Error: Tangle Failed!") t)
+      (if (not (file-exists-p org-babel-lilypond-tangled-file))
+	  (error "Error: Tangle Failed!")
+	(when (file-exists-p org-babel-lilypond-temp-file)
+	  (delete-file org-babel-lilypond-temp-file))
+	(rename-file org-babel-lilypond-tangled-file
+		     org-babel-lilypond-temp-file))
       (switch-to-buffer-other-window "*lilypond*")
       (erase-buffer)
       (org-babel-lilypond-compile-lilyfile org-babel-lilypond-temp-file)
       (goto-char (point-min))
-      (if (not (org-babel-lilypond-check-for-compile-error org-babel-lilypond-temp-file))
-          (progn
-            (other-window -1)
-            (org-babel-lilypond-attempt-to-open-pdf org-babel-lilypond-temp-file)
-            (org-babel-lilypond-attempt-to-play-midi org-babel-lilypond-temp-file))
-        (error "Error in Compilation!")))) nil)
+      (if (org-babel-lilypond-check-for-compile-error org-babel-lilypond-temp-file)
+	  (error "Error in Compilation!")
+	(other-window -1)
+	(org-babel-lilypond-attempt-to-open-pdf org-babel-lilypond-temp-file)
+	(org-babel-lilypond-attempt-to-play-midi org-babel-lilypond-temp-file)))))
 
 (defun org-babel-lilypond-compile-lilyfile (file-name &optional test)
   "Compile lilypond file and check for compile errors
 FILE-NAME is full path to lilypond (.ly) file"
   (message "Compiling LilyPond...")
-  (let ((arg-1 (org-babel-lilypond-determine-ly-path)) ;program
+  (let ((arg-1 org-babel-lilypond-ly-command) ;program
         (arg-2 nil)                    ;infile
         (arg-3 "*lilypond*")           ;buffer
 	(arg-4 t)                      ;display
@@ -225,11 +243,10 @@ FILE-NAME is full path to lilypond file.
 If TEST is t just return nil if no error found, and pass
 nil as file-name since it is unused in this context"
   (let ((is-error (search-forward "error:" nil t)))
-    (if (not test)
-        (if (not is-error)
-            nil
-          (org-babel-lilypond-process-compile-error file-name))
-      is-error)))
+    (if test
+	is-error
+      (when is-error
+	(org-babel-lilypond-process-compile-error file-name)))))
 
 (defun org-babel-lilypond-process-compile-error (file-name)
   "Process the compilation error that has occurred.
@@ -300,13 +317,13 @@ If TEST is non-nil, the shell command is returned and is not run"
     (let ((pdf-file (org-babel-lilypond-switch-extension file-name ".pdf")))
       (if (file-exists-p pdf-file)
           (let ((cmd-string
-                 (concat (org-babel-lilypond-determine-pdf-path) " " pdf-file)))
+                 (concat org-babel-lilypond-pdf-command " " pdf-file)))
             (if test
                 cmd-string
 	      (start-process
 	       "\"Audition pdf\""
 	       "*lilypond*"
-	       (org-babel-lilypond-determine-pdf-path)
+	       org-babel-lilypond-pdf-command
 	       pdf-file)))
 	(message  "No pdf file generated so can't display!")))))
 
@@ -318,49 +335,16 @@ If TEST is non-nil, the shell command is returned and is not run"
     (let ((midi-file (org-babel-lilypond-switch-extension file-name ".midi")))
       (if (file-exists-p midi-file)
           (let ((cmd-string
-                 (concat (org-babel-lilypond-determine-midi-path) " " midi-file)))
+                 (concat org-babel-lilypond-midi-command " " midi-file)))
             (if test
                 cmd-string
               (start-process
                "\"Audition midi\""
                "*lilypond*"
-               (org-babel-lilypond-determine-midi-path)
+               org-babel-lilypond-midi-command
                midi-file)))
         (message "No midi file generated so can't play!")))))
 
-(defun org-babel-lilypond-determine-ly-path (&optional test)
-  "Return correct path to ly binary depending on OS
-If TEST is non-nil, it contains a simulation of the OS for test purposes"
-  (let ((sys-type
-         (or test system-type)))
-    (cond ((string= sys-type  "darwin")
-           org-babel-lilypond-OSX-ly-path)
-          ((string= sys-type "windows-nt")
-           org-babel-lilypond-w32-ly-path)
-          (t org-babel-lilypond-nix-ly-path))))
-
-(defun org-babel-lilypond-determine-pdf-path (&optional test)
-  "Return correct path to pdf viewer depending on OS
-If TEST is non-nil, it contains a simulation of the OS for test purposes"
-  (let ((sys-type
-         (or test system-type)))
-    (cond ((string= sys-type  "darwin")
-           org-babel-lilypond-OSX-pdf-path)
-          ((string= sys-type "windows-nt")
-           org-babel-lilypond-w32-pdf-path)
-          (t org-babel-lilypond-nix-pdf-path))))
-
-(defun org-babel-lilypond-determine-midi-path (&optional test)
-  "Return correct path to midi player depending on OS
-If TEST is non-nil, it contains a simulation of the OS for test purposes"
-  (let ((sys-type
-         (or test test system-type)))
-    (cond ((string= sys-type  "darwin")
-           org-babel-lilypond-OSX-midi-path)
-          ((string= sys-type "windows-nt")
-           org-babel-lilypond-w32-midi-path)
-          (t org-babel-lilypond-nix-midi-path))))
-
 (defun org-babel-lilypond-toggle-midi-play ()
   "Toggle whether midi will be played following a successful compilation."
   (interactive)

testing/lisp/test-ob-lilypond.el

@@ -52,7 +52,7 @@
 
 (ert-deftest ob-lilypond/ly-compile-lilyfile ()
   (should (equal
-           `(,(org-babel-lilypond-determine-ly-path)    ;program
+           `(,org-babel-lilypond-ly-command    ;program
              nil                        ;infile
              "*lilypond*"               ;buffer
              t                          ;display
@@ -74,41 +74,84 @@
 (ert-deftest ob-lilypond/ly-play-midi-post-tangle ()
   (should (boundp 'org-babel-lilypond-play-midi-post-tangle)))
 
-(ert-deftest ob-lilypond/ly-OSX-ly-path ()
-  (should (boundp 'org-babel-lilypond-OSX-ly-path))
-  (should (stringp org-babel-lilypond-OSX-ly-path)))
-
-(ert-deftest ob-lilypond/ly-OSX-pdf-path ()
-  (should (boundp 'org-babel-lilypond-OSX-pdf-path))
-  (should (stringp org-babel-lilypond-OSX-pdf-path)))
-
-(ert-deftest ob-lilypond/ly-OSX-midi-path ()
-  (should (boundp 'org-babel-lilypond-OSX-midi-path))
-  (should (stringp org-babel-lilypond-OSX-midi-path)))
-
-(ert-deftest ob-lilypond/ly-nix-ly-path ()
-  (should (boundp 'org-babel-lilypond-nix-ly-path))
-  (should (stringp org-babel-lilypond-nix-ly-path)))
-
-(ert-deftest ob-lilypond/ly-nix-pdf-path ()
-  (should (boundp 'org-babel-lilypond-nix-pdf-path))
-  (should (stringp org-babel-lilypond-nix-pdf-path)))
-
-(ert-deftest ob-lilypond/ly-nix-midi-path ()
-  (should (boundp 'org-babel-lilypond-nix-midi-path))
-  (should (stringp org-babel-lilypond-nix-midi-path)))
-
-(ert-deftest ob-lilypond/ly-w32-ly-path ()
-  (should (boundp 'org-babel-lilypond-w32-ly-path))
-  (should (stringp org-babel-lilypond-w32-ly-path)))
-
-(ert-deftest ob-lilypond/ly-w32-pdf-path ()
-  (should (boundp 'org-babel-lilypond-w32-pdf-path))
-  (should (stringp org-babel-lilypond-w32-pdf-path)))
-
-(ert-deftest ob-lilypond/ly-w32-midi-path ()
-  (should (boundp 'org-babel-lilypond-w32-midi-path))
-  (should (stringp org-babel-lilypond-w32-midi-path)))
+(ert-deftest ob-lilypond/ly-command-ly/bound ()
+  (should (boundp 'org-babel-lilypond-ly-command)))
+(ert-deftest ob-lilypond/ly-command-ly/stringp ()
+  (should (stringp org-babel-lilypond-ly-command)))
+(ert-deftest ob-lilypond/ly-command-pdf/bound ()
+  (should (boundp 'org-babel-lilypond-pdf-command)))
+(ert-deftest ob-lilypond/ly-command-pdf/stringp ()
+  (should (stringp org-babel-lilypond-pdf-command)))
+(ert-deftest ob-lilypond/ly-command-midi/bound ()
+  (should (boundp 'org-babel-lilypond-midi-command)))
+(ert-deftest ob-lilypond/ly-command-midi/stringp ()
+  (should (stringp org-babel-lilypond-midi-command)))
+(ert-deftest ob-lilypond/ly-commands/darwin ()
+  (let ((system-type 'darwin)
+	org-babel-lilypond-ly-command
+	org-babel-lilypond-pdf-command
+	org-babel-lilypond-midi-command)
+    (custom-reevaluate-setting 'org-babel-lilypond-commands)
+    (should (equal
+	     (list
+	      org-babel-lilypond-ly-command
+	      org-babel-lilypond-pdf-command
+	      org-babel-lilypond-midi-command)
+	     (list
+	      "/Applications/lilypond.app/Contents/Resources/bin/lilypond"
+	      "open"
+	      "open"))))
+  (custom-reevaluate-setting 'org-babel-lilypond-commands))
+(ert-deftest ob-lilypond/ly-commands/windows-nt ()
+  (let ((system-type 'windows-nt)
+	org-babel-lilypond-ly-command
+	org-babel-lilypond-pdf-command
+	org-babel-lilypond-midi-command)
+    (custom-reevaluate-setting 'org-babel-lilypond-commands)
+    (should (equal
+	     (list
+	      org-babel-lilypond-ly-command
+	      org-babel-lilypond-pdf-command
+	      org-babel-lilypond-midi-command)
+	     (list
+	      "lilypond"
+	      ""
+	      ""))))
+  (custom-reevaluate-setting 'org-babel-lilypond-commands))
+(ert-deftest ob-lilypond/ly-commands/other ()
+  (let ((system-type 'other)
+	org-babel-lilypond-ly-command
+	org-babel-lilypond-pdf-command
+	org-babel-lilypond-midi-command)
+    (custom-reevaluate-setting 'org-babel-lilypond-commands)
+    (should (equal
+	     (list
+	      org-babel-lilypond-ly-command
+	      org-babel-lilypond-pdf-command
+	      org-babel-lilypond-midi-command)
+	     (list
+	      "lilypond"
+	      "xdg-open"
+	      "xdg-open"))))
+  (custom-reevaluate-setting 'org-babel-lilypond-commands))
+
+(ert-deftest ob-lilypond/ly-commands/customize ()
+  (let ((system-type 'other)
+	org-babel-lilypond-ly-command
+	org-babel-lilypond-pdf-command
+	org-babel-lilypond-midi-command)
+    (custom-initialize-reset 'org-babel-lilypond-commands
+			     '(list "nonsense" "bla" "fasel"))
+    (should (equal
+	     (list
+	      org-babel-lilypond-ly-command
+	      org-babel-lilypond-pdf-command
+	      org-babel-lilypond-midi-command)
+	     (list
+	      "nonsense"
+	      "bla"
+	      "fasel"))))
+  (custom-reevaluate-setting 'org-babel-lilypond-commands))
 
 (ert-deftest ob-lilypond/ly-gen-png ()
   (should (boundp 'org-babel-lilypond-gen-png)))
@@ -222,7 +265,7 @@
       (write-file pdf-file))
     (should (equal
              (concat
-              (org-babel-lilypond-determine-pdf-path) " " pdf-file)
+              org-babel-lilypond-pdf-command " " pdf-file)
              (org-babel-lilypond-attempt-to-open-pdf org-babel-lilypond-file t)))
     (delete-file pdf-file)
     (kill-buffer (file-name-nondirectory pdf-file))
@@ -245,7 +288,7 @@
       (write-file midi-file))
     (should (equal
              (concat
-              (org-babel-lilypond-determine-midi-path) " " midi-file)
+              org-babel-lilypond-midi-command " " midi-file)
              (org-babel-lilypond-attempt-to-play-midi org-babel-lilypond-file t)))
     (delete-file midi-file)
     (kill-buffer (file-name-nondirectory midi-file))
@@ -254,30 +297,6 @@
              (org-babel-lilypond-attempt-to-play-midi midi-file)))
     (setq org-babel-lilypond-play-midi-post-tangle post-tangle)))
 
-(ert-deftest ob-lilypond/ly-determine-ly-path ()
-  (should (equal org-babel-lilypond-OSX-ly-path
-                 (org-babel-lilypond-determine-ly-path "darwin")))
-  (should (equal org-babel-lilypond-w32-ly-path
-                 (org-babel-lilypond-determine-ly-path "windows-nt")))
-  (should (equal org-babel-lilypond-nix-ly-path
-                 (org-babel-lilypond-determine-ly-path "nix"))))
-
-(ert-deftest ob-lilypond/ly-determine-pdf-path ()
-  (should (equal org-babel-lilypond-OSX-pdf-path
-                 (org-babel-lilypond-determine-pdf-path "darwin")))
-  (should (equal org-babel-lilypond-w32-pdf-path
-                 (org-babel-lilypond-determine-pdf-path "windows-nt")))
-  (should (equal org-babel-lilypond-nix-pdf-path
-                 (org-babel-lilypond-determine-pdf-path "nix"))))
-
-(ert-deftest ob-lilypond/ly-determine-midi-path ()
-  (should (equal org-babel-lilypond-OSX-midi-path
-                 (org-babel-lilypond-determine-midi-path "darwin")))
-  (should (equal org-babel-lilypond-w32-midi-path
-                 (org-babel-lilypond-determine-midi-path "windows-nt")))
-  (should (equal org-babel-lilypond-nix-midi-path
-                 (org-babel-lilypond-determine-midi-path "nix"))))
-
 (ert-deftest ob-lilypond/ly-toggle-midi-play-toggles-flag ()
   (if org-babel-lilypond-play-midi-post-tangle
       (progn