* Org mode version 9.7-pre (9.7-pre-n/a-g63e8ca @ /home/n/.emacs.d/elpaca/builds/org/); [PATCH] refactor org-babel-lilypond-compile-lilyfile @ 2023-08-24 13:48 No Wayman 2023-08-25 8:06 ` Ihor Radchenko 0 siblings, 1 reply; 6+ messages in thread From: No Wayman @ 2023-08-24 13:48 UTC (permalink / raw) To: emacs-orgmode [-- Attachment #1: Type: text/plain, Size: 307 bytes --] The attached patch: - Improves the compilation message. (The target is the file, not LilyPond itself). - Refactors the body of the function to remove the many nondescript arg-n local variables. - Removes the optional TEST parameter, which is unused and better served through debugging tools. [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0001-lisp-ob-lilypond.el-refactor-org-babel-lilypond-comp.patch --] [-- Type: text/x-patch, Size: 2772 bytes --] From 5765891b9de22260454d2cee8365c14841d0d744 Mon Sep 17 00:00:00 2001 From: Nicholas Vollmer <iarchivedmywholelife@gmail.com> Date: Thu, 24 Aug 2023 09:32:27 -0400 Subject: [PATCH] lisp/ob-lilypond.el: refactor org-babel-lilypond-compile-lilyfile * ob-lilypond.el (org-babel-lilypond-compile-lilyfile): Correct compilation message. Remove superfluous local variables. Remove unused ad-hoc debugging TEST parameter. --- lisp/ob-lilypond.el | 32 +++++++++++++------------------- 1 file changed, 13 insertions(+), 19 deletions(-) diff --git a/lisp/ob-lilypond.el b/lisp/ob-lilypond.el index 8b2bd231f..92cd59685 100644 --- a/lisp/ob-lilypond.el +++ b/lisp/ob-lilypond.el @@ -247,27 +247,21 @@ org-babel-lilypond-execute-tangled-ly (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) +(defun org-babel-lilypond-compile-lilyfile (file-name) "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-ly-command) ;program - ;; (arg-2 nil) ;infile - (arg-3 "*lilypond*") ;buffer - (arg-4 t) ;display - (arg-5 (if org-babel-lilypond-gen-png "--png" "")) ;&rest... - (arg-6 (if org-babel-lilypond-gen-html "--html" "")) - (arg-7 (if org-babel-lilypond-gen-pdf "--pdf" "")) - (arg-8 (if org-babel-lilypond-use-eps "-dbackend=eps" "")) - (arg-9 (if org-babel-lilypond-gen-svg "-dbackend=svg" "")) - (arg-10 (concat "--output=" (file-name-sans-extension file-name))) - (arg-11 file-name)) - (if test - `(,arg-1 ,nil ,arg-3 ,arg-4 ,arg-5 ,arg-6 ;; arg-2 - ,arg-7 ,arg-8 ,arg-9 ,arg-10 ,arg-11) - (call-process - arg-1 nil arg-3 arg-4 arg-5 arg-6 ;; arg-2 - arg-7 arg-8 arg-9 arg-10 arg-11)))) + (message "Compiling %s..." file-name) + (let ((args + (delq nil (list + (and org-babel-lilypond-gen-png "--png") + (and org-babel-lilypond-gen-html "--html") + (and org-babel-lilypond-gen-pdf "--pdf") + (and org-babel-lilypond-use-eps "-dbackend=eps") + (and org-babel-lilypond-gen-svg "-dbackend=svg") + (concat "--output=" (file-name-sans-extension file-name)) + file-name)))) + (apply #'call-process org-babel-lilypond-ly-command nil + "*lilypond*" 'display args))) (defun org-babel-lilypond-check-for-compile-error (file-name &optional test) "Check for compile error. -- 2.41.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: Org mode version 9.7-pre (9.7-pre-n/a-g63e8ca @ /home/n/.emacs.d/elpaca/builds/org/); [PATCH] refactor org-babel-lilypond-compile-lilyfile 2023-08-24 13:48 Org mode version 9.7-pre (9.7-pre-n/a-g63e8ca @ /home/n/.emacs.d/elpaca/builds/org/); [PATCH] refactor org-babel-lilypond-compile-lilyfile No Wayman @ 2023-08-25 8:06 ` Ihor Radchenko 2023-09-03 19:35 ` No Wayman 0 siblings, 1 reply; 6+ messages in thread From: Ihor Radchenko @ 2023-08-25 8:06 UTC (permalink / raw) To: No Wayman; +Cc: emacs-orgmode No Wayman <iarchivedmywholelife@gmail.com> writes: > The attached patch: > > - Improves the compilation message. (The target is the file, not > LilyPond itself). > - Refactors the body of the function to remove the many > nondescript arg-n local variables. Thanks! LGTM. > - Removes the optional TEST parameter, which is unused and better > served through debugging tools. This is technically a breaking change. So, we (1) need to mention it in ORG-NEWS; (2) may consider (file-name &optional _) function definition to be 100% sure that no existing code will be broken. -- Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at <https://orgmode.org/>. Support Org development at <https://liberapay.com/org-mode>, or support my work at <https://liberapay.com/yantar92> ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Org mode version 9.7-pre (9.7-pre-n/a-g63e8ca @ /home/n/.emacs.d/elpaca/builds/org/); [PATCH] refactor org-babel-lilypond-compile-lilyfile 2023-08-25 8:06 ` Ihor Radchenko @ 2023-09-03 19:35 ` No Wayman 2023-09-04 7:47 ` Ihor Radchenko 0 siblings, 1 reply; 6+ messages in thread From: No Wayman @ 2023-09-03 19:35 UTC (permalink / raw) To: Ihor Radchenko; +Cc: emacs-orgmode [-- Attachment #1: Type: text/plain, Size: 483 bytes --] Ihor Radchenko <yantar92@posteo.net> writes: >> - Removes the optional TEST parameter, which is unused and >> better >> served through debugging tools. > > This is technically a breaking change. So, we (1) need to > mention it in > ORG-NEWS; (2) may consider (file-name &optional _) function > definition > to be 100% sure that no existing code will be broken. I've added the NEWS entry and updated the signature to ignore the second, optional parameter. See attached. [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: revised-patch --] [-- Type: text/x-patch, Size: 3446 bytes --] From ace59bb72e8fdd6f01033d327de221cfdd9090fe Mon Sep 17 00:00:00 2001 From: Nicholas Vollmer <iarchivedmywholelife@gmail.com> Date: Thu, 24 Aug 2023 09:32:27 -0400 Subject: [PATCH] lisp/ob-lilypond.el: refactor org-babel-lilypond-compile-lilyfile * ob-lilypond.el (org-babel-lilypond-compile-lilyfile): Correct compilation message. Remove superfluous local variables. Remove unused ad-hoc debugging TEST parameter. --- etc/ORG-NEWS | 3 +++ lisp/ob-lilypond.el | 35 ++++++++++++++--------------------- 2 files changed, 17 insertions(+), 21 deletions(-) diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS index 842d3fe4d..c40e1be82 100644 --- a/etc/ORG-NEWS +++ b/etc/ORG-NEWS @@ -304,6 +304,9 @@ setting the ~STYLE~ property for each sub-task. The change is breaking when ~org-use-property-inheritance~ is set to ~t~. +*** ~org-babel-lilypond-compile-lilyfile~ ignores optional second argument + +The =TEST= parameter is better served by Emacs debugging tools. ** New and changed options *** ~org-export-smart-quotes-alist~ is now a custom option diff --git a/lisp/ob-lilypond.el b/lisp/ob-lilypond.el index 8b2bd231f..c2dba36aa 100644 --- a/lisp/ob-lilypond.el +++ b/lisp/ob-lilypond.el @@ -247,27 +247,20 @@ org-babel-lilypond-execute-tangled-ly (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-ly-command) ;program - ;; (arg-2 nil) ;infile - (arg-3 "*lilypond*") ;buffer - (arg-4 t) ;display - (arg-5 (if org-babel-lilypond-gen-png "--png" "")) ;&rest... - (arg-6 (if org-babel-lilypond-gen-html "--html" "")) - (arg-7 (if org-babel-lilypond-gen-pdf "--pdf" "")) - (arg-8 (if org-babel-lilypond-use-eps "-dbackend=eps" "")) - (arg-9 (if org-babel-lilypond-gen-svg "-dbackend=svg" "")) - (arg-10 (concat "--output=" (file-name-sans-extension file-name))) - (arg-11 file-name)) - (if test - `(,arg-1 ,nil ,arg-3 ,arg-4 ,arg-5 ,arg-6 ;; arg-2 - ,arg-7 ,arg-8 ,arg-9 ,arg-10 ,arg-11) - (call-process - arg-1 nil arg-3 arg-4 arg-5 arg-6 ;; arg-2 - arg-7 arg-8 arg-9 arg-10 arg-11)))) +;;Ignoring second arg for pre Org 9.7 compatibility +(defun org-babel-lilypond-compile-lilyfile (filename &optional _) + "Compile Lilypond FILENAME and check for compile errors." + (message "Compiling %s..." filename) + (let ((args (delq nil (list + (and org-babel-lilypond-gen-png "--png") + (and org-babel-lilypond-gen-html "--html") + (and org-babel-lilypond-gen-pdf "--pdf") + (and org-babel-lilypond-use-eps "-dbackend=eps") + (and org-babel-lilypond-gen-svg "-dbackend=svg") + (concat "--output=" (file-name-sans-extension filename)) + filename)))) + (apply #'call-process org-babel-lilypond-ly-command nil + "*lilypond*" 'display args))) (defun org-babel-lilypond-check-for-compile-error (file-name &optional test) "Check for compile error. -- 2.42.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: Org mode version 9.7-pre (9.7-pre-n/a-g63e8ca @ /home/n/.emacs.d/elpaca/builds/org/); [PATCH] refactor org-babel-lilypond-compile-lilyfile 2023-09-03 19:35 ` No Wayman @ 2023-09-04 7:47 ` Ihor Radchenko 2023-09-06 1:11 ` No Wayman 0 siblings, 1 reply; 6+ messages in thread From: Ihor Radchenko @ 2023-09-04 7:47 UTC (permalink / raw) To: No Wayman; +Cc: emacs-orgmode No Wayman <iarchivedmywholelife@gmail.com> writes: > Ihor Radchenko <yantar92@posteo.net> writes: > >>> - Removes the optional TEST parameter, which is unused and >>> better >>> served through debugging tools. >> >> This is technically a breaking change. So, we (1) need to >> mention it in >> ORG-NEWS; (2) may consider (file-name &optional _) function >> definition >> to be 100% sure that no existing code will be broken. > > I've added the NEWS entry and updated the signature to ignore the > second, optional parameter. > See attached. Thanks! > +*** ~org-babel-lilypond-compile-lilyfile~ ignores optional second argument > + > +The =TEST= parameter is better served by Emacs debugging tools. It looks like ob-lilypond/ly-compile-lilyfile test is relying on this optional second argument. So, after applying your patch, make test is failing. -- Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at <https://orgmode.org/>. Support Org development at <https://liberapay.com/org-mode>, or support my work at <https://liberapay.com/yantar92> ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Org mode version 9.7-pre (9.7-pre-n/a-g63e8ca @ /home/n/.emacs.d/elpaca/builds/org/); [PATCH] refactor org-babel-lilypond-compile-lilyfile 2023-09-04 7:47 ` Ihor Radchenko @ 2023-09-06 1:11 ` No Wayman 2023-09-06 9:35 ` Ihor Radchenko 0 siblings, 1 reply; 6+ messages in thread From: No Wayman @ 2023-09-06 1:11 UTC (permalink / raw) To: Ihor Radchenko; +Cc: emacs-orgmode [-- Attachment #1: Type: text/plain, Size: 431 bytes --] Ihor Radchenko <yantar92@posteo.net> writes: > Thanks! You're welcome. > It looks like ob-lilypond/ly-compile-lilyfile test is relying on > this > optional second argument. So, after applying your patch, make > test is > failing. I've amended the test using cl-letf. I also bound the variables which may generate output files to nil to prevent generating any output files if the test is run interactively. See attached. [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0001-lisp-ob-lilypond.el-refactor-org-babel-lilypond-comp.patch --] [-- Type: text/x-patch, Size: 5750 bytes --] From 94a97e1581c28a5330ddaf6664e1bc00516be807 Mon Sep 17 00:00:00 2001 From: Nicholas Vollmer <iarchivedmywholelife@gmail.com> Date: Thu, 24 Aug 2023 09:32:27 -0400 Subject: [PATCH] lisp/ob-lilypond.el: refactor org-babel-lilypond-compile-lilyfile * ob-lilypond.el (org-babel-lilypond-compile-lilyfile): Correct compilation message. Remove superfluous local variables. Remove unused ad-hoc debugging TEST parameter. * test-ob-lilypond.el (ob-lilypond/ly-compile-lilyfile): Locally bind variables which may generate output files. Refactor to account for removed TEST parameter. --- etc/ORG-NEWS | 3 +++ lisp/ob-lilypond.el | 35 +++++++++++++------------------- testing/lisp/test-ob-lilypond.el | 31 ++++++++++++++++------------ 3 files changed, 35 insertions(+), 34 deletions(-) diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS index 842d3fe4d..c40e1be82 100644 --- a/etc/ORG-NEWS +++ b/etc/ORG-NEWS @@ -304,6 +304,9 @@ setting the ~STYLE~ property for each sub-task. The change is breaking when ~org-use-property-inheritance~ is set to ~t~. +*** ~org-babel-lilypond-compile-lilyfile~ ignores optional second argument + +The =TEST= parameter is better served by Emacs debugging tools. ** New and changed options *** ~org-export-smart-quotes-alist~ is now a custom option diff --git a/lisp/ob-lilypond.el b/lisp/ob-lilypond.el index 8b2bd231f..c2dba36aa 100644 --- a/lisp/ob-lilypond.el +++ b/lisp/ob-lilypond.el @@ -247,27 +247,20 @@ org-babel-lilypond-execute-tangled-ly (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-ly-command) ;program - ;; (arg-2 nil) ;infile - (arg-3 "*lilypond*") ;buffer - (arg-4 t) ;display - (arg-5 (if org-babel-lilypond-gen-png "--png" "")) ;&rest... - (arg-6 (if org-babel-lilypond-gen-html "--html" "")) - (arg-7 (if org-babel-lilypond-gen-pdf "--pdf" "")) - (arg-8 (if org-babel-lilypond-use-eps "-dbackend=eps" "")) - (arg-9 (if org-babel-lilypond-gen-svg "-dbackend=svg" "")) - (arg-10 (concat "--output=" (file-name-sans-extension file-name))) - (arg-11 file-name)) - (if test - `(,arg-1 ,nil ,arg-3 ,arg-4 ,arg-5 ,arg-6 ;; arg-2 - ,arg-7 ,arg-8 ,arg-9 ,arg-10 ,arg-11) - (call-process - arg-1 nil arg-3 arg-4 arg-5 arg-6 ;; arg-2 - arg-7 arg-8 arg-9 arg-10 arg-11)))) +;;Ignoring second arg for pre Org 9.7 compatibility +(defun org-babel-lilypond-compile-lilyfile (filename &optional _) + "Compile Lilypond FILENAME and check for compile errors." + (message "Compiling %s..." filename) + (let ((args (delq nil (list + (and org-babel-lilypond-gen-png "--png") + (and org-babel-lilypond-gen-html "--html") + (and org-babel-lilypond-gen-pdf "--pdf") + (and org-babel-lilypond-use-eps "-dbackend=eps") + (and org-babel-lilypond-gen-svg "-dbackend=svg") + (concat "--output=" (file-name-sans-extension filename)) + filename)))) + (apply #'call-process org-babel-lilypond-ly-command nil + "*lilypond*" 'display args))) (defun org-babel-lilypond-check-for-compile-error (file-name &optional test) "Check for compile error. diff --git a/testing/lisp/test-ob-lilypond.el b/testing/lisp/test-ob-lilypond.el index b829b8a10..5f439793b 100644 --- a/testing/lisp/test-ob-lilypond.el +++ b/testing/lisp/test-ob-lilypond.el @@ -46,19 +46,24 @@ ob-lilypond/org-babel-prep-session:lilypond :type 'error) (ert-deftest ob-lilypond/ly-compile-lilyfile () - (should (equal - `(,org-babel-lilypond-ly-command ;program - nil ;infile - "*lilypond*" ;buffer - t ;display - ,(if org-babel-lilypond-gen-png "--png" "") ;&rest... - ,(if org-babel-lilypond-gen-html "--html" "") - ,(if org-babel-lilypond-gen-pdf "--pdf" "") - ,(if org-babel-lilypond-use-eps "-dbackend=eps" "") - ,(if org-babel-lilypond-gen-svg "-dbackend=svg" "") - "--output=test-file" - "test-file.ly") - (org-babel-lilypond-compile-lilyfile "test-file.ly" t)))) + (cl-letf (((symbol-function 'call-process) 'list) + (org-babel-lilypond-gen-png nil) + (org-babel-lilypond-gen-html nil) + (org-babel-lilypond-use-eps nil) + (org-babel-lilypond-gen-svg nil)) + (should (equal + `(,org-babel-lilypond-ly-command ;program + nil ;infile + "*lilypond*" ;buffer + display + ,@(when org-babel-lilypond-gen-png '("--png")) ;&rest... + ,@(when org-babel-lilypond-gen-html '("--html")) + ,@(when org-babel-lilypond-gen-pdf '("--pdf")) + ,@(when org-babel-lilypond-use-eps '("-dbackend=eps")) + ,@(when org-babel-lilypond-gen-svg '("-dbackend=svg")) + "--output=test-file" + "test-file.ly") + (org-babel-lilypond-compile-lilyfile "test-file.ly"))))) (ert-deftest ob-lilypond/ly-compile-post-tangle () (should (boundp 'org-babel-lilypond-compile-post-tangle))) -- 2.42.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: Org mode version 9.7-pre (9.7-pre-n/a-g63e8ca @ /home/n/.emacs.d/elpaca/builds/org/); [PATCH] refactor org-babel-lilypond-compile-lilyfile 2023-09-06 1:11 ` No Wayman @ 2023-09-06 9:35 ` Ihor Radchenko 0 siblings, 0 replies; 6+ messages in thread From: Ihor Radchenko @ 2023-09-06 9:35 UTC (permalink / raw) To: No Wayman; +Cc: emacs-orgmode No Wayman <iarchivedmywholelife@gmail.com> writes: > I've amended the test using cl-letf. > I also bound the variables which may generate output files to nil > to prevent generating any output files if the test is run > interactively. > See attached. Applied, onto main. https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=4fe52fc8a -- Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at <https://orgmode.org/>. Support Org development at <https://liberapay.com/org-mode>, or support my work at <https://liberapay.com/yantar92> ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-09-06 9:36 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-08-24 13:48 Org mode version 9.7-pre (9.7-pre-n/a-g63e8ca @ /home/n/.emacs.d/elpaca/builds/org/); [PATCH] refactor org-babel-lilypond-compile-lilyfile No Wayman 2023-08-25 8:06 ` Ihor Radchenko 2023-09-03 19:35 ` No Wayman 2023-09-04 7:47 ` Ihor Radchenko 2023-09-06 1:11 ` No Wayman 2023-09-06 9:35 ` Ihor Radchenko
Code repositories for project(s) associated with this external index https://git.savannah.gnu.org/cgit/emacs.git https://git.savannah.gnu.org/cgit/emacs/org-mode.git This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.