Issue 10782045: Updates to NR sections 3 and 4 (Closed)
|
Created:
10 years, 9 months ago by PhilEHolmes Modified:
10 years, 9 months ago CC:
lilypond-devel_gnu.org Visibility:
Public. |
DescriptionThere's a fair number of changes here: generally they fall into 2 camps: 1) changing paper size to make the example display properly and 2) closing up examples with too many line breaks or restated note values. There are also a few over-long lines with breaks put in them. Passes make doc. Really needs checking with the PDF NR to show the benefits.
Patch Set 1 #
Total comments: 4
Patch Set 2 : Updates following comments #
MessagesTotal messages: 13
Please review
Sign in to reply to this message.
Just one comment, otherwise LGTM Trevor https://codereview.appspot.com/10782045/diff/1/Documentation/notation/input.i... File Documentation/notation/input.itely (right): https://codereview.appspot.com/10782045/diff/1/Documentation/notation/input.i... Documentation/notation/input.itely:2198: \relative c'' { No. \relative must be applied to the music in a constant sequence, not to a sequence that might change depending on the tags.
Sign in to reply to this message.
https://codereview.appspot.com/10782045/diff/1/Documentation/notation/input.i... File Documentation/notation/input.itely (right): https://codereview.appspot.com/10782045/diff/1/Documentation/notation/input.i... Documentation/notation/input.itely:2198: \relative c'' { On 2013/06/30 07:11:23, Trevor Daniels wrote: > No. \relative must be applied to the music > in a constant sequence, not to a sequence that > might change depending on the tags. Well, it is perfectly "legal" to use \relative like this, it just is unlikely to lead to reliable results unless you put something like \resetRelativeOctave c'' after each tagged sequence. In other words, I support undoing this change. https://codereview.appspot.com/10782045/diff/1/Documentation/notation/input.i... Documentation/notation/input.itely:2535: \score { ... } Actually, it would make more sense to write @example showLastLength = R1*5 \score @{ @dots{} @} @end example There is no need to use @verbatim here.
Sign in to reply to this message.
----- Original Message ----- From: <tdanielsmusic@googlemail.com> To: <PhilEHolmes@googlemail.com> Cc: <reply@codereview-hr.appspotmail.com>; <lilypond-devel@gnu.org> Sent: Sunday, June 30, 2013 8:11 AM Subject: Re: Updates to NR sections 3 and 4 (issue 10782045) > Just one comment, otherwise LGTM > > Trevor > > > > https://codereview.appspot.com/10782045/diff/1/Documentation/notation/input.i... > File Documentation/notation/input.itely (right): > > https://codereview.appspot.com/10782045/diff/1/Documentation/notation/input.i... > Documentation/notation/input.itely:2198: \relative c'' { > No. \relative must be applied to the music > in a constant sequence, not to a sequence that > might change depending on the tags. > > https://codereview.appspot.com/10782045/ I think it might actually be worth a comment to that effect with this example? "Note where the \relative statement is placed..." -- Phil Holmes
Sign in to reply to this message.
"Phil Holmes" <mail@philholmes.net> writes: > ----- Original Message ----- > From: <tdanielsmusic@googlemail.com> > To: <PhilEHolmes@googlemail.com> > Cc: <reply@codereview-hr.appspotmail.com>; <lilypond-devel@gnu.org> > Sent: Sunday, June 30, 2013 8:11 AM > Subject: Re: Updates to NR sections 3 and 4 (issue 10782045) > > >> Just one comment, otherwise LGTM >> >> Trevor >> >> >> >> https://codereview.appspot.com/10782045/diff/1/Documentation/notation/input.i... >> File Documentation/notation/input.itely (right): >> >> https://codereview.appspot.com/10782045/diff/1/Documentation/notation/input.i... >> Documentation/notation/input.itely:2198: \relative c'' { >> No. \relative must be applied to the music >> in a constant sequence, not to a sequence that >> might change depending on the tags. >> >> https://codereview.appspot.com/10782045/ > > > I think it might actually be worth a comment to that effect with this > example? "Note where the \relative statement is placed..." Don't think so. Notation manual is a reference manual. Information like that belongs to \relative. It would be a different matter in the Learning Manual. -- David Kastrup
Sign in to reply to this message.
----- Original Message ----- From: "David Kastrup" <dak@gnu.org> To: "Phil Holmes" <mail@philholmes.net> Cc: <PhilEHolmes@googlemail.com>; <tdanielsmusic@googlemail.com>; <lilypond-devel@gnu.org>; <reply@codereview-hr.appspotmail.com> Sent: Sunday, June 30, 2013 11:23 AM Subject: Re: Updates to NR sections 3 and 4 (issue 10782045) > "Phil Holmes" <mail@philholmes.net> writes: > >> ----- Original Message ----- >> From: <tdanielsmusic@googlemail.com> >> To: <PhilEHolmes@googlemail.com> >> Cc: <reply@codereview-hr.appspotmail.com>; <lilypond-devel@gnu.org> >> Sent: Sunday, June 30, 2013 8:11 AM >> Subject: Re: Updates to NR sections 3 and 4 (issue 10782045) >> >> >>> Just one comment, otherwise LGTM >>> >>> Trevor >>> >>> >>> >>> https://codereview.appspot.com/10782045/diff/1/Documentation/notation/input.i... >>> File Documentation/notation/input.itely (right): >>> >>> https://codereview.appspot.com/10782045/diff/1/Documentation/notation/input.i... >>> Documentation/notation/input.itely:2198: \relative c'' { >>> No. \relative must be applied to the music >>> in a constant sequence, not to a sequence that >>> might change depending on the tags. >>> >>> https://codereview.appspot.com/10782045/ >> >> >> I think it might actually be worth a comment to that effect with this >> example? "Note where the \relative statement is placed..." > > Don't think so. Notation manual is a reference manual. Information > like that belongs to \relative. It would be a different matter in the > Learning Manual. Seems relevant to tagging, too? -- Phil Holmes
Sign in to reply to this message.
"Phil Holmes" <email@philholmes.net> writes: >>>> https://codereview.appspot.com/10782045/ >>> >>> I think it might actually be worth a comment to that effect with this >>> example? "Note where the \relative statement is placed..." >> >> Don't think so. Notation manual is a reference manual. Information >> like that belongs to \relative. It would be a different matter in the >> Learning Manual. > > > Seems relevant to tagging, too? That's why the section with tagging already contains Known issues and warnings Calling \relative on a music expression obtained by filtering music through \keepWithTag or \removeWithTag might cause the octave relations to change, as only the pitches actually remaining in the filtered expression will be considered. Applying \relative first, before \keepWithTag or \removeWithTag, avoids this danger as \relative then acts on all the pitches as-input. -- David Kastrup
Sign in to reply to this message.
----- Original Message ----- From: "David Kastrup" <dak@gnu.org> To: "Phil Holmes" <email@philholmes.net> Cc: <lilypond-devel@gnu.org>; <reply@codereview-hr.appspotmail.com>; <PhilEHolmes@googlemail.com>; <tdanielsmusic@googlemail.com> Sent: Sunday, June 30, 2013 11:30 AM Subject: Re: Updates to NR sections 3 and 4 (issue 10782045) > "Phil Holmes" <email@philholmes.net> writes: > >>>>> https://codereview.appspot.com/10782045/ >>>> >>>> I think it might actually be worth a comment to that effect with this >>>> example? "Note where the \relative statement is placed..." >>> >>> Don't think so. Notation manual is a reference manual. Information >>> like that belongs to \relative. It would be a different matter in the >>> Learning Manual. >> >> >> Seems relevant to tagging, too? > > That's why the section with tagging already contains > > Known issues and warnings > > Calling \relative on a music expression obtained by filtering music > through \keepWithTag or \removeWithTag might cause the octave relations > to change, as only the pitches actually remaining in the filtered > expression will be considered. Applying \relative first, before > \keepWithTag or \removeWithTag, avoids this danger as \relative then > acts on all the pitches as-input. It's a fair cop. -- Phil Holmes
Sign in to reply to this message.
https://codereview.appspot.com/10782045/diff/1/Documentation/notation/input.i... File Documentation/notation/input.itely (right): https://codereview.appspot.com/10782045/diff/1/Documentation/notation/input.i... Documentation/notation/input.itely:921: opus = \markup { \italic "Op. 13" } Change "Op.13" to "BWV 846", so it's consistent with the rest of the examples.
Sign in to reply to this message.
Updates following comments
Sign in to reply to this message.
----- Original Message ----- From: <tdanielsmusic@googlemail.com> To: <PhilEHolmes@googlemail.com> Cc: <lilypond-devel@gnu.org>; <reply@codereview-hr.appspotmail.com> Sent: Sunday, June 30, 2013 8:11 AM Subject: Re: Updates to NR sections 3 and 4 (issue 10782045) > Just one comment, otherwise LGTM > > Trevor > > > > https://codereview.appspot.com/10782045/diff/1/Documentation/notation/input.i... > File Documentation/notation/input.itely (right): > > https://codereview.appspot.com/10782045/diff/1/Documentation/notation/input.i... > Documentation/notation/input.itely:2198: \relative c'' { > No. \relative must be applied to the music > in a constant sequence, not to a sequence that > might change depending on the tags. > > https://codereview.appspot.com/10782045/ Updated to add a \new Voice to avoid the problem shown in http://lilypond.org/doc/v2.17/Documentation/usage/common-errors -- Phil Holmes
Sign in to reply to this message.
LGTM Trevor
Sign in to reply to this message.
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
