Issue 6303095: Documentation of Kievan notation (Closed)
|
Created:
12 years, 10 months ago by aleksandr.andreev Modified:
12 years, 10 months ago CC:
lilypond-devel_gnu.org Base URL:
http://git.savannah.gnu.org/gitweb/?p=lilypond.git/trunk/ Visibility:
Public. |
DescriptionDocumentation of Kievan notation
Adds a subsection to the Ancient notation section of the Notation reference documenting support of Kievan square notation.
Issue 2317.
Patch Set 1 #
Total comments: 17
Patch Set 2 : Updating Kievan documentation #
Total comments: 2
Patch Set 3 : Removing 'fragment' from ancient.itely #MessagesTotal messages: 13
Please review.
Sign in to reply to this message.
LGTM, minor style issues. http://codereview.appspot.com/6303095/diff/1/Documentation/music-glossary.tely File Documentation/music-glossary.tely (right): http://codereview.appspot.com/6303095/diff/1/Documentation/music-glossary.tel... Documentation/music-glossary.tely:4539: jurisdictions of Orthodoxy and Byzantine-rite Catholicism. It is There should be two spaces after period that closes sentence (here and in some places in the second file). http://codereview.appspot.com/6303095/diff/1/Documentation/notation/ancient.i... File Documentation/notation/ancient.itely (right): http://codereview.appspot.com/6303095/diff/1/Documentation/notation/ancient.i... Documentation/notation/ancient.itely:2464: There is only one clef used in Kievan notation (the Tse-fa-ut Clef). It is used to indicate the position of @code{c}, as in the following example: Please split this line. http://codereview.appspot.com/6303095/diff/1/Documentation/notation/ancient.i... Documentation/notation/ancient.itely:2530: \markup { Should this be included as a lilypond example? http://codereview.appspot.com/6303095/diff/1/Documentation/notation/ancient.i... Documentation/notation/ancient.itely:2562: @lilypond[quote,ragged-right,staffsize=24] similar to above - i don't think people should use it this way? (i.e. via \musicglyph)
Sign in to reply to this message.
This will be an excellent addition to the NR. We don't enforce house style quite as rigorously in Chapter 2 as we do in Chapter 1. But one, which is to use working examples of code and to let those examples speak for themselves, is worth adopting here. I've shown what I mean towards the end, in the paragraphs about accidentals, but there may be other places where a similar change might improve clarity. http://codereview.appspot.com/6303095/diff/1/Documentation/notation/ancient.i... File Documentation/notation/ancient.itely (right): http://codereview.appspot.com/6303095/diff/1/Documentation/notation/ancient.i... Documentation/notation/ancient.itely:41: Gregorian notation. "... contexts for these styles." maybe. http://codereview.appspot.com/6303095/diff/1/Documentation/notation/ancient.i... Documentation/notation/ancient.itely:2431: as the following excerpt demonstrates: Stop after the comma, and change that to a colon. http://codereview.appspot.com/6303095/diff/1/Documentation/notation/ancient.i... Documentation/notation/ancient.itely:2464: There is only one clef used in Kievan notation (the Tse-fa-ut Clef). It is used to indicate the position of @code{c}, as in the following example: Stop after the comma, and change that to a colon. http://codereview.appspot.com/6303095/diff/1/Documentation/notation/ancient.i... Documentation/notation/ancient.itely:2537: @end lilypond Rather than displaying the glyphs (which already appear in Appendix 8 and should be referenced there) it would be better to move the following para about accidental style before this one and then show the use of accidentals in context in a bar of music. http://codereview.appspot.com/6303095/diff/1/Documentation/notation/ancient.i... Documentation/notation/ancient.itely:2541: e.g.: "The @code{kievan} style for accidentals is selected with" and use @example for the code. http://codereview.appspot.com/6303095/diff/1/Documentation/notation/ancient.i... Documentation/notation/ancient.itely:2566: @end lilypond Could this example include a bar of music to show how the two fit together?
Sign in to reply to this message.
Updated patch based on comments by Janek and Trevor. http://codereview.appspot.com/6303095/diff/1/Documentation/music-glossary.tely File Documentation/music-glossary.tely (right): http://codereview.appspot.com/6303095/diff/1/Documentation/music-glossary.tel... Documentation/music-glossary.tely:4539: jurisdictions of Orthodoxy and Byzantine-rite Catholicism. It is On 2012/06/18 07:41:55, janek wrote: > There should be two spaces after period that closes sentence (here and in some > places in the second file). Done. http://codereview.appspot.com/6303095/diff/1/Documentation/notation/ancient.i... File Documentation/notation/ancient.itely (right): http://codereview.appspot.com/6303095/diff/1/Documentation/notation/ancient.i... Documentation/notation/ancient.itely:41: Gregorian notation. On 2012/06/18 08:06:15, Trevor Daniels wrote: > "... contexts for these styles." maybe. Done. http://codereview.appspot.com/6303095/diff/1/Documentation/notation/ancient.i... Documentation/notation/ancient.itely:2431: as the following excerpt demonstrates: On 2012/06/18 08:06:15, Trevor Daniels wrote: > Stop after the comma, and change that to a colon. Done. http://codereview.appspot.com/6303095/diff/1/Documentation/notation/ancient.i... Documentation/notation/ancient.itely:2464: There is only one clef used in Kievan notation (the Tse-fa-ut Clef). It is used to indicate the position of @code{c}, as in the following example: On 2012/06/18 08:06:15, Trevor Daniels wrote: > Stop after the comma, and change that to a colon. Done. http://codereview.appspot.com/6303095/diff/1/Documentation/notation/ancient.i... Documentation/notation/ancient.itely:2537: @end lilypond On 2012/06/18 08:06:15, Trevor Daniels wrote: > Rather than displaying the glyphs (which already appear in Appendix 8 and should > be referenced there) it would be better to move the following para about > accidental style before this one and then show the use of accidentals in context > in a bar of music. Done. http://codereview.appspot.com/6303095/diff/1/Documentation/notation/ancient.i... Documentation/notation/ancient.itely:2566: @end lilypond On 2012/06/18 08:06:15, Trevor Daniels wrote: > Could this example include a bar of music to show how the two fit together? Maybe the last two examples should be merged? I'm not sure the Kievan bar line deserves a subsection all to itself. What do you think?
Sign in to reply to this message.
LGTM http://codereview.appspot.com/6303095/diff/1/Documentation/notation/ancient.i... File Documentation/notation/ancient.itely (right): http://codereview.appspot.com/6303095/diff/1/Documentation/notation/ancient.i... Documentation/notation/ancient.itely:2566: @end lilypond On 2012/06/19 00:07:13, aleksandr.andreev wrote: > On 2012/06/18 08:06:15, Trevor Daniels wrote: > > Could this example include a bar of music to show how the two fit together? > > Maybe the last two examples should be merged? I'm not sure the Kievan bar line > deserves a subsection all to itself. What do you think? I don't have any opinion here.
Sign in to reply to this message.
LGTM I'm happy with the last two examples separated, as you have them. Thanks! Trevor
Sign in to reply to this message.
LGTM given that it's in NR 2, in particular Ancient, since as Trevor noted we aren't as strict about "house style" and Ancient in particular wasn't updated to follow that style during the grand documentation project. http://codereview.appspot.com/6303095/diff/4/Documentation/notation/ancient.i... File Documentation/notation/ancient.itely (right): http://codereview.appspot.com/6303095/diff/4/Documentation/notation/ancient.i... Documentation/notation/ancient.itely:2530: @lilypond[quote,fragment,relative=1,notime,verbatim] IIRC [fragment] has no meaning if there's also a [relative=1] in there.
Sign in to reply to this message.
http://codereview.appspot.com/6303095/diff/4/Documentation/notation/ancient.i... File Documentation/notation/ancient.itely (right): http://codereview.appspot.com/6303095/diff/4/Documentation/notation/ancient.i... Documentation/notation/ancient.itely:2530: @lilypond[quote,fragment,relative=1,notime,verbatim] On 2012/06/20 02:35:23, Graham Percival wrote: > IIRC [fragment] has no meaning if there's also a [relative=1] in there. It seems to be used that way throughout the ancient.itely file. However, if someone can confirm that it's not necessary, I'd be happy to fix it throughout the file as part of this patch.
Sign in to reply to this message.
On 2012/06/20 03:04:21, aleksandr.andreev wrote: > It seems to be used that way throughout the ancient.itely file. > However, if someone can confirm that it's not necessary, I'd be > happy to fix it throughout the file as part of this patch. "fragment" is no longer necessary. Please do fix it throughout the file. It can be part of this patch set for reviewing, but keep it as a separate commit when it is pushed. Trevor
Sign in to reply to this message.
Per comments by Graham and Trevor, I have removed 'fragment' throughout ancient.itely where 'relative' is also used.
Sign in to reply to this message.
pushed as a10311ff02578de9f979dc6ad83ba9535f8e4e4c. Aleksandr, please close this Rietveld issue. thanks!
Sign in to reply to this message.
On 2012/06/23 05:37:05, janek wrote: > pushed as a10311ff02578de9f979dc6ad83ba9535f8e4e4c. > > Aleksandr, please close this Rietveld issue. > thanks! Janek, I was going to have the removal of 'fragment' reviewed in this issue, per Trevor's recommendation. Or shall I open a separate issue for that?
Sign in to reply to this message.
On Sat, Jun 23, 2012 at 3:19 PM, <aleksandr.andreev@gmail.com> wrote: > I was going to have the removal of 'fragment' reviewed in this issue, > per Trevor's recommendation. Or shall I open a separate issue for that? It would make sense to use the same Rietveld issue for reviewing that if both changes were pushed at the same time. But as the documentation change was pushed already, i suggest to create another issue for greater clarity. cheers, Janek
Sign in to reply to this message.
|
