Re: bar-line interface part 2/2 (issue 6498052)
Am 02.09.2012 22:24, schrieb thomasmorle...@googlemail.com: More nit-picking: http://codereview.appspot.com/6498052/diff/7003/python/convertrules.py File python/convertrules.py (right): http://codereview.appspot.com/6498052/diff/7003/python/convertrules.py#newcode3395 python/convertrules.py:3395: @rule ((2, 17, 2), r"New bar line interface") There's no converting rule for the old "|s" (Note the small `s') I'd suggest to do: "|s" -> "|-s" and to add #(define-bar-line "|-s" "" "|" "|") to /scm/bar-line.scm http://codereview.appspot.com/6498052/ You are right. I'll include them in my next patch set, probably after reworking the volta bracket stuff. Thanks, Marc ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: bar-line interface part 2/2 (issue 6498052)
More nit-picking: http://codereview.appspot.com/6498052/diff/7003/python/convertrules.py File python/convertrules.py (right): http://codereview.appspot.com/6498052/diff/7003/python/convertrules.py#newcode3395 python/convertrules.py:3395: @rule ((2, 17, 2), r"New bar line interface") There's no converting rule for the old "|s" (Note the small `s') I'd suggest to do: "|s" -> "|-s" and to add #(define-bar-line "|-s" "" "|" "|") to /scm/bar-line.scm http://codereview.appspot.com/6498052/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: bar-line interface part 2/2 (issue 6498052)
Am 31.08.2012 02:05, schrieb thomasmorle...@googlemail.com: http://codereview.appspot.com/6498052/diff/10001/Documentation/snippets/printing-a-repeat-sign-at-the-beginning-of-a-piece.ly File Documentation/snippets/printing-a-repeat-sign-at-the-beginning-of-a-piece.ly (right): http://codereview.appspot.com/6498052/diff/10001/Documentation/snippets/printing-a-repeat-sign-at-the-beginning-of-a-piece.ly#newcode13 Documentation/snippets/printing-a-repeat-sign-at-the-beginning-of-a-piece.ly:13: A @code{|:} bar line can be printed at the beginning of a piece, by Should be: @code{.|:} Uh, this is part of LSR. Should this be changed for a development release, or when LSR is updated to 2.18? http://codereview.appspot.com/6498052/diff/10001/input/regression/span-bar-break.ly File input/regression/span-bar-break.ly (right): http://codereview.appspot.com/6498052/diff/10001/input/regression/span-bar-break.ly#newcode5 input/regression/span-bar-break.ly:5: texidoc = "At the beginning of a system, the @code{|:} repeat Should be: texidoc = "At the beginning of a system, the @code{.|:} repeat barline is drawn between the staves, but the @code{:|.} is not." http://codereview.appspot.com/6498052/ Ok, done. I do not upload changes yet, because the patchy test will fail anyway. Regards, Marc ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: bar-line interface part 2/2 (issue 6498052)
http://codereview.appspot.com/6498052/diff/10001/Documentation/snippets/printing-a-repeat-sign-at-the-beginning-of-a-piece.ly File Documentation/snippets/printing-a-repeat-sign-at-the-beginning-of-a-piece.ly (right): http://codereview.appspot.com/6498052/diff/10001/Documentation/snippets/printing-a-repeat-sign-at-the-beginning-of-a-piece.ly#newcode13 Documentation/snippets/printing-a-repeat-sign-at-the-beginning-of-a-piece.ly:13: A @code{|:} bar line can be printed at the beginning of a piece, by Should be: @code{.|:} http://codereview.appspot.com/6498052/diff/10001/input/regression/span-bar-break.ly File input/regression/span-bar-break.ly (right): http://codereview.appspot.com/6498052/diff/10001/input/regression/span-bar-break.ly#newcode5 input/regression/span-bar-break.ly:5: texidoc = "At the beginning of a system, the @code{|:} repeat Should be: texidoc = "At the beginning of a system, the @code{.|:} repeat barline is drawn between the staves, but the @code{:|.} is not." http://codereview.appspot.com/6498052/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: bar-line interface part 2/2 (issue 6498052)
Am 30.08.2012 01:46, schrieb thomasmorle...@googlemail.com: Great work so far. Some minor suggestions: http://codereview.appspot.com/6498052/diff/5001/scm/bar-line.scm File scm/bar-line.scm (right): http://codereview.appspot.com/6498052/diff/5001/scm/bar-line.scm#newcode203 scm/bar-line.scm:203: If a user defines some curious bar-lines like: #(define-bar-line "|SS...SS|" "|SS.." "..SS|" "|==...==|") LilyPond will create some weird output without any hint whats wrong, I'd suggest adding a warning like: (map (lambda (x) (if (not (assoc-get x bar-glyph-alist)) (ly:warning (_ "bar-glyph ~a has to be defined before, in separate definition") x))) (list eol-glyph bol-glyph)) and to change the order of the predefined bar-lines at the bottom of the file. Thanks for spotting this! I decided to give a warning and an empty stencil; this has the advantage of the order of the definitions is not relevant. http://codereview.appspot.com/6498052/diff/5001/scm/bar-line.scm#newcode212 scm/bar-line.scm:212: If a user defines his own bar-line and chooses a glyph with string-length greater than 1, compiling will fail without a useful log-message. I'd suggest: (define-public (add-bar-glyph-print-procedure glyph proc) "Specify the single glyph @var{glyph} that calls print procedure @var{proc}. The procedure @var{proc} has to be defined in the form @code{(make-...-bar-line grob extent)} even if the @var{extent} is not used within the routine." (if (or (not (string? glyph)) (> (string-length glyph) 1)) (ly:warning (_ "glyph ~a is not of string-length 1") glyph) (set! bar-glyph-print-procedures (acons glyph proc bar-glyph-print-procedures or sth like this Of course! Done. http://codereview.appspot.com/6498052/diff/5001/scm/bar-line.scm#newcode878 scm/bar-line.scm:878: (define-bar-line ":.|.:" ":|." ".|:" " .|.") The following to lines should be moved to the position directly after ;;repeats (see my comment above) http://codereview.appspot.com/6498052/diff/5001/scm/bar-line.scm#newcode882 scm/bar-line.scm:882: (define-bar-line ":|]" "|" ":|]" " |") I think it should be: (define-bar-line ":|]" ":|]" "" " |") Oh well ... Done. http://codereview.appspot.com/6498052/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: bar-line interface part 2/2 (issue 6498052)
Great work so far. Some minor suggestions: http://codereview.appspot.com/6498052/diff/5001/scm/bar-line.scm File scm/bar-line.scm (right): http://codereview.appspot.com/6498052/diff/5001/scm/bar-line.scm#newcode203 scm/bar-line.scm:203: If a user defines some curious bar-lines like: #(define-bar-line "|SS...SS|" "|SS.." "..SS|" "|==...==|") LilyPond will create some weird output without any hint whats wrong, I'd suggest adding a warning like: (map (lambda (x) (if (not (assoc-get x bar-glyph-alist)) (ly:warning (_ "bar-glyph ~a has to be defined before, in separate definition") x))) (list eol-glyph bol-glyph)) and to change the order of the predefined bar-lines at the bottom of the file. http://codereview.appspot.com/6498052/diff/5001/scm/bar-line.scm#newcode212 scm/bar-line.scm:212: If a user defines his own bar-line and chooses a glyph with string-length greater than 1, compiling will fail without a useful log-message. I'd suggest: (define-public (add-bar-glyph-print-procedure glyph proc) "Specify the single glyph @var{glyph} that calls print procedure @var{proc}. The procedure @var{proc} has to be defined in the form @code{(make-...-bar-line grob extent)} even if the @var{extent} is not used within the routine." (if (or (not (string? glyph)) (> (string-length glyph) 1)) (ly:warning (_ "glyph ~a is not of string-length 1") glyph) (set! bar-glyph-print-procedures (acons glyph proc bar-glyph-print-procedures or sth like this http://codereview.appspot.com/6498052/diff/5001/scm/bar-line.scm#newcode878 scm/bar-line.scm:878: (define-bar-line ":.|.:" ":|." ".|:" " .|.") The following to lines should be moved to the position directly after ;;repeats (see my comment above) http://codereview.appspot.com/6498052/diff/5001/scm/bar-line.scm#newcode882 scm/bar-line.scm:882: (define-bar-line ":|]" "|" ":|]" " |") I think it should be: (define-bar-line ":|]" ":|]" "" " |") http://codereview.appspot.com/6498052/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
bar-line interface part 2/2 (issue 6498052)
LGTM http://codereview.appspot.com/6498052/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel