From 64821bd967828ed33463987a60d947157c86a30c Mon Sep 17 00:00:00 2001 From: Achim Gratz Date: Sun, 17 Aug 2014 10:29:24 +0200 Subject: [PATCH] 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 | 122 +++++++++++++++------------------ testing/lisp/test-ob-lilypond.el | 143 ++++++++++++++++++++++----------------- 2 files changed, 134 insertions(+), 131 deletions(-) diff --git a/lisp/ob-lilypond.el b/lisp/ob-lilypond.el index b24b8e110..43433a357 100644 --- a/lisp/ob-lilypond.el +++ b/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) diff --git a/testing/lisp/test-ob-lilypond.el b/testing/lisp/test-ob-lilypond.el index c0cd475f3..355f91f78 100644 --- a/testing/lisp/test-ob-lilypond.el +++ b/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 -- 2.11.4.GIT