Issue 317270043: Add lilypond version predicates/operators
|
Created:
9 years, 1 month ago by git Modified:
9 years ago CC:
lilypond-devel_gnu.org Visibility:
Public. |
DescriptionAdd lilypond version predicates/operators
This set of predicates/operators compares a given reference version
to the LilyPond version that is currently being executed.
This makes it possible to implement "version switches" to write
(library) code that is compatible over syntax changes.
NOTE: I'm not sure where (and if) this should be documented.
Please make suggestions
Patch Set 1 #
Total comments: 2
Patch Set 2 : Reflect discussions. #
Total comments: 4
Patch Set 3 : Lexicographic comparison and ly:version? wrapper #
Total comments: 5
Patch Set 4 : Rename secondary function after discussion #Patch Set 5 : Fix omission of previous patch set #Patch Set 6 : Limit ly:version? to number list (as suggested by Paul) #MessagesTotal messages: 26
https://codereview.appspot.com/317270043/diff/1/scm/lily-library.scm File scm/lily-library.scm (right): https://codereview.appspot.com/317270043/diff/1/scm/lily-library.scm#newcode894 scm/lily-library.scm:894: (define (calculate-version ref-version) This seems overly complicated. Why not just stick with a number list? Something like #(define (ly:version? op . rest) (let loop ((ver (ly:version)) (rest rest)) (if (or (null? ver) (null? rest)) (op 0 0) ;; return #t iff op includes equality (let* ((a (car ver)) (b (car rest)) (axb (op a b)) (bxa (op b a))) (if (eq? axb bxa) (loop (cdr ver) (cdr rest)) axb))))) #(format #t ">2.19 : ~S, >= 2.19 : ~S\n" (ly:version? > 2 19) (ly:version? >= 2 19))
Sign in to reply to this message.
https://codereview.appspot.com/317270043/diff/1/scm/lily-library.scm File scm/lily-library.scm (right): https://codereview.appspot.com/317270043/diff/1/scm/lily-library.scm#newcode909 scm/lily-library.scm:909: (define-public (lilypond>? ref-version) Instead of a separate procedure for each comparison, what about making it generic: (define-public (version-compare v1 proc v2) "Compare two versions @var{v1} and @var{v2} with the procedure @var{proc}. The versions are lists of three numbers like @code{'(2 18 0)}. The procedure would typically be <, >, <=, >=, =, etc." (proc (calculate-version v1) (calculate-version v2))) Usage looks like: (version-compare (ly:version) >= '(2 18 0)) Then it can be used to compare any two versions, not necessarily LilyPond (might also be useful for packages some day?) and the user learns ly:version. (Having done this, I see David's suggestion... haven't looked at it closely yet.)
Sign in to reply to this message.
I'm choking a little bit with having the operator as the first argument instead of actually being the first element. But apart from that this solution works very well. And as this is not intended to be end-user facing code it's of course OK to have just one single possible input format (number list and no version string).
Sign in to reply to this message.
On 2017/02/14 15:00:10, pwm wrote: > https://codereview.appspot.com/317270043/diff/1/scm/lily-library.scm > File scm/lily-library.scm (right): > > https://codereview.appspot.com/317270043/diff/1/scm/lily-library.scm#newcode909 > scm/lily-library.scm:909: (define-public (lilypond>? ref-version) > Instead of a separate procedure for each comparison, what about making it > generic: > > (define-public (version-compare v1 proc v2) > "Compare two versions @var{v1} and @var{v2} with the > procedure @var{proc}. The versions are lists of > three numbers like @code{'(2 18 0)}. The procedure > would typically be <, >, <=, >=, =, etc." > (proc (calculate-version v1) (calculate-version v2))) > > Usage looks like: > > (version-compare (ly:version) >= '(2 18 0)) > > Then it can be used to compare any two versions, not necessarily LilyPond (might > also be useful for packages some day?) and the user learns ly:version. > > (Having done this, I see David's suggestion... haven't looked at it closely > yet.) I like this more because it's more of a typical procedure invocation. I think I'll create a new patch with this and a simplified calculate-version (that doesn't accept string lists)
Sign in to reply to this message.
On 2017/02/14 15:15:07, git wrote: > I like this more because it's more of a typical procedure invocation. > I think I'll create a new patch with this and a simplified calculate-version > (that doesn't accept string lists) Well, having the procedure as the first argument might be more scheme-ish? Maybe best would be to have a "version-compare" function that compares two version lists using a procedure. Then define "ly:version?" so it calls "version-compare" with the current LilyPond version (to handle the primary use case). The "version-compare" function could be David's looping function, but modified so that it took two version lists and a procedure as arguments.
Sign in to reply to this message.
On 2017/02/14 15:37:35, pwm wrote: > On 2017/02/14 15:15:07, git wrote: > > I like this more because it's more of a typical procedure invocation. > > I think I'll create a new patch with this and a simplified calculate-version > > (that doesn't accept string lists) > > Well, having the procedure as the first argument might be more scheme-ish? > > Maybe best would be to have a "version-compare" function that compares two > version lists using a procedure. Then define "ly:version?" so it calls > "version-compare" with the current LilyPond version (to handle the primary use > case). The "version-compare" function could be David's looping function, but > modified so that it took two version lists and a procedure as arguments. This is what I'm currently working on. However, I found an issue in David's suggestion, namely the (op 0 0) ;; return #t iff op includes equality line. This makes the function return #t when comparing with = and one list shortened Try e.g. (ly:version? = 2 16) This returns #t because in the third iteration when the reference version has run out of elements it compares 0 to 0.
Sign in to reply to this message.
On 2017/02/14 15:37:35, pwm wrote: > On 2017/02/14 15:15:07, git wrote: > > I like this more because it's more of a typical procedure invocation. > > I think I'll create a new patch with this and a simplified calculate-version > > (that doesn't accept string lists) > > Well, having the procedure as the first argument might be more scheme-ish? > > Maybe best would be to have a "version-compare" function that compares two > version lists using a procedure. Then define "ly:version?" so it calls > "version-compare" with the current LilyPond version (to handle the primary use > case). The "version-compare" function could be David's looping function, but > modified so that it took two version lists and a procedure as arguments. It's not a "version-compare" as much as a "lexicographic-compare".
Sign in to reply to this message.
On 2017/02/14 15:46:20, git wrote: > On 2017/02/14 15:37:35, pwm wrote: > > On 2017/02/14 15:15:07, git wrote: > > > I like this more because it's more of a typical procedure invocation. > > > I think I'll create a new patch with this and a simplified calculate-version > > > (that doesn't accept string lists) > > > > Well, having the procedure as the first argument might be more scheme-ish? > > > > Maybe best would be to have a "version-compare" function that compares two > > version lists using a procedure. Then define "ly:version?" so it calls > > "version-compare" with the current LilyPond version (to handle the primary use > > case). The "version-compare" function could be David's looping function, but > > modified so that it took two version lists and a procedure as arguments. > > This is what I'm currently working on. > However, I found an issue in David's suggestion, namely the > (op 0 0) ;; return #t iff op includes equality > line. > This makes the function return #t when comparing with = and one list shortened > Try e.g. (ly:version? = 2 16) > This returns #t because in the third iteration when the reference version has > run out of elements it compares 0 to 0. Have you actually tried it? If your major version is not actually 2.16, you will not even _get_ into a third iteration.
Sign in to reply to this message.
On 2017/02/14 15:49:26, dak wrote: > On 2017/02/14 15:46:20, git wrote: > > On 2017/02/14 15:37:35, pwm wrote: > > > On 2017/02/14 15:15:07, git wrote: > > > > I like this more because it's more of a typical procedure invocation. > > > > I think I'll create a new patch with this and a simplified > calculate-version > > > > (that doesn't accept string lists) > > > > > > Well, having the procedure as the first argument might be more scheme-ish? > > > > > > Maybe best would be to have a "version-compare" function that compares two > > > version lists using a procedure. Then define "ly:version?" so it calls > > > "version-compare" with the current LilyPond version (to handle the primary > use > > > case). The "version-compare" function could be David's looping function, > but > > > modified so that it took two version lists and a procedure as arguments. > > > > This is what I'm currently working on. > > However, I found an issue in David's suggestion, namely the > > (op 0 0) ;; return #t iff op includes equality > > line. > > This makes the function return #t when comparing with = and one list shortened > > Try e.g. (ly:version? = 2 16) > > This returns #t because in the third iteration when the reference version has > > run out of elements it compares 0 to 0. > > Have you actually tried it? If your major version is not actually 2.16, you > will not even _get_ into a third iteration. Ok, I take that back. I admit that the tie-breaker does not work with strict equality. I'll have to think about it.
Sign in to reply to this message.
> > > However, I found an issue in David's suggestion, namely the > > > (op 0 0) ;; return #t iff op includes equality > > > line. > > > This makes the function return #t when comparing with = and one list > shortened > > > Try e.g. (ly:version? = 2 16) > > > This returns #t because in the third iteration when the reference version > has > > > run out of elements it compares 0 to 0. > > > > Have you actually tried it? If your major version is not actually 2.16, you > > will not even _get_ into a third iteration. > > Ok, I take that back. I admit that the tie-breaker does not work with strict > equality. I'll have to think about it. Yes, my interpretation was wrong (as it's not related to the list length). But I've found the solution (I hope) I need a three-way switch when jumping in the loop: - Both lists have no further elements: Return current result - One list has no further elements: Return (op 0 0) - Both lists have further elements: go into the recursion. I'll upload a new patch, combining this with Paul's interface suggestion
Sign in to reply to this message.
On 2017/02/14 15:51:13, dak wrote:
> On 2017/02/14 15:49:26, dak wrote:
> > On 2017/02/14 15:46:20, git wrote:
> > > On 2017/02/14 15:37:35, pwm wrote:
> > > > On 2017/02/14 15:15:07, git wrote:
> > > > > I like this more because it's more of a typical procedure invocation.
> > > > > I think I'll create a new patch with this and a simplified
> > calculate-version
> > > > > (that doesn't accept string lists)
> > > >
> > > > Well, having the procedure as the first argument might be more
scheme-ish?
> > > >
> > > > Maybe best would be to have a "version-compare" function that compares
two
> > > > version lists using a procedure. Then define "ly:version?" so it calls
> > > > "version-compare" with the current LilyPond version (to handle the
primary
> > use
> > > > case). The "version-compare" function could be David's looping
function,
> > but
> > > > modified so that it took two version lists and a procedure as arguments.
> > >
> > > This is what I'm currently working on.
> > > However, I found an issue in David's suggestion, namely the
> > > (op 0 0) ;; return #t iff op includes equality
> > > line.
> > > This makes the function return #t when comparing with = and one list
> shortened
> > > Try e.g. (ly:version? = 2 16)
> > > This returns #t because in the third iteration when the reference version
> has
> > > run out of elements it compares 0 to 0.
> >
> > Have you actually tried it? If your major version is not actually 2.16, you
> > will not even _get_ into a third iteration.
>
> Ok, I take that back. I admit that the tie-breaker does not work with strict
> equality. I'll have to think about it.
How about:
#(define (ly:version? op . rest)
(define eqlop (op 0 0))
(let loop ((ver (ly:version)) (rest rest))
(if (or (null? ver) (null? rest))
eqlop
(let* ((a (car ver)) (b (car rest))
(axb (op a b)) (bxa (op b a)))
(if (eq? axb bxa eqlop)
(loop (cdr ver) (cdr rest))
axb)))))
#(format #t "> 2.19 : ~S, >= 2.19 : ~S\n= 2.16 : ~S, = 2.19 : ~S\n"
(ly:version? > 2 19)
(ly:version? >= 2 19)
(ly:version? = 2 16)
(ly:version? = 2 19))
Sign in to reply to this message.
Reflect discussions.
Sign in to reply to this message.
https://codereview.appspot.com/317270043/diff/20001/scm/lily-library.scm File scm/lily-library.scm (right): https://codereview.appspot.com/317270043/diff/20001/scm/lily-library.scm#newc... scm/lily-library.scm:899: (let* ((a (car v1)) (b (car v2)) Requires v1/v2 to have at least one element. Admittedly quite a reasonable precondition. OIf we have that, we don't need (op 0 0) but can use (op a a) instead. The advantage is that we are no longer bound to _numeric_ comparisons then. This lexicographic compare will then also work with string= and similar. https://codereview.appspot.com/317270043/diff/20001/scm/lily-library.scm#newc... scm/lily-library.scm:903: ((not axb) #f) This will let (version-compare? '(2 18) < '(2 19)) deliver #f after comparing only 2 with 2, right?
Sign in to reply to this message.
https://codereview.appspot.com/317270043/diff/20001/scm/lily-library.scm File scm/lily-library.scm (right): https://codereview.appspot.com/317270043/diff/20001/scm/lily-library.scm#newc... scm/lily-library.scm:899: (let* ((a (car v1)) (b (car v2)) On 2017/02/14 17:29:33, dak wrote: > Requires v1/v2 to have at least one element. Admittedly quite a reasonable > precondition. > > OIf we have that, we don't need (op 0 0) but can use (op a a) instead. The > advantage is that we are no longer bound to _numeric_ comparisons then. This > lexicographic compare will then also work with string= and similar. Acknowledged. https://codereview.appspot.com/317270043/diff/20001/scm/lily-library.scm#newc... scm/lily-library.scm:903: ((not axb) #f) On 2017/02/14 17:29:33, dak wrote: > This will let (version-compare? '(2 18) < '(2 19)) deliver #f after comparing > only 2 with 2, right? Ah yes. I had removed the (and (eq? op =) part, which I shouldn't have done
Sign in to reply to this message.
On 2017/02/14 18:20:56, git wrote:
> Ah yes. I had removed the
> (and (eq? op =)
> part, which I shouldn't have done
Well, that works only for a very particular equivalence. How about something
like
#(define (lexicographically op a b)
(let* ((ca (car a))
(iseql (op ca ca)))
(let loop ((ca ca) (cb (car b)) (a (cdr a)) (b (cdr b)))
(let ((axb (op ca cb)))
(if (and (pair? a) (pair? b)
(eq? axb iseql (op cb ca)))
(loop (car a) (car b) (cdr a) (cdr b))
axb)))))
#(define (ly:version? op . rest)
(lexicographically op (ly:version) rest))
#(format #t "> 2.19 : ~S, >= 2.19 : ~S\n= 2.16 : ~S, = 2.19 : ~S\n"
(ly:version? > 2 19)
(ly:version? >= 2 19)
(ly:version? = 2 16)
(ly:version? = 2 19))
Sign in to reply to this message.
To throw in my own 2cts.
Why not compare strings, looks more straight forward to me.
(define (calculate-version-harm ref-version)
(cond ((string? ref-version) ref-version)
((number-list? ref-version)
(string-concatenate (list-join (map number->string ref-version) ".")))
(else (ly:error "whatever-massage"))))
(define (version-compare? op v1 v2)
"Compare two versions @var{v1} and @var{v2} with the operator @var{op}.
The operator would typically be
string=?, string<?, string<=? , string>? , string>=? , etc."
(op (calculate-version-harm v1) (calculate-version-harm v2)))
;; Examples
(write (version-compare? string=? (lilypond-version) (ly:version)))
(write (version-compare? string>? "2.19.57" (ly:version)))
(write (version-compare? string<=? "2.19.57" (lilypond-version)))
The issue I can imagine: probably more expensive, especially with guilev2
In a ly-file:
$ time lilydevel atest-48.ly
GNU LilyPond 2.19.52
Processing `atest-48.ly'
Parsing...#t#t#f
Success: compilation successfully completed
real 0m1.279s
user 0m1.148s
sys 0m0.128s
Using a build from (not so) recent master, with guile-2.1.6:
$ time lilypond-git atest-48.ly
GNU LilyPond 2.19.55
Import (ice-9 threads) to have access to `call-with-new-thread'.
Import (ice-9 threads) to have access to `current-thread'.
Processing `atest-48.ly'
Parsing...#t#t#f
Success: compilation successfully completed
real 0m5.145s
user 0m5.620s
sys 0m0.120s
Cheers,
Harm
Sign in to reply to this message.
On 2017/02/14 21:03:19, thomasmorley651 wrote:
> To throw in my own 2cts.
>
> Why not compare strings, looks more straight forward to me.
>
> (define (calculate-version-harm ref-version)
> (cond ((string? ref-version) ref-version)
> ((number-list? ref-version)
> (string-concatenate (list-join (map number->string ref-version)
".")))
> (else (ly:error "whatever-massage"))))
>
> (define (version-compare? op v1 v2)
> "Compare two versions @var{v1} and @var{v2} with the operator @var{op}.
> The operator would typically be
> string=?, string<?, string<=? , string>? , string>=? , etc."
> (op (calculate-version-harm v1) (calculate-version-harm v2)))
>
> ;; Examples
> (write (version-compare? string=? (lilypond-version) (ly:version)))
> (write (version-compare? string>? "2.19.57" (ly:version)))
> (write (version-compare? string<=? "2.19.57" (lilypond-version)))
>
> The issue I can imagine: probably more expensive, especially with guilev2
>
> In a ly-file:
>
> $ time lilydevel atest-48.ly
> GNU LilyPond 2.19.52
> Processing `atest-48.ly'
> Parsing...#t#t#f
> Success: compilation successfully completed
>
> real 0m1.279s
> user 0m1.148s
> sys 0m0.128s
>
> Using a build from (not so) recent master, with guile-2.1.6:
>
> $ time lilypond-git atest-48.ly
> GNU LilyPond 2.19.55
> Import (ice-9 threads) to have access to `call-with-new-thread'.
> Import (ice-9 threads) to have access to `current-thread'.
> Processing `atest-48.ly'
> Parsing...#t#t#f
> Success: compilation successfully completed
>
> real 0m5.145s
> user 0m5.620s
> sys 0m0.120s
>
>
> Cheers,
> Harm
Here the values for Urs' proposal
$ time lilydevel atest-48.ly
GNU LilyPond 2.19.52
Processing `atest-48.ly'
Parsing...#t#f#f
Success: compilation successfully completed
real 0m1.307s
user 0m1.204s
sys 0m0.100s
$ time lilypond-git atest-48.ly
GNU LilyPond 2.19.55
Import (ice-9 threads) to have access to `call-with-new-thread'.
Import (ice-9 threads) to have access to `current-thread'.
Processing `atest-48.ly'
Parsing...#f#f#f
Success: compilation successfully completed
real 0m5.239s
user 0m5.756s
sys 0m0.084s
Sign in to reply to this message.
On 2017/02/14 21:08:48, thomasmorley651 wrote: > > The issue I can imagine: probably more expensive, especially with guilev2 Please disregard the time values from my previous posts, they are adulterated by the problems of _LilyPond_ with guile2 Doing all in pure guile-2.1.6 gives: string-comparison: $ time guile --no-auto-compile guile-test.scm #t#t#f real 0m0.038s user 0m0.028s sys 0m0.008s list-comparison: $ time guile --no-auto-compile guile-test.scm #t#f#f real 0m0.037s user 0m0.024s sys 0m0.016s
Sign in to reply to this message.
thomasmorley65@gmail.com writes: > To throw in my own 2cts. > > Why not compare strings, looks more straight forward to me. > > (define (calculate-version-harm ref-version) > (cond ((string? ref-version) ref-version) > ((number-list? ref-version) > (string-concatenate (list-join (map number->string ref-version) > "."))) > (else (ly:error "whatever-massage")))) > > (define (version-compare? op v1 v2) > "Compare two versions @var{v1} and @var{v2} with the operator @var{op}. > The operator would typically be > string=?, string<?, string<=? , string>? , string>=? , etc." > (op (calculate-version-harm v1) (calculate-version-harm v2))) > > The issue I can imagine: probably more expensive, especially with > guilev2 Because (string<? "2.18.27" "2.18.4") => #t > https://codereview.appspot.com/317270043/ -- David Kastrup
Sign in to reply to this message.
Lexicographic comparison and ly:version? wrapper
Sign in to reply to this message.
https://codereview.appspot.com/317270043/diff/40001/scm/lily-library.scm File scm/lily-library.scm (right): https://codereview.appspot.com/317270043/diff/40001/scm/lily-library.scm#newc... scm/lily-library.scm:895: "Lexicographically compare to lists @var{a} and @var{b} using I like David's suggestion for a more general function name, something like "lexicographic-list-compare?", "list-compare?", "lexicographic-compare?", or someting similar? Typo: compare to lists -> compare two lists https://codereview.appspot.com/317270043/diff/40001/scm/lily-library.scm#newc... scm/lily-library.scm:911: sequence or a list of numbers with up to three elements." To me it would be simpler and clearer to only accept a list for the version. That also makes it easy to use a variable: (ly:version? > '(2 18 2)) (ly:version? > my-version) I can't think of advantages to also accepting the other form (aside from saving a few keystrokes). Am I missing something? (ly:version? > 2 18 2)
Sign in to reply to this message.
https://codereview.appspot.com/317270043/diff/40001/scm/lily-library.scm File scm/lily-library.scm (right): https://codereview.appspot.com/317270043/diff/40001/scm/lily-library.scm#newc... scm/lily-library.scm:895: "Lexicographically compare to lists @var{a} and @var{b} using On 2017/02/18 15:36:22, pwm wrote: > I like David's suggestion for a more general function name, something like > "lexicographic-list-compare?", "list-compare?", "lexicographic-compare?", or > someting similar? My reasoning for this name was to capture the specific *behaviour* with regard to missing trailing elements, which is typical for hierarchical versioning schemes. Any kind of generic -compare? name seems to me to hide that. But I'm still open to suggestions. > > Typo: compare to lists -> compare two lists I've fixed this but won't create a patch set *for this* https://codereview.appspot.com/317270043/diff/40001/scm/lily-library.scm#newc... scm/lily-library.scm:911: sequence or a list of numbers with up to three elements." On 2017/02/18 15:36:22, pwm wrote: > To me it would be simpler and clearer to only accept a list for the version. > That also makes it easy to use a variable: > (ly:version? > '(2 18 2)) > (ly:version? > my-version) > > I can't think of advantages to also accepting the other form (aside from saving > a few keystrokes). Am I missing something? > (ly:version? > 2 18 2) I had originally used list arguments only, but David seemed to be insisting (at least through his repeated code suggestions) to use the "open" form. I'd be more than willing to restrict this to one form only, but from my POV this should be the list version (as you suggest, Paul)
Sign in to reply to this message.
Rename secondary function after discussion
Sign in to reply to this message.
Fix omission of previous patch set
Sign in to reply to this message.
https://codereview.appspot.com/317270043/diff/40001/scm/lily-library.scm File scm/lily-library.scm (right): https://codereview.appspot.com/317270043/diff/40001/scm/lily-library.scm#newc... scm/lily-library.scm:895: "Lexicographically compare to lists @var{a} and @var{b} using On 2017/02/18 16:08:10, git wrote: > On 2017/02/18 15:36:22, pwm wrote: > > I like David's suggestion for a more general function name, something like > > "lexicographic-list-compare?", "list-compare?", "lexicographic-compare?", or > > someting similar? > > My reasoning for this name was to capture the specific *behaviour* > with regard to missing trailing elements, which is typical for > hierarchical versioning schemes. Any kind of generic -compare? name > seems to me to hide that. categoric-compare? hierarchic-compare?
Sign in to reply to this message.
|
