Re: define-grobs.scm properties not alphabetical

[Carl D. Sorensen]

Mark,

I like this, because it makes the out-of-order stuff be only a programmer's
problem, and programmers can use searches to find the code they're looking
for.




A couple of other comments:

The name of the file is lily-sort.scm, but the comment in the header says
ly-sort.scm.  I much prefer lily-sort.scm.

All of the define-public functions need doc strings.

I think that the doc strings would be better if instead of focusing on the
mechanism (move the special characters to the front of the ASCII set), they
described the function (something like "Determine if string @var{a} is less
than string @var{b} in case-sensitive LilyPond sort order.")

I think the rationale should be put into the comments of the file header,
rather than a pointer to the discussion on the -devel list.  This will help
future developers.

I think that init-list should be defined only once, and used in the various
functions as needed.  One way to achieve that would be to define

(define (ly:char-compare<? a b ci)
  ....)

(define (ly:char<? a b)
  (ly:char-compare<? a b #f))

(define (ly:char-ci<? a b)
  (ly:char-compare<? a b #t))

Similarly, I think there should be only one real definition of ly:string*<?,
which should have ci as a parameter.

Then

(define (ly:string<? a b)
   (ly:string-compare<? a b #f))

(define (ly:string-ci<? a b)
   (ly:string-compare<? a b #t))

Same for alist and symbol.


I think I would prefer mismatch if the idiom were explicit recursion, rather
than the loop construct.  I think it's more scheme idiomatic that way.
Also, I think that mismatch needs a doc string because its function isn't
transparent.

Perhaps something like

(define (mismatch str0 str1 ci)
  "Return a pair containing the first character from str0 and str1 where
the two strings differ.  If one of the strings is a substring of the other,
the entry for that string will be #f."
  (define (helper list0 list1 ci)
            (cond ((and (null? list0) (null? list1))
                     #f)
                  ((null? list0)
                     (cons #f (car list1)))
                  ((null? list1)
                     (cons (car list0) #f))
                  ((not (if ci char-ci=? char=?) (car list0) (car list1)))
                     (cons (car list0) (car list1))
                  (else
                     (helper (cdr list0) (cdr list1)))))

   (helper (string->list str0) (string->list str1) ci))


Note: the above code has not been tested.

I like the recursion rather than the loop because the termination point is
uncertain, so I think the loop structure is less idiomatic.

Also, I have named the variables differently.

I like list0 and list1 instead of a and b because the variable names show
the relationship.  But I would be totally fine with a and b instead.

I have not put a question mark on ci because ? is used in SICP to indicate a
predicate (i.e. a function returning a boolean) rather than a boolean value
directly.

I've used the (not ....) on the next-to-last cond clause because I want the
recursive call to show up last in the structure.

Although it works to have both a globally-defined and locally-defined
mismatch in ly:string-*<?, I think it's potentially confusing for new
developers, so I'd recommend using different names.

Anyway, I think this is a solid contribution.  Thanks!

Carl