Re: Start work on adding annotations for horizontal paper variables.

[Neil Puttock]

Reviewers: xmichael-k,

Message:
On 2009/11/02 08:01:38, xmichael-k wrote:
> Nice work, very helpful!

Cheers.

> Don't worry if my comments are stupid... ;))

No, they're very useful.


> http://codereview.appspot.com/143071/diff/1/2
> File scm/page.scm (right):

> http://codereview.appspot.com/143071/diff/1/2#newcode38
> Line 38: (define (annotate-x? layout)
> What about
> (define (annotate? layout . dir)
>    (eq? #t (case dir
>              (('X) (ly:output-def-lookup layout 'annotate-x-spacing))
>              (('Y) (ly:output-def-lookup layout 'annotate-y-spacing))
>              (('()) (or (annotate? layout X) (annotate? layout Y)))
>              (else #f)))

I can't get this to work, unfortunately.  The rest argument will be a
list, and `case' doesn't want to match '(X) or '(Y).


> http://codereview.appspot.com/143071/diff/1/2#newcode126
> Line 126: (add-x-stencil (lambda (x)
> Again, this seems to me a little redundant. Maybe one add-stencil
procedure
> would do it?

OK, I'll combine the two procs and add an axis arg.


> http://codereview.appspot.com/143071/diff/1/2#newcode134
> Line 134: (if (annotate-y? layout)
> I would prefer to put the items to annotate in an alist and do a
(for-each)

That would be more elegant. :)

Thanks,
Neil



Description:
Start work on adding annotations for horizontal paper variables.

Please review this at http://codereview.appspot.com/143071

Affected files:
   M scm/page.scm
   M scm/paper-system.scm
   M scm/stencil.scm




_______________________________________________
lilypond-devel mailing list
lilypond-devel@...
http://lists.gnu.org/mailman/listinfo/lilypond-devel

[Michael Käppler-2]

>> http://codereview.appspot.com/143071/diff/1/2#newcode38
>> Line 38: (define (annotate-x? layout)
>> What about
>> (define (annotate? layout . dir)
>>    (eq? #t (case dir
>>              (('X) (ly:output-def-lookup layout 'annotate-x-spacing))
>>              (('Y) (ly:output-def-lookup layout 'annotate-y-spacing))
>>              (('()) (or (annotate? layout X) (annotate? layout Y)))
>>              (else #f)))
>
> I can't get this to work, unfortunately.  The rest argument will be a
> list, and `case' doesn't want to match '(X) or '(Y).

Hmm... I did not test this.
Try this:

(define annotate? layout . dir)
      (case (and (pair? dir) (car dir))
         (('X) (ly:output-def-lookup layout 'annotate-x-spacing))
         (('Y) (ly:output-def-lookup layout 'annotate-y-spacing))
         ((#f) (or (annotate? layout X) (annotate? layout Y))
         (else #f)))

Regards,
Michael



_______________________________________________
lilypond-devel mailing list
lilypond-devel@...
http://lists.gnu.org/mailman/listinfo/lilypond-devel

[Neil Puttock]

2009/11/5 Michael Käppler <xmichael-k@...>:

> Hmm... I did not test this.
> Try this:
>
> (define annotate? layout . dir)
>     (case (and (pair? dir) (car dir))
>        (('X) (ly:output-def-lookup layout 'annotate-x-spacing))
>        (('Y) (ly:output-def-lookup layout 'annotate-y-spacing))
>        ((#f) (or (annotate? layout X) (annotate? layout Y))
>        (else #f)))

You haven't tested this either, have you. :P

I've uploaded a revised version based on your suggestion. :)

Regards,
Neil


_______________________________________________
lilypond-devel mailing list
lilypond-devel@...
http://lists.gnu.org/mailman/listinfo/lilypond-devel