[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Woodwind diagrams (issue1425041)

From: Carl . D . Sorensen
Subject: Woodwind diagrams (issue1425041)
Date: Mon, 31 May 2010 14:06:49 +0000

The new patch worked.

I've put some comments in line.

Great job, Mike!
File scm/lily-library.scm (right):
scm/lily-library.scm:492: (cons
In general, I prefer to have the first argument on the line with (cons.
There may be a case where wrapping works better if it's split, but that
should be the exception, not the rule.

Is this really intended as an interval function [(x-min . x-max )] or is
intended as a coordinate function[(x . y)]?  If these functions are
applied to
(x . y) pairs, let's change the name to (coord-* instead of (interval-*.
scm/lily-library.scm:514: (define-public (interval-rotate interval
This looks like you're using an interval as an x-y pair.  But intervals
are (x-min . x-max) or (y-min . y-max) pairs.

Used coord (see bezier.scm, and maybe move all the coord stuff to
lily-library.scm) instead of intervals.
scm/lily-library.scm:520: (radius
(radius should line up with (interval, not ((interval
scm/lily-library.scm:588: (if
It's more standard practice to put the test on the same line as the if.
scm/lily-library.scm:610: (/
I think you should not have the operators by themselves on a line.
File scm/output-ps.scm (right):
scm/output-ps.scm:105: (define (connected-shape pointlist thick x-scale
y-scale connect fill)
I find it much easier to read this function in this indentation format:

(define (connected-shape pointlist thick x-scale y-scale connect fill)
   (ly:format "~a~4f ~4f ~4f ~4f ~a ~a draw_connected_shape"
       (map (lambda (x)
              (apply (if (eq? (length x) 6)
               (lambda (x1 x2 x3 x4 x5 x6)
                 (ly:format "~4f ~4f ~4f ~4f ~4f ~4f 6 " x1 x2 x3 x4 x5
               (lambda (x1 x2)
                 (ly:format "~4f ~4f 2 " x1 x2)))
         (reverse pointlist)))       (length pointlist)
       thick       (if connect "true" "false")
       (if fill "true" "false")))
scm/output-ps.scm:112: (eqv? (length x) 6)
Integer comparisons can always be eq?
File scm/output-svg.scm (right):
scm/output-svg.scm:359: (new-start-angle
line up with second  (new-start-angle, not ((new-start-angle
scm/output-svg.scm:363: (sqrt
line up with (*
scm/output-svg.scm:365: (*
No space is saved by breaking the lines after these operators, because
the indent is the same place as you'd be without the break.  So you
should keep them on the same line:

(+ (* (* y-radius y-radius))
      (* (cos new-start-angle) (cos new-start-angle))

scm/output-svg.scm:446: "M0 0~a ~a"
can we avoid the ~a ~a construct?  That is a potential place for a user
to insert arbitrary PS code.  If we can't, we should either check the
data or identify (in a comment) the potential security risk.
File scm/stencil.scm (right):
scm/stencil.scm:538: "Returns a function drawing an arrow from here to
Should be more like "Returns a function drawing a line from current
point to @var{destination}, with optional arrows of @var{max-size} on
start and end controlled by @var{start?} and @var{end?}."

reply via email to

[Prev in Thread] Current Thread [Next in Thread]