Issue 3732046: Documentation of Dynamics context and postfix dynamics changes (Closed)
|
Created:
13 years, 4 months ago by Carl Modified:
13 years, 4 months ago CC:
lilypond-devel_gnu.org Visibility:
Public. |
DescriptionDocumentation of Dynamics context and postfix dynamics changes
Patch Set 1 #
Total comments: 18
MessagesTotal messages: 11
Here are Keith's suggested doc changes for the Dynamics context and postfix (de)crescendo notation.
Sign in to reply to this message.
Looks mostly good to me -- one styling nit. Thanks! Carl http://codereview.appspot.com/3732046/diff/1/Documentation/notation/expressiv... File Documentation/notation/expressive.itely (right): http://codereview.appspot.com/3732046/diff/1/Documentation/notation/expressiv... Documentation/notation/expressive.itely:313: \bar "||" e2\p\< f | e2\> d\! | a2\< b | c1\decr s1*0\! \bar "|." I don't like having all this on one line. Is there a reason for it?
Sign in to reply to this message.
Tell me if it is trivial to load patches from lily-git onto Reitveld. The Contributors Guide implies that the lily-git patches are intended for emailing to regular git-users. http://codereview.appspot.com/3732046/diff/1/Documentation/notation/expressiv... File Documentation/notation/expressive.itely (right): http://codereview.appspot.com/3732046/diff/1/Documentation/notation/expressiv... Documentation/notation/expressive.itely:313: \bar "||" e2\p\< f | e2\> d\! | a2\< b | c1\decr s1*0\! \bar "|." On 2010/12/20 05:59:46, Carl wrote: > I don't like having all this on one line. Is there a reason for it? So that the reader can keep his place when shifting gaze from the LilyPond input to the engraved output.
Sign in to reply to this message.
Keith Some of your new material is not in the agreed style the NR, in particular your attempts to make the examples prettier by inserting \bar commands. This should not be done without wider agreement to insert them in all the examples - a change I would oppose as I believe the code in the examples should be as simple as possible. Also the style of writing LilyPond code should follow the rest of the NR, in particular in the use of newlines. Otherwise the changes look fine. Trevor http://codereview.appspot.com/3732046/diff/1/Documentation/notation/expressiv... File Documentation/notation/expressive.itely (right): http://codereview.appspot.com/3732046/diff/1/Documentation/notation/expressiv... Documentation/notation/expressive.itely:313: \bar "||" e2\p\< f | e2\> d\! | a2\< b | c1\decr s1*0\! \bar "|." Although it looks prettier with inserted \bar commands these are unnecessary to illustrate the point and are inconsistent with the rest of the document. They should all be removed. Start a new line in the music code after each hairpin is terminated. Insert bar checks consistently. http://codereview.appspot.com/3732046/diff/1/Documentation/notation/expressiv... Documentation/notation/expressive.itely:319: or @code{\!} or its synonym @{\endcresc}. @code{\endcresc} http://codereview.appspot.com/3732046/diff/1/Documentation/notation/expressiv... Documentation/notation/expressive.itely:324: \bar "||" drop \bar http://codereview.appspot.com/3732046/diff/1/Documentation/notation/expressiv... Documentation/notation/expressive.itely:352: \dimHairpin c2\< d\! | e\> d\! \bar "||" drop \bar's start music on newlines I'd also prefer to see the full set of \dimText's illustrated
Sign in to reply to this message.
Missed this earlier http://codereview.appspot.com/3732046/diff/1/Documentation/notation/expressiv... File Documentation/notation/expressive.itely (right): http://codereview.appspot.com/3732046/diff/1/Documentation/notation/expressiv... Documentation/notation/expressive.itely:390: in @rlearning{Moving-objects}. Broken ref
Sign in to reply to this message.
On 2010/12/20 06:44:18, Keith wrote: > Tell me if it is trivial to load patches from lily-git onto Reitveld. The > Contributors Guide implies that the lily-git patches are intended for emailing > to regular git-users. > It is not too hard, but not trivial, to load patches from lily-git onto Rietveld. See CG 2.3.4 Uploading patches for review. You will need to download git-cl. After creating a patch in lily-git, you can upload it to Rietveld by issuing the command git-cl upload HEAD^ In general, to do this effectively long-term requires more knowledge of git than lily-git generally develops, so we don't ask new contributors to do it. My role as Frog Meister requires me to shepherd patches by new contributors. That's why I posted your patch on Rietveld for comments. Once you've responded to the comments, send another patch and I'll upload it. BTW, because you have two different patches, it may be somewhat difficult from here on out using lily-git. lily-git really only works well for one patch at a time.
Sign in to reply to this message.
http://codereview.appspot.com/3732046/diff/1/Documentation/notation/expressiv... File Documentation/notation/expressive.itely (right): http://codereview.appspot.com/3732046/diff/1/Documentation/notation/expressiv... Documentation/notation/expressive.itely:313: \bar "||" e2\p\< f | e2\> d\! | a2\< b | c1\decr s1*0\! \bar "|." On 2010/12/20 06:44:18, Keith wrote: > On 2010/12/20 05:59:46, Carl wrote: > > I don't like having all this on one line. Is there a reason for it? > > So that the reader can keep his place when shifting gaze from the LilyPond input > to the engraved output. I think it's far easier to keep place when the line is broken up as Trevor requests. We need *logical* grouping in the lilypond input files. Trying to group the input files according to lilypond output is a nightmare long-term.
Sign in to reply to this message.
On Mon, Dec 20, 2010 at 06:44:18AM +0000, k-ohara5a5a@oco.net wrote: > Tell me if it is trivial to load patches from lily-git onto Reitveld. It is not trivial. Yet. (my answer may change in a few days) > The Contributors Guide implies that the lily-git patches are intended > for emailing to regular git-users. That is correct; this is our current policy. (my answer may change in a few days) Cheers, - Graham
Sign in to reply to this message.
Quite nice for an initial patch. For future reference, I would recommend working on slightly smaller patches -- this would let us go through more rounds of draft+comment faster, without you having to change so many examples when we clarify bits of the doc policy. http://codereview.appspot.com/3732046/diff/1/Documentation/notation/expressiv... File Documentation/notation/expressive.itely (right): http://codereview.appspot.com/3732046/diff/1/Documentation/notation/expressiv... Documentation/notation/expressive.itely:313: \bar "||" e2\p\< f | e2\> d\! | a2\< b | c1\decr s1*0\! \bar "|." On 2010/12/20 15:22:30, Carl wrote: > I think it's far easier to keep place when the line is broken up as Trevor > requests. We need *logical* grouping in the lilypond input files. Trying to > group the input files according to lilypond output is a nightmare long-term. Yes. That said, we have sometimes used \break to get the desired output. For example, you could have 4 bars, then a \break then another 4 bars. If you do this, the \break should be on its own line, and make sure that you don't try to pack a lot of notation onto one line. http://codereview.appspot.com/3732046/diff/1/Documentation/notation/expressiv... Documentation/notation/expressive.itely:319: or @code{\!} or its synonym @{\endcresc}. On 2010/12/20 10:57:44, Trevor Daniels wrote: > @code{\endcresc} Yes. I'm surprised that the code compiled with this error! (and if you didn't check that it compiled before sending the patch, then consider yourself lucky that I'm giving you the benefit of the doubt here. growl.) http://codereview.appspot.com/3732046/diff/1/Documentation/notation/expressiv... Documentation/notation/expressive.itely:324: \bar "||" On 2010/12/20 10:57:44, Trevor Daniels wrote: > drop \bar I'll accept a \break instead, if Keith insists. http://codereview.appspot.com/3732046/diff/1/Documentation/notation/expressiv... Documentation/notation/expressive.itely:341: The commands @code{\crescTextCresc}, @code{\dimTextDecresc}, Holy talking through the code, batman! Delete this whole paragraph, and replace with " Textual marks can be produced instead of hairpins: " I'm willing to discuss this point, but I really don't see any benefit to the verbosity. The example is clear. Trust your example. re the example: what about using 4 durs instead of 2 ? That way, each line would be one bar. Easier to keep track of. Maybe divide the example into two separate lines with \break.
Sign in to reply to this message.
The patch looks bigger than it is, due to my moving a block. I explain in reply-comments below how I am incorporating the suggestions. You deduced correctly that I sinned in labeling something a PATCH that didn't first get a build test. (I dual-boot and only connect through windows, causing some impatience that leads to makes.) Minor delay while I straighten that out before another mistake. http://codereview.appspot.com/3732046/diff/1/Documentation/notation/expressiv... File Documentation/notation/expressive.itely (right): http://codereview.appspot.com/3732046/diff/1/Documentation/notation/expressiv... Documentation/notation/expressive.itely:313: \bar "||" e2\p\< f | e2\> d\! | a2\< b | c1\decr s1*0\! \bar "|." On 2010/12/20 18:53:19, Graham Percival wrote: > That said, we have sometimes used \break I have difficulty keeping my place in the input when it gets this long vertically, in contrast to Carl. Probably too much detail here for one @lilypond to convey to the variety of human readers. I'll revert this example, and convert my long line to a style-compliant separate @lilypond, introduced by "The horizontal extent of a hairpin reflects the musical moments on which it begins and ends." The purpose of this example is to demonstrate the information that James pulled in from the hairpin-length reg-test, and the two-dynamics syntax c4\p\<, without making the section too long. http://codereview.appspot.com/3732046/diff/1/Documentation/notation/expressiv... Documentation/notation/expressive.itely:324: \bar "||" I'll put this in standard style http://codereview.appspot.com/3732046/diff/1/Documentation/notation/expressiv... Documentation/notation/expressive.itely:341: The commands @code{\crescTextCresc}, @code{\dimTextDecresc}, On 2010/12/20 18:53:19, Graham Percival wrote: > Delete this whole paragraph, and replace with Amen, brother! This was a moved block, and I was timid in my editing. http://codereview.appspot.com/3732046/diff/1/Documentation/notation/expressiv... Documentation/notation/expressive.itely:352: \dimHairpin c2\< d\! | e\> d\! \bar "||" On 2010/12/20 10:57:44, Trevor Daniels wrote: > drop \bar's [...] full set of \dimTexts Okay. Starting again from the old example, Graham's suggestion to use crotchets gives enough room for all the texts. http://codereview.appspot.com/3732046/diff/1/Documentation/notation/expressiv... Documentation/notation/expressive.itely:390: in @rlearning{Moving-objects}. On 2010/12/20 12:54:37, Trevor Daniels wrote: > Broken ref Working on it. Also fixing "can by" -> "can be" http://codereview.appspot.com/3732046/diff/1/Documentation/notation/text.itely File Documentation/notation/text.itely (right): http://codereview.appspot.com/3732046/diff/1/Documentation/notation/text.itel... Documentation/notation/text.itely:170: Text spanners are part of the @code{Dynamics} context; see I should have deleted this sentence, as the Dynamic context is now mentioned in the section on dynamics, and text spanners can live in several contexts.
Sign in to reply to this message.
I posted a revised patch under my own login, directly from the system that compiled it,at http://codereview.appspot.com/3743045/ Carl, the C G was clear enough that I had no trouble going straight to Rietveld, but I can send (or you can grab) the revised patch to post under this issue# if you prefer. The revision changes less than this one did, but it gets the job done and complies, I'm pretty sure, with doc style.
Sign in to reply to this message.
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
