Re: Issue 3951: Fix broken LSR links in docs. (issue 102410043 by markpole...@gmail.com)
Yeah! I've just uploaded an update to Sebastiano's `cd' package for LaTeX, and I was also bitten by the change of his e-mail address... AFAIK, the URL change is out of Sebastiano's control. Thanks for working on this. https://codereview.appspot.com/102410043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Issue 3951: Fix broken LSR links in docs. (issue 102410043 by markpole...@gmail.com)
[I mean `uploaded' to CTAN, of course - this is not related to lilypond.] https://codereview.appspot.com/102410043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Issue 3254: align unassociated lyrics using NoteColumn extent. (issue 108110044 by janek.lilyp...@gmail.com)
LGTM.
https://codereview.appspot.com/108110044/diff/1/input/regression/unassociated-lyrics-alignment.ly
File input/regression/unassociated-lyrics-alignment.ly (right):
https://codereview.appspot.com/108110044/diff/1/input/regression/unassociated-lyrics-alignment.ly#newcode5
input/regression/unassociated-lyrics-alignment.ly:5: texidoc = Lyrics
without an associatedVoice should align properly.
Please format this text with @code{...} and the like.
https://codereview.appspot.com/108110044/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Issue 3254: align unassociated lyrics using NoteColumn extent. (issue 108110044 by janek.lilyp...@gmail.com)
LGTM, thanks! https://codereview.appspot.com/108110044/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Issue 3254: align unassociated lyrics using NoteColumn extent. (issue 108110044 by janek.lilyp...@gmail.com)
https://codereview.appspot.com/108110044/diff/60001/lily/lyric-engraver.cc File lily/lyric-engraver.cc (right): https://codereview.appspot.com/108110044/diff/60001/lily/lyric-engraver.cc#newcode187 lily/lyric-engraver.cc:187: Syllable will be attached to a PaperColumn instead.)); I suggest that you change this last sentence to Syllables will be attached to PaperColumn grobs instead. so that everything is in plural form. https://codereview.appspot.com/108110044/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Pitch alterations use SCM rather than flower rationals (issue 106190043 by d...@gnu.org)
Or maybe some cpp macro trickery improves the legibility. https://codereview.appspot.com/106190043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Cleanup in self_alignment_interface (issue 105410046 by janek.lilyp...@gmail.com)
LGTM (without any testing). https://codereview.appspot.com/105410046/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
\retrograde fails on slurs (issue 110540043 by d...@gnu.org)
LGTM.
https://codereview.appspot.com/110540043/diff/1/scm/modal-transforms.scm
File scm/modal-transforms.scm (right):
https://codereview.appspot.com/110540043/diff/1/scm/modal-transforms.scm#newcode134
scm/modal-transforms.scm:134: \n(see).
@code{transposer-factory}
https://codereview.appspot.com/110540043/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: \retrograde fails on slurs (issue 110540043 by d...@gnu.org)
Thanks. LGTM. https://codereview.appspot.com/110540043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Allow specifying different alignment for grob and its parent (issue 118950043 by janek.lilyp...@gmail.com)
LGTM.
https://codereview.appspot.com/118950043/diff/1/scm/define-grob-properties.scm
File scm/define-grob-properties.scm (right):
https://codereview.appspot.com/118950043/diff/1/scm/define-grob-properties.scm#newcode814
scm/define-grob-properties.scm:814: Value @w{@code{-1}} means left
aligned, @code{0}@tie{}centered, and
... value@tie{}@code{-1} ...
https://codereview.appspot.com/118950043/diff/1/scm/define-grob-properties.scm#newcode818
scm/define-grob-properties.scm:818: setting @code{LyricText} alignment
to @code{'(-1 . 1)} will result in
... will result in a ...
https://codereview.appspot.com/118950043/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Interpret and in \chordmode (issue 123160043 by d...@gnu.org)
LGTM. https://codereview.appspot.com/123160043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Don't rescale \layout in \score markup when already scaled (issue 124240043 by d...@gnu.org)
LGTM. https://codereview.appspot.com/124240043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Issue 2507: Stop the entanglement of context properties and grob property internals (issue 126280043 by d...@gnu.org)
Fix incredibly reckless context/grob property mixup in event-listener I would rather call this clueless :-) https://codereview.appspot.com/126280043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Issue 3072: Nested overrides get confused with multiple contexts in play (issue 131770043 by d...@gnu.org)
I don't actually understand the code, but I like the encapsulation, so: LGTM. https://codereview.appspot.com/131770043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Issue 3518: Support temporary divisi staves (issue 131970043 by d...@gnu.org)
LGTM. Very nice! I guess it makes sense to actually provide `\boring' and `\tricky' with better names... https://codereview.appspot.com/131970043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Get rid of lilypond-book warning for indent=1.5cm in essay (issue 127700043 by d...@gnu.org)
LGTM. This is the kind of patches that I would immediately apply to staging, not wasting a whole build cycle... https://codereview.appspot.com/127700043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Provide some more smob functionality and use it (issue 135240043 by d...@gnu.org)
LGTM. Very nice! This indeed feels more C++ now. https://codereview.appspot.com/135240043/diff/20001/scripts/auxiliar/smob-convert.sh File scripts/auxiliar/smob-convert.sh (right): https://codereview.appspot.com/135240043/diff/20001/scripts/auxiliar/smob-convert.sh#newcode2 scripts/auxiliar/smob-convert.sh:2: for i in $(git grep -l '^\s\+DECLARE_SIMPLE_SMOBS' lily) Will this script be ever used again? It perhaps deserves a comment... https://codereview.appspot.com/135240043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Lexer: don't go through int for fractions (issue 140170043 by d...@gnu.org)
LGTM. https://codereview.appspot.com/140170043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Limit tuplet bracket length to actual tuplet length (issue 139190044 by d...@gnu.org)
LGTM. https://codereview.appspot.com/139190044/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Issue 346: monochord issues (issue 138150043 by d...@gnu.org)
LGTM, thanks. https://codereview.appspot.com/138150043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Issue 4156: Define Smob constructors. (issue 152370043 by nine.fierce.ball...@gmail.com)
LGTM, from visual inspection only :-) https://codereview.appspot.com/152370043/diff/1/lily/include/book.hh File lily/include/book.hh (right): https://codereview.appspot.com/152370043/diff/1/lily/include/book.hh#newcode31 lily/include/book.hh:31: typedef SmobBook base_type; I would insert an empty line before the `public' keyword... https://codereview.appspot.com/152370043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Issue 461: LilyPond should accept a tie between notes which are enharmonically identical (issue 159050043 by d...@gnu.org)
Very nice! Thanks for implementing this feature. LGTM. https://codereview.appspot.com/159050043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Issue 4151: implicitTimeSignatureVisibility-initialTimeSignatureVisibility (issue 163520043 by tdanielsmu...@googlemail.com)
LGTM https://codereview.appspot.com/163520043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
staff-symbol-referencer: ledger logic; issue 4184 (issue 167040043 by k-ohara5...@oco.net)
LGTM https://codereview.appspot.com/167040043/diff/1/lily/side-position-interface.cc File lily/side-position-interface.cc (right): https://codereview.appspot.com/167040043/diff/1/lily/side-position-interface.cc#newcode383 lily/side-position-interface.cc:383: /* If we are closer to the staff than the notehad, quantize for ledger lines. */ s/notehad/notehead/ But I wonder whether this is the best description – it's rather about note heads and attached grobs having the same direction, right? https://codereview.appspot.com/167040043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Issue 216: ability to change staff line spacing inside a \layout{} block (issue 168700043 by d...@gnu.org)
LGTM. https://codereview.appspot.com/168700043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
issue 2724 (issue 174230043 by k-ohara5...@oco.net)
LGTM. https://codereview.appspot.com/174230043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: note-spacing: pad accidentals from neighbor as far as from parent; issue 3869 (issue 65260044)
Aaand another LGTM :-) https://codereview.appspot.com/65260044/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
multi-measure rest: include normal note-spacing; issue 3135 (issue 134220043 by k-ohara5...@oco.net)
LGTM. https://codereview.appspot.com/134220043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
improve initial estimate of beamed-rest position; issue 472 (issue 179320043 by k-ohara5...@oco.net)
LGTM. https://codereview.appspot.com/179320043/diff/1/lily/beam.cc File lily/beam.cc (right): https://codereview.appspot.com/179320043/diff/1/lily/beam.cc#newcode1357 lily/beam.cc:1357: /* Estimate the closest beam to be four positions away from the heads.. */ I always notice the most unimportant things, e.g. two final dots... :-) https://codereview.appspot.com/179320043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
widen multimeasure rests with numbers (issue 173280043 by k-ohara5...@oco.net)
LGTM, save a small remark. https://codereview.appspot.com/173280043/diff/40001/scm/define-grob-properties.scm File scm/define-grob-properties.scm (right): https://codereview.appspot.com/173280043/diff/40001/scm/define-grob-properties.scm#newcode1287 scm/define-grob-properties.scm:1287: containing a multimeasure rest, per factor of two in its duration.) I've seen the code, so I understand this sentence. However, I think that other people will stumble... Can you reformulate this to be more verbose? https://codereview.appspot.com/173280043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Issue 4211: Add an alternative quarter rest shaped like a mirrored Z. (issue 177640043 by nine.fierce.ball...@gmail.com)
LGTM, thanks! https://codereview.appspot.com/177640043/diff/1/mf/feta-rests.mf File mf/feta-rests.mf (right): https://codereview.appspot.com/177640043/diff/1/mf/feta-rests.mf#newcode416 mf/feta-rests.mf:416: labels (9, 12); Please remove those two label statements. You are rotating and flipping the whole shape, however, point labels don't follow, making them look arbitrary. https://codereview.appspot.com/177640043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Issue 4211: Add an alternative quarter rest shaped like a mirrored Z. (issue 177640043 by nine.fierce.ball...@gmail.com)
https://codereview.appspot.com/177640043/diff/20001/mf/feta-rests.mf File mf/feta-rests.mf (right): https://codereview.appspot.com/177640043/diff/20001/mf/feta-rests.mf#newcode438 mf/feta-rests.mf:438: rest := rest xscaled -1 shifted (w, 0); Please use tabs for indentation https://codereview.appspot.com/177640043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
docstring (issue 187140044 by k-ohara5...@oco.net)
Some nits. Thanks for adding this!
https://codereview.appspot.com/187140044/diff/20001/lily/separation-item.cc
File lily/separation-item.cc (right):
https://codereview.appspot.com/187140044/diff/20001/lily/separation-item.cc#newcode190
lily/separation-item.cc:190: Draws the @code{horizontal-sklines} of
each @code{PaperColumn},
s/sklines/skylines/
https://codereview.appspot.com/187140044/diff/20001/lily/separation-item.cc#newcode191
lily/separation-item.cc:191: showing the shapes used to the minimum
distances between
`used to'? This sentence looks weird...
https://codereview.appspot.com/187140044/diff/20001/lily/separation-item.cc#newcode193
lily/separation-item.cc:193: before staves have been spaced on the
page.)
s/have been spaced/ have been spaced (vertically)/
https://codereview.appspot.com/187140044/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
skyline.cc: merge algorithm terminates independent of floating-point (issue 186530043 by k-ohara5...@oco.net)
LGTM https://codereview.appspot.com/186530043/diff/1/lily/skyline.cc File lily/skyline.cc (right): https://codereview.appspot.com/186530043/diff/1/lily/skyline.cc#newcode249 lily/skyline.cc:249: printf (We are here); Debugging remnants? https://codereview.appspot.com/186530043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: skyline.cc: merge algorithm terminates independent of floating-point (issue 186530043 by k-ohara5...@oco.net)
LGTM. Maybe it makes sense to commit the removal of the `deholify' stuff separately. https://codereview.appspot.com/186530043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
lilypond-what-beat.el: Allow \tuplet to work like \times (issue 186570043 by d...@gnu.org)
LGTM. https://codereview.appspot.com/186570043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Reduce size of PDF files when inc. in *TeX docs (issue 194090043 by pkx1...@gmail.com)
Ah, nice! I wasn't aware of this script. Thanks for mentioning it. https://codereview.appspot.com/194090043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Allow independent adjustment of minimum length for spanner siblings (issue 201140043 by david.nales...@gmail.com)
https://codereview.appspot.com/201140043/diff/20001/lily/spanner.cc File lily/spanner.cc (right): https://codereview.appspot.com/201140043/diff/20001/lily/spanner.cc#newcode394 lily/spanner.cc:394: /* Minor nit: Please use spaces, not tabs. https://codereview.appspot.com/201140043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Allow independent adjustment of minimum length for spanner siblings (issue 201140043 by david.nales...@gmail.com)
LGTM. https://codereview.appspot.com/201140043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Clean up inconsistencies in engraver-init.ly and performer-init.ly (issue 199460043 by thomasmorle...@gmail.com)
LGTM. It would be nice if David's checker script could be added, too. https://codereview.appspot.com/199460043/diff/20001/ly/performer-init.ly File ly/performer-init.ly (right): https://codereview.appspot.com/199460043/diff/20001/ly/performer-init.ly#newcode174 ly/performer-init.ly:174: \accepts ChordNames I wonder whether it makes sense to sort the many \accept lines alphabetically... https://codereview.appspot.com/199460043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Reduce size of PDF files when inc. in *TeX docs (issue 194090043 by pkx1...@gmail.com)
Thank *you* for your hard work. Here's the next round of comments.
https://codereview.appspot.com/194090043/diff/40001/Documentation/de/usage/running.itely
File Documentation/de/usage/running.itely (right):
https://codereview.appspot.com/194090043/diff/40001/Documentation/de/usage/running.itely#newcode160
Documentation/de/usage/running.itely:160: pdftex-, xetex-, oder
luatex-Dokumente eingebettet werden;
@w{pdftex-}, @w{xetex-},
https://codereview.appspot.com/194090043/diff/40001/Documentation/de/usage/running.itely#newcode162
Documentation/de/usage/running.itely:162: zusammenzufassen. Mit
pdfsizeopt.py sind dann noch weitere
@file{pdfsizeopt.py}
https://codereview.appspot.com/194090043/diff/40001/Documentation/de/usage/running.itely#newcode166
Documentation/de/usage/running.itely:166: notation.pdf (Lilypond 2.18.2)
ist ca. 26MB groß, erzeugt
@file{notation.pdf}
https://codereview.appspot.com/194090043/diff/40001/Documentation/usage/running.itely
File Documentation/usage/running.itely (right):
https://codereview.appspot.com/194090043/diff/40001/Documentation/usage/running.itely#newcode185
Documentation/usage/running.itely:185: @code{PDF} files generated will
be much larger than normal (due to
I think there is no reason to use @code{...} here.
https://codereview.appspot.com/194090043/diff/40001/Documentation/usage/running.itely#newcode186
Documentation/usage/running.itely:186: little or no font optimization).
However, if two or more files are
Please always use two spaces after a full stop.
https://codereview.appspot.com/194090043/diff/40001/Documentation/usage/running.itely#newcode187
Documentation/usage/running.itely:187: to be included within
@w{@code{pdftex}}, @w{@code{xetex} or
Ditto: no @code necessary, since you don't talk about the binary itself
but the respective program in general.
https://codereview.appspot.com/194090043/diff/40001/Documentation/usage/running.itely#newcode189
Documentation/usage/running.itely:189: gostscript) resulting @code{PDF}
in @emph{significantly} smaller files.
s/gostscript/ghostscript. And somehow the whole sentence seems to be
mangled...
https://codereview.appspot.com/194090043/diff/40001/lily/general-scheme.cc
File lily/general-scheme.cc (right):
https://codereview.appspot.com/194090043/diff/40001/lily/general-scheme.cc#newcode307
lily/general-scheme.cc:307: Return true if the command line includes
the --bigpdf parameter.
@option{--bigpdf}
https://codereview.appspot.com/194090043/diff/40001/lily/main.cc
File lily/main.cc (right):
https://codereview.appspot.com/194090043/diff/40001/lily/main.cc#newcode161
lily/main.cc:161: {0, bigpdfs, 'b', _i(generate big pdf files)},
s/pdf/PDF/
https://codereview.appspot.com/194090043/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Reduce size of PDF files when inc. in *TeX docs (issue 194090043 by pkx1...@gmail.com)
David's concerns are very specific to the Lilypond documentation, not covering the general case. Many programs simply can't process PS output at all, so the suggestion to collect PS data that gets reduced later on is not applicable. The only valid alternative is to make Lilypond natively produce PDF, but this is a long-term solution. And it seems to me that even then we will need a '--bigpdf' option (but implemented in a different way) to allow optimal PDF merging later on by post-processing tools. For this reason I vote to include Knut's work right now, since it quickly solves the given issue in a reliable way, with the only ugliness of having very large intermediate files. https://codereview.appspot.com/194090043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Reduce size of PDF files when inc. in *TeX docs (issue 194090043 by pkx1...@gmail.com)
Well, we get a large size reduction even if we don't use pdfsizeopt! Using this program is an extra bonus but not mandatory. And you are right, I hope that Péter fixes the reported issues, provided someone is going to add them to the bug tracker (which hasn't happened yet, looking at https://code.google.com/p/pdfsizeopt/issues/list). The hyperlink issue is not related to the --bigpdf option (since it is a bug in ghostscript), so I don't think that this is a showstopper. Regarding your b) issue: I fully agree. Contacting Péter might be very helpful. Nevertheless, this takes time. Given that it should be straightforward to make --bigpdf a no-op in case it is no longer useful, I still vote for incorporating the patch. https://codereview.appspot.com/194090043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Issue 2535: Staccato on stem side alignment when other articulations are present (issue 196260043 by david.nales...@gmail.com)
LGTM, thanks! https://codereview.appspot.com/196260043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Issue 2535: Staccato on stem side alignment when other articulations are present (issue 196260043 by david.nales...@gmail.com)
On 2015/01/27 07:26:53, dak wrote: It seems to me like this number-pair consists of two settings that would almost always be adjusted independently. Wouldn't it make more sense to make a separate property here? That way, the user would not need to remember and restate the setting he is not interested in. IMHO it's better to logically stick stuff together. We already have far too much properties, and in this particular case I doubt that the user will change the values very often so I think the benefits of having a single property are exceeding the potential disadvantages. https://codereview.appspot.com/196260043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Issue 2535: Staccato on stem side alignment when other articulations are present (issue 196260043 by david.nales...@gmail.com)
https://codereview.appspot.com/196260043/diff/20001/input/regression/script-shift.ly File input/regression/script-shift.ly (right): https://codereview.appspot.com/196260043/diff/20001/input/regression/script-shift.ly#newcode6 input/regression/script-shift.ly:6: means centered on the stem.) I'm going by the American practice that the period goes inside the parentheses if the parentheses enclose the sentence, outside if the sentence begins outside the parentheses. I'm happy to change it back though--it's kind of pedantic of me! I fully agree with what you say, however, parentheses do *not* enclose a full sentence here (contrary to another place of the patch). https://codereview.appspot.com/196260043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Issue 2535: Staccato on stem side alignment when other articulations are present (issue 196260043 by david.nales...@gmail.com)
LGTM. https://codereview.appspot.com/196260043/diff/20001/input/regression/script-shift.ly File input/regression/script-shift.ly (right): https://codereview.appspot.com/196260043/diff/20001/input/regression/script-shift.ly#newcode6 input/regression/script-shift.ly:6: means centered on the stem.) The old code is correct... https://codereview.appspot.com/196260043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Reduce size of PDF files when inc. in *TeX docs (issue 194090043 by pkx1...@gmail.com)
LGTM, thanks! I only have some minor comments regarding improved
legibility of the source code.
https://codereview.appspot.com/194090043/diff/1/Documentation/de/usage/running.itely
File Documentation/de/usage/running.itely (right):
https://codereview.appspot.com/194090043/diff/1/Documentation/de/usage/running.itely#newcode160
Documentation/de/usage/running.itely:160: pdftex-, xetex-, oder
luatex-Dokumente eingebettet werden,
I would end this line with `;' or `:' instead of a comma.
https://codereview.appspot.com/194090043/diff/1/ps/encodingdefs.ps
File ps/encodingdefs.ps (right):
https://codereview.appspot.com/194090043/diff/1/ps/encodingdefs.ps#newcode8
ps/encodingdefs.ps:8: /LilyNoteHeadEncoding [ /.notdef
/noteheads.d0doFunk /noteheads.d0fa
For better orientation, please reformat this to have a fixed number of
entries per line (I suggest 4 items), together with comments that
indicate the current index (something like `% 0x50').
https://codereview.appspot.com/194090043/diff/1/ps/encodingdefs.ps#newcode91
ps/encodingdefs.ps:91: /noteheads.d0doFunk {01 show} def
/noteheads.d0fa {02 show} def
Here, I would prefer one entry per line.
https://codereview.appspot.com/194090043/diff/1/ps/encodingdefs.ps#newcode214
ps/encodingdefs.ps:214: /LilyScriptEncoding [ /.notdef
/clefs.blackmensural.c
The same comment as above.
https://codereview.appspot.com/194090043/diff/1/scm/output-ps.scm
File scm/output-ps.scm (right):
https://codereview.appspot.com/194090043/diff/1/scm/output-ps.scm#newcode126
scm/output-ps.scm:126: (ly:format currentpoint ~4f ~4f rmoveto ~a
moveto ~4f 0 rmoveto x y g w)))
Please reformat this (and similar) code to stay within the
80-characters-per-line limit if possible.
https://codereview.appspot.com/194090043/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Reduce size of PDF files when inc. in *TeX docs (issue 194090043 by pkx1...@gmail.com)
https://codereview.appspot.com/194090043/diff/20001/Documentation/de/usage/running.itely
File Documentation/de/usage/running.itely (right):
https://codereview.appspot.com/194090043/diff/20001/Documentation/de/usage/running.itely#newcode160
Documentation/de/usage/running.itely:160: pdftex-, xetex-, oder
luatex-Dokumente eingebettet werden;
I suggest to write this as
@w{pdftex-}, @w{xetex-}, ...
to indicate that the hyphen is a non-breakable one. This probably
doesn't work correctly with HTML, but IMHO such cases should be tagged.
https://codereview.appspot.com/194090043/diff/20001/Documentation/de/usage/running.itely#newcode161
Documentation/de/usage/running.itely:161: es ermöglicht ghostscript im
Anschluß, duplizierte Font-Daten
According to new German orthography, this should be `Anschluss'.
https://codereview.appspot.com/194090043/diff/20001/Documentation/de/usage/running.itely#newcode166
Documentation/de/usage/running.itely:166: notation.pdf (Lilypond 2.18.2)
ist ca. 26MB groß, erzeugt
... ca.@: 26MB ...
https://codereview.appspot.com/194090043/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Reduce size of PDF files when inc. in *TeX docs (issue 194090043 by pkx1...@gmail.com)
Thanks for your help and assistance! For better orientation, please reformat this to have a fixed number of entries per line (I suggest 4 items), I did 3 items maximum and kept things within the line length. Basically, this is fine. However, three is incommensurable to 16, ... Werner, I don't understand what you mean by ...together with comments that indicate the current index (something like `% 0x50'). Example: % 0x00 /foo /bar /baz /bla /buf /urgh /pfft /zap /boinck % 0x09 ... Please reformat this (and similar) code to stay within the 80-characters-per-line limit if possible. Hope this is better (I don't code so am not sure if there are specific 'rules' about how scm is formatted here. It looks better, thanks. In case of doubt, the formatting produced by Emacs is the one we follow. https://codereview.appspot.com/194090043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Reduce size of PDF files when inc. in *TeX docs (issue 194090043 by pkx1...@gmail.com)
I don't object to the name! I only state that the option's name doesn't have an explanation in the English documentation, and I think it would be good if it gets added. https://codereview.appspot.com/194090043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Reduce size of PDF files when inc. in *TeX docs (issue 194090043 by pkx1...@gmail.com)
https://codereview.appspot.com/194090043/diff/20001/Documentation/usage/running.itely
File Documentation/usage/running.itely (right):
https://codereview.appspot.com/194090043/diff/20001/Documentation/usage/running.itely#newcode187
Documentation/usage/running.itely:187: font data which can make
significant reductions in file size.
One point is missing: Why is the option called --bigpdfs if the we get
significant reductions in file size?
https://codereview.appspot.com/194090043/diff/20001/Documentation/usage/running.itely#newcode199
Documentation/usage/running.itely:199: Using @code{pdfsizeopt.py} can
then be used to further optimize the size
I would start with
Optionally, using @code{pdfsizeopt.py} ...
to indicate that this is an extra bonus.
https://codereview.appspot.com/194090043/diff/20001/Documentation/usage/running.itely#newcode206
Documentation/usage/running.itely:206:
Should the (current) problem with external links be mentioned here too?
https://codereview.appspot.com/194090043/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Reduce size of PDF files when inc. in *TeX docs (issue 194090043 by pkx1...@gmail.com)
Hmm. I can't find in your description that `--bigpdfs' creates *big* output files that get later reduced to small one by running ghostscript again. https://codereview.appspot.com/194090043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Reduce size of PDF files when inc. in *TeX docs (issue 194090043 by pkx1...@gmail.com)
Knut, *your* patch set has this, but James's version (in patch set 2) misses it. https://codereview.appspot.com/194090043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Replace most uses of scm_{from,to}_locale_* with fixed encodings (issue 219780044 by d...@gnu.org)
LGTM. https://codereview.appspot.com/219780044/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Avoid floating-point compares on Stem:head_positions; issue 4310 (issue 217720043 by k-ohara5...@oco.net)
LGTM. https://codereview.appspot.com/217720043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Avoid floating-point compares on Stem:head_positions; issue 4310 (issue 217720043 by k-ohara5...@oco.net)
Any idea how to find it? https://codereview.appspot.com/217720043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Decrease space between vertical beams by length-fraction. (issue 214250043 by hanw...@gmail.com)
Thanks for the test diffs. Looking at the regression tests, I see some degradations. . The second beam in `grace-start' is too far offset from the staff. Ditto for grace-`stem-length' and others. Note that I got similar glitches earlier too in my real-life scores but was never be able to reduce them to a minimum example, since it seems to be highly dependent on the surrounding grobs. So maybe this is another bug that just has become more visible here. . Regarding `magnifyMusic-dots-beamlets' I wonder whether a minimum distance between beam lines for a given beam thickness should be retained to avoid the visual clash shown in the first beam. . `quote-cue-event-types' now shows too short stems for the grace notes IMHO. https://codereview.appspot.com/214250043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Make multimeasure rests obey tweaks (issue 206370043 by d...@gnu.org)
LGTM. https://codereview.appspot.com/206370043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Fix issue 4355 -- broken beam subdivision (issue 226700043 by carl.d.soren...@gmail.com)
LGTM. https://codereview.appspot.com/226700043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Fix issue 4355 -- broken beam subdivision (issue 226700043 by carl.d.soren...@gmail.com)
LGTM. https://codereview.appspot.com/226700043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: absolute pitch entry: accept an offset octave (issue 235010043 by k-ohara5...@oco.net)
Nice! However, I second David's concern: Please use \absolute pitch instead of \absolute number https://codereview.appspot.com/235010043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: absolute pitch entry: accept an offset octave (issue 235010043 by k-ohara5...@oco.net)
I also favour
\absolute c'' { ... }
in Keith's original interpretation. However, I suggest to document
somewhere that only letter `c' is supported.
https://codereview.appspot.com/235010043/diff/60001/ly/music-functions-init.ly
File ly/music-functions-init.ly (right):
https://codereview.appspot.com/235010043/diff/60001/ly/music-functions-init.ly#newcode38
ly/music-functions-init.ly:38: by the nubmer of octave marks on
@var{pitch}. Wrapping as
s/nubmer/number/
https://codereview.appspot.com/235010043/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add stencil-flip function (issue 235090043 by paulwmor...@gmail.com)
LGTM.
https://codereview.appspot.com/235090043/diff/1/input/regression/stencil-scale.ly
File input/regression/stencil-scale.ly (right):
https://codereview.appspot.com/235090043/diff/1/input/regression/stencil-scale.ly#newcode8
input/regression/stencil-scale.ly:8: its bounding box.)
Please use two spaces after a period that ends a sentence.
https://codereview.appspot.com/235090043/diff/1/scm/stencil.scm
File scm/stencil.scm (right):
https://codereview.appspot.com/235090043/diff/1/scm/stencil.scm#newcode667
scm/stencil.scm:667: @var{axis} is 0 for X-axis, 1 for Y-axis.
Can you somehow add the words `horizontally' and `vertically'?
Additionally, I think it helps to tell the reader where the mirroring
axis is positioned.
https://codereview.appspot.com/235090043/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add stencil-flip function (issue 235090043 by paulwmor...@gmail.com)
https://codereview.appspot.com/235090043/diff/40001/scm/stencil.scm
File scm/stencil.scm (right):
https://codereview.appspot.com/235090043/diff/40001/scm/stencil.scm#newcode668
scm/stencil.scm:668: An @var{axis} of Y or 1 will flip it vertically.
What about:
Value @code{X} (or @code{0}) for @var{axis} flips it horizontally.
Value @code{Y} (or @code{1}) flips it vertically.
Exactly defined values for a variable should be represented with
`@code'. And I'm a great fan of avoiding future tense in documentation
:-)
https://codereview.appspot.com/235090043/diff/40001/scm/stencil.scm#newcode669
scm/stencil.scm:669: @var{stil} is flipped in place. Its position, the
I suggest s/./;/
https://codereview.appspot.com/235090043/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Start up GUILE type system before anything else (issue 234260043 by d...@gnu.org)
LGTM. https://codereview.appspot.com/234260043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Issue 4372: convert-ly misses some bflat - b-flat (issue 233300043 by d...@gnu.org)
LGTM. https://codereview.appspot.com/233300043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Use mkstemp for intermediate ps files (issue 233230044 by truer...@gmail.com)
LGTM https://codereview.appspot.com/233230044/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Decrease space between vertical beams by length-fraction. (issue 214250043 by hanw...@gmail.com)
* I had a further look at the short stems, and should have fixed more cases by disabling forbidden beam quanting in case of grace notes. Yes, everything looks OK now, thanks. * magnifyMusic: can you show examples of what music at 50% scale should look like in print? No, I can't. My observation was not targeted at anything particular. It seems that you are asking for a separate beam-gap-fraction variable? No, I don't :-) To be serious: I suggest working on such a feature only in case someone *really* needs it. https://codereview.appspot.com/214250043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add stencil-flip function (issue 235090043 by paulwmor...@gmail.com)
LGTM. https://codereview.appspot.com/235090043/diff/60001/input/regression/stencil-flip.ly File input/regression/stencil-flip.ly (right): https://codereview.appspot.com/235090043/diff/60001/input/regression/stencil-flip.ly#newcode1 input/regression/stencil-flip.ly:1: \version 2.19.19 I would rename the file to `flip-stencil.ly' now. And everywhere you need s/stencil-flip/flip-stencil/ of course. https://codereview.appspot.com/235090043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: absolute pitch entry: accept an offset octave (issue 235010043 by k-ohara5...@oco.net)
The best choice seems to be \fixed, both for the good fit of the meaning of the word to the command, and because it is relatively easy to type. If we allow its reference pitch to be optional, then the relatively new command \absolute is the same as \fixed with no reference pitch, and we can document \fixed briefly where we had documented \absolute. Sounds sensible to me. Given that we are currently producing development releases, I suggest that this gets implemented, then we simply wait a few months so that people can test it in real life, and then we do a final decision. https://codereview.appspot.com/235010043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add sans-serif and monospace fonts (issue 224800043 by truer...@gmail.com)
Letting PostScript ask for Helvetica which will let GhostScript fall back to the URW version when the original Helvetica is not available. If I understand correctly, we currently ask for and embed the URW version. But maybe printers have their own way to resubstitute the original. No idea. Today, it is very important to *embed* fonts since the world has changed from PS to PDF, and assuming that the original PS fonts are available everywhere is no longer true. So asking for and embedding the free URW fonts is the right way to go. An additional bonus is that the URW fonts come with more glyphs than the original Adobe fonts. Additionally, Adobe itself distributed multiple variants of its core PS fonts, depending on the locale – for example, I have the AFM files for the font versions with 229 and 313 glyphs, respectively. In summary, there is no such thing as the `original Helvetica' that works everywhere as soon as you are using some non-ASCII glyphs... https://codereview.appspot.com/224800043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Decrease space between vertical beams by length-fraction. (issue 214250043 by hanw...@gmail.com)
Thanks again for the regtests. All my previous comments still hold, unfortunately. https://codereview.appspot.com/214250043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add sans-serif and monospace fonts (issue 224800043 by truer...@gmail.com)
Wouldn't it make more sense to have one setting for all of the included fonts? Are there really circumstances where we can expect all of these to be different? Probably not. I think that the patch is a proof of concept, and we should comment on it. https://codereview.appspot.com/224800043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add sans-serif and monospace fonts (issue 224800043 by truer...@gmail.com)
I like the current overall look for _this_ choice of fonts better afterwards as compared to before, even though the C glyph has a more conspicuous rounding and the G glyph has the well-known somewhat poignant Helvetica shape. Yes, it's more consistent. On the other hand, I very much dislike Helvetica, and maybe there's a better choice eventually... It's a pity that Pango does not seem to support some setup where there would be a graceful upgradation to the identically-metriced Helvetica when available: many professional printers/publishers have the original Helvetica readily available in their setup. It's not clear to me what you expect and how it should work. https://codereview.appspot.com/224800043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Define default fonts in fontconfig configuration file (issue 241340043 by truer...@gmail.com)
Nice! LGTM. https://codereview.appspot.com/241340043/diff/1/lily/font-config.cc File lily/font-config.cc (right): https://codereview.appspot.com/241340043/diff/1/lily/font-config.cc#newcode60 lily/font-config.cc:60: confs.push_back (lilypond_datadir + /fonts/lilypond-fonts.conf); This line adds one .conf file, right? So... https://codereview.appspot.com/241340043/diff/1/lily/font-config.cc#newcode62 lily/font-config.cc:62: for (vsize i = 0; i confs.size (); i++) ... do we have a loop here? https://codereview.appspot.com/241340043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Define default fonts in fontconfig configuration file (issue 241340043 by truer...@gmail.com)
https://codereview.appspot.com/241340043/diff/1/lily/font-config.cc File lily/font-config.cc (right): https://codereview.appspot.com/241340043/diff/1/lily/font-config.cc#newcode62 lily/font-config.cc:62: for (vsize i = 0; i confs.size (); i++) ... do we have a loop here? I mean: *Why* do we have a loop here? https://codereview.appspot.com/241340043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Use mkstemp for PNG scale down (issue 243310043 by truer...@gmail.com)
LGTM https://codereview.appspot.com/243310043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Define default fonts in fontconfig configuration file (issue 241340043 by truer...@gmail.com)
LGTM, thanks! https://codereview.appspot.com/241340043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
freetype.cc: solve algebra puzzle for converting quadratics to cubics (issue 252810043 by d...@gnu.org)
LGTM. Funny that noone has every tried to simplify this... https://codereview.appspot.com/252810043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Issue 4571 / 4: Add font aliases settings for NR: antient.itely (issue 261980044 by truer...@gmail.com)
LGTM, thanks. https://codereview.appspot.com/261980044/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: stencil-integrate.cc: root out slopes (issue 246590043 by d...@gnu.org)
Oh, I haven't meant that you should reformat everything but just the code you are actually modifying! But thanks anyway. For long names I suggest a formatting like this Grob::maybe_pure_internal_simple_skylines_from_extents( foo arg1, bar arg2, ...) to avoid exactly the problem you mention. On the other hand, such long function names are overly descriptive IMHO. I think they don't really increase readability of the code... https://codereview.appspot.com/246590043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: stencil-integrate.cc: root out slopes (issue 246590043 by d...@gnu.org)
LGTM https://codereview.appspot.com/246590043/diff/20001/lily/stencil-integral.cc File lily/stencil-integral.cc (right): https://codereview.appspot.com/246590043/diff/20001/lily/stencil-integral.cc#newcode62 lily/stencil-integral.cc:62: void create_path_cap (vectorBox boxes, vectorDrul_arrayOffset buildings, PangoMatrix trans, Offset pt, Real rad, Offset dir); 80 characters per line, please... https://codereview.appspot.com/246590043/diff/20001/lily/stencil-integral.cc#newcode363 lily/stencil-integral.cc:363: create_path_cap (vectorBox boxes, vectorDrul_arrayOffset buildings, PangoMatrix trans, Offset pt, Real rad, Offset dir) ditto https://codereview.appspot.com/246590043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Avoid Parsed object should be dead message from issue 4505 (issue 258030043 by d...@gnu.org)
LGTM https://codereview.appspot.com/258030043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Set the sequence name in MIDI using title information from \header block (issue 256230045 by ht.lilypond.developm...@gmail.com)
I don't have enough knowledge for a LGTM, but reading the source code I noticed one detail... https://codereview.appspot.com/256230045/diff/1/lily/performance.cc File lily/performance.cc (right): https://codereview.appspot.com/256230045/diff/1/lily/performance.cc#newcode92 lily/performance.cc:92: text-text_string_ = name; This code allows overwriting `control track' only once. I guess this is intended, however, it looks limiting. Wouldn't it be better to identify the control track by another property, say Audio_text::CONTROL_TRACK_NAME, instead of a comparison with a string that the user might modify? https://codereview.appspot.com/256230045/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Let \addlyrics accept an optional context mod (issue 255610043 by d...@gnu.org)
LGTM, thanks. https://codereview.appspot.com/255610043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Fix LilyPond default fonts definition (issue 258160043 by truer...@gmail.com)
LGTM https://codereview.appspot.com/258160043/diff/1/lily/font-config.cc File lily/font-config.cc (right): https://codereview.appspot.com/258160043/diff/1/lily/font-config.cc#newcode43 lily/font-config.cc:43: /* Create an empty configureation */ s/configureation/configuration/ https://codereview.appspot.com/258160043/diff/1/lily/font-config.cc#newcode70 lily/font-config.cc:70: error (_f (failed adding fontconfig configuration file: %s, I think it's better to have failed to add fontconfig configuration file `foo' instead of failed adding fontconfig configuration file: foo https://codereview.appspot.com/258160043/diff/1/mf/99-lilypond-fonts.conf.in File mf/99-lilypond-fonts.conf.in (right): https://codereview.appspot.com/258160043/diff/1/mf/99-lilypond-fonts.conf.in#newcode1 mf/99-lilypond-fonts.conf.in:1: ?xml version=1.0 encoding=UTF-8? just curious: Why is this `99-lilypond-fonts.conf.in' and not `99-lilypond-fonts.conf'? What can be configured in this file? https://codereview.appspot.com/258160043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Add font settings for Unicode demonstration (issue 258190043 by truer...@gmail.com)
LGTM https://codereview.appspot.com/258190043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Issue 4560: group #include directives at top of file (issue 263730043 by nine.fierce.ball...@gmail.com)
This is a good idea. LGTM. https://codereview.appspot.com/263730043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add font settings for Unicode demonstration (issue 258190043 by truer...@gmail.com)
LGTM, thanks. https://codereview.appspot.com/258190043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Change the LilyPond default fonts to TeX Gyre (issue 258250043 by truer...@gmail.com)
LGTM, thanks! https://codereview.appspot.com/258250043/diff/1/tex/lilypond.map.in File tex/lilypond.map.in (left): https://codereview.appspot.com/258250043/diff/1/tex/lilypond.map.in#oldcode1 tex/lilypond.map.in:1: pncr8r CenturySchL-Roma @NCSB_DIR@/c059013l.pfb This is now an empty file... Can it be removed completely? https://codereview.appspot.com/258250043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: modify coord-rotate to get exact values for (sin PI) etc (issue 269530043 by thomasmorle...@gmail.com)
+1 https://codereview.appspot.com/269530043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Issue 4587: Add font settings to kievan-notation.ly (issue 264080043 by truer...@gmail.com)
LGTM https://codereview.appspot.com/264080043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add ly:get-font-format to get the font format (issue 296350043 by truer...@gmail.com)
My main purpose was the distinction between the TTC and OTC. So this patch cannot distinguish between the PFA and PFB. OK, but IMHO we should get rid of being dependent on the file name suffix. I'm trying the following `font-file-as-ps-string' with this Patch Set 2. Hmm, this looks like the original, unpatched code... https://codereview.appspot.com/296350043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Add ly:get-font-format to get the font format (issue 296350043 by truer...@gmail.com)
You were faster than me :-)
LGTM, but note that FreeType's `FT_Get_Font_Format` doesn't really
return the font format but the name of the module used to handle a font.
In particular, it doesn't make a difference between PFA and PFB.
While I haven't started coding yet, I've played around to write a
documentation string first. Here is what I cooked up yesterday.
LY_DEFINE (ly_font_format_for_conversion,
"ly:font-format-for-conversion",
1, 0, 0,
(SCM font_file_name),
"Return the font format of the font file as a symbol,
indicating"
" how to convert it to a resource that can be embedded into
a"
" PostScript output file as created by LilyPond. Possible
values"
" are @code{'TrueType}, @code{'PFA}, @code{'PFB},
@code{'Type42},"
" @code{'CID}, @code{'CFF}, and @code{'OTF}. If the font
format"
" is not recognized or not supported, @code{'()} gets
returned.\n"
"\n"
"Fonts tagged as @code{'TrueType} can be converted to a"
" PostScript Type@tie{}42 font resource with function"
" @code{ly:ttf->pfa}. @code{'PFB} is a binary Type@tie{}1
font"
" that must be converted to a Type@tie{}1 font resource with"
" function @code{ly:pfb->pfa}. @code{'OTF} indicates a CFF"
" contained in an SFNT wrapper; it should be converted to a
bare"
" CFF with function @code{ly:otf->cff}.\n"
"\n"
"Bare CFFs (tagged as @code{'CFF}) should be converted to an"
" embeddable resource with function @code{ps-embed-cff}.
ASCII"
" Type@tie{}1 fonts (tagged as @code{'PFA}) should be
converted"
" to embeddable resources with function
@code{embed-document}."
"\n"
"Bare CID and Type@tie{}42 font resources (tagged as
@code{'CID}"
" and @code{'Type42}, respectively) can be directly embedded"
" into the output.")
Such a function could be then used to rewrite `font-file-as-ps-string`
to avoid any dependency on the file extension.
https://codereview.appspot.com/296350043/diff/1/lily/open-type-font-scheme.cc
File lily/open-type-font-scheme.cc (right):
https://codereview.appspot.com/296350043/diff/1/lily/open-type-font-scheme.cc#newcode133
lily/open-type-font-scheme.cc:133: " returning it as a string. The
optional"
Wouldn't it be better to return a symbol instead?
https://codereview.appspot.com/296350043/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Integrate Type1 font embedding procedures (issue 293710043 by truer...@gmail.com)
LGTM. https://codereview.appspot.com/293710043/diff/1/lily/pfb-scheme.cc File lily/pfb-scheme.cc (right): https://codereview.appspot.com/293710043/diff/1/lily/pfb-scheme.cc#newcode13 lily/pfb-scheme.cc:13: " to PFA format. If the file is already in PFA format," Please two spaces after a full dot. https://codereview.appspot.com/293710043/diff/1/lily/pfb-scheme.cc#newcode35 lily/pfb-scheme.cc:35: /* The file is in PFA format. Pass through it. */ I think this should be `Pass it through.' https://codereview.appspot.com/293710043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Replace `-dgs-load-fonts' to `--bigpdfs' in lilypond-book (issue 300280043 by truer...@gmail.com)
LGTM – please update the title of this Rietveld issue https://codereview.appspot.com/300280043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Autoconf variable $(LD) should be just for C (issue 299200043 by d...@gnu.org)
LGTM https://codereview.appspot.com/299200043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Delay import of `midi' module. (issue 297420043 by lemzw...@googlemail.com)
Reviewers: dak, Message: Ok, running the script with just --version or --help does indeed not need the midi module. Yep. This is ugly as whatever but likely will do the trick. :-) Interestingly, I haven't find out yet how `midi.c' gets compiled at all; I can't find any trace of it in the log file that I catch make all &> make.all.log Can you push this right to staging Done. Description: Delay import of `midi' module. `make all' starts with generating the manual pages for lilypond's python scripts using help2man. However, at the time `midi2ly --help' gets called, the `midi' python module (which is based on code written in C) imported by `midi2ly' has not been compiled yet, making help2man abort. Please review this at https://codereview.appspot.com/297420043/ Affected files (+2, -1 lines): M scripts/midi2ly.py Index: scripts/midi2ly.py diff --git a/scripts/midi2ly.py b/scripts/midi2ly.py index f4a47fb79ab963c889a9d372b9952e89ac3cdbd9..b3600fa2e3b416e848bb62bfa5f1f8332c87de00 100644 --- a/scripts/midi2ly.py +++ b/scripts/midi2ly.py @@ -32,7 +32,6 @@ import sys @relocate-preamble@ """ -import midi import lilylib as ly global _;_=ly._ @@ -923,6 +922,8 @@ class Staff: return dump_track (self.voices, i) def convert_midi (in_file, out_file): +import midi + global clocks_per_1, clocks_per_4, key global start_quant_clocks global duration_quant_clocks ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Delay import of `midi' module. (issue 297420043 by lemzw...@googlemail.com)
Werner, do you think you can push a fixed version? Done. For me, `make test` succeeded. https://codereview.appspot.com/297420043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
