Issue 3705042: Doc: More NR and LM additions based on user emails (Closed)
|
Created:
14 years, 3 months ago by pkx166h Modified:
14 years, 2 months ago CC:
lilypond-devel_gnu.org Visibility:
Public. |
DescriptionDoc: More NR and LM additions based on user emails
http://lists.gnu.org/archive/html/lilypond-user/2010-12/msg00067.html
Repeats.itely (NR)
Added additional information to @warning about not placing anything outside of the braced groups within an \alternative block. Original comments for this patch to this file
were in http://codereview.appspot.com/3581041/patch/1/2. I have made
some adjustments to the original suggestion incorporating comments
but also adding bar checks to the examples in this specific section
to help illustrate what this statememt means rather than having
to create an @example that doesn't work in the doc.
http://lists.gnu.org/archive/html/lilypond-user/2010-08/msg00372.html
Fundamental.itely (LM)
Added @knownissue about nothing being printed if the notehead engraver
is turned off, even though the expected behaviour is that beams and
stems would still be printed.
Patch Set 1 #
Total comments: 16
Patch Set 2 : Doc: More NR and LM additions based on user emails - with corrections #
Total comments: 2
Patch Set 3 : More Amendments #
Total comments: 1
Patch Set 4 : Final amendments (based on TD's comments) #
MessagesTotal messages: 15
Hello, This is another patch following on from http://codereview.appspot.com/3581041/ James
Sign in to reply to this message.
http://codereview.appspot.com/3705042/diff/1/Documentation/learning/fundament... File Documentation/learning/fundamental.itely (right): http://codereview.appspot.com/3705042/diff/1/Documentation/learning/fundament... Documentation/learning/fundamental.itely:2114: objects to note heads, if you remove the @code{Note_heads_engraver} I'm not convinced that the comma is grammatical. How do you feel about semicolons? http://codereview.appspot.com/3705042/diff/1/Documentation/notation/repeats.i... File Documentation/notation/repeats.itely (right): http://codereview.appspot.com/3705042/diff/1/Documentation/notation/repeats.i... Documentation/notation/repeats.itely:150: @rprogram{An extra staff appears}. Do not place bar checks outside Could this be in a separate @warning? I'd rather not have two totally distinct warnings in the same box. I also think it's a bit wordier than it needs to be -- I really want to keep warnings short, otherwise they lose their power. How about "Bar checks must be placed @emph{inside} the braces of each alternate block." ? ok, that's not quite the same thing (it says that you *must* use bar checks)... but please try to think of a reformulation along those lines.
Sign in to reply to this message.
Thanks, James! Carl http://codereview.appspot.com/3705042/diff/1/Documentation/learning/fundament... File Documentation/learning/fundamental.itely (right): http://codereview.appspot.com/3705042/diff/1/Documentation/learning/fundament... Documentation/learning/fundamental.itely:2114: objects to note heads, if you remove the @code{Note_heads_engraver} I agree with Graham. You either need to change the comma to a semicolon, or add a conjunction like "so". I think it would actually be best to put in a period and make it two sentences. I also think it would be better to reword it as "If the @code{Note_heads_engraver} is removed" http://codereview.appspot.com/3705042/diff/1/Documentation/notation/repeats.i... File Documentation/notation/repeats.itely (right): http://codereview.appspot.com/3705042/diff/1/Documentation/notation/repeats.i... Documentation/notation/repeats.itely:151: of the braces of the grouped notes within an @code{@bs{}alternative} block, otherwise you will not get the expected number of endings. Any bar checks used inside @code{@bs{}alternative} block must be placed in inside the braces of each group of notes.} I think that with the addition of the bar checks to the examples, this warning can be eliminated. The only reason it was needed was because the example we had previously had no bar checks, so a user guessed on where to put them and guessed wrong.
Sign in to reply to this message.
http://codereview.appspot.com/3705042/diff/1/Documentation/notation/repeats.i... File Documentation/notation/repeats.itely (right): http://codereview.appspot.com/3705042/diff/1/Documentation/notation/repeats.i... Documentation/notation/repeats.itely:150: @rprogram{An extra staff appears}. Do not place bar checks outside Hopefully this is better. I have also moved this warning because there was another place it could go, (the first warning couldn't move anywhere really) and I thought that having two warnings one after another looked odd. http://codereview.appspot.com/3705042/diff/1/Documentation/notation/repeats.i... Documentation/notation/repeats.itely:151: of the braces of the grouped notes within an @code{@bs{}alternative} block, otherwise you will not get the expected number of endings. Any bar checks used inside @code{@bs{}alternative} block must be placed in inside the braces of each group of notes.} No I think it is a good warning and would rather this is left in, I have modified the wording as per Graham's suggestion.
Sign in to reply to this message.
Fine to push, but one last try on the barcheck warning. Thanks, Carl http://codereview.appspot.com/3705042/diff/1/Documentation/notation/repeats.i... File Documentation/notation/repeats.itely (right): http://codereview.appspot.com/3705042/diff/1/Documentation/notation/repeats.i... Documentation/notation/repeats.itely:151: of the braces of the grouped notes within an @code{@bs{}alternative} block, otherwise you will not get the expected number of endings. Any bar checks used inside @code{@bs{}alternative} block must be placed in inside the braces of each group of notes.} Why a warning for barchecks, and not for markups or individual notes? I still think this is logically inconsistent. But I will approve of pushing it in the revised state if you disagree with me.
Sign in to reply to this message.
Like Carl, I'm happy to approve this as it is, but I'd prefer to see a comprehensive and correct explanation of the effect of inserting code between alternatives. http://codereview.appspot.com/3705042/diff/1/Documentation/learning/fundament... File Documentation/learning/fundamental.itely (right): http://codereview.appspot.com/3705042/diff/1/Documentation/learning/fundament... Documentation/learning/fundamental.itely:2114: objects to note heads, if you remove the @code{Note_heads_engraver} Like Carl, I prefer two sentences. http://codereview.appspot.com/3705042/diff/1/Documentation/notation/repeats.i... File Documentation/notation/repeats.itely (right): http://codereview.appspot.com/3705042/diff/1/Documentation/notation/repeats.i... Documentation/notation/repeats.itely:151: of the braces of the grouped notes within an @code{@bs{}alternative} block, otherwise you will not get the expected number of endings. Any bar checks used inside @code{@bs{}alternative} block must be placed in inside the braces of each group of notes.} Why is this all on one line? I agree with Graham - it's too long. Use his words with "should" instead of "must". http://codereview.appspot.com/3705042/diff/8001/Documentation/notation/repeat... Documentation/notation/repeats.itely:239: @cindex repeats with ties I agree with Carl. If there is nothing special about bar checks would it be correct to say, "if there are two or more alternatives nothing should appear between the closing brace of one and the opening brace of the next, or ..."
Sign in to reply to this message.
Thanks for the input. Corrections are done. http://codereview.appspot.com/3705042/diff/1/Documentation/learning/fundament... File Documentation/learning/fundamental.itely (right): http://codereview.appspot.com/3705042/diff/1/Documentation/learning/fundament... Documentation/learning/fundamental.itely:2114: objects to note heads, if you remove the @code{Note_heads_engraver} On 2010/12/17 14:21:03, Carl wrote: > I agree with Graham. You either need to change the comma to a semicolon, or add > a conjunction like "so". I think it would actually be best to put in a period > and make it two sentences. > > I also think it would be better to reword it as > > "If the @code{Note_heads_engraver} is removed" Done. http://codereview.appspot.com/3705042/diff/1/Documentation/learning/fundament... Documentation/learning/fundamental.itely:2114: objects to note heads, if you remove the @code{Note_heads_engraver} On 2010/12/17 14:21:03, Carl wrote: > I agree with Graham. You either need to change the comma to a semicolon, or add > a conjunction like "so". I think it would actually be best to put in a period > and make it two sentences. > > I also think it would be better to reword it as > > "If the @code{Note_heads_engraver} is removed" Done. http://codereview.appspot.com/3705042/diff/1/Documentation/learning/fundament... Documentation/learning/fundamental.itely:2114: objects to note heads, if you remove the @code{Note_heads_engraver} On 2010/12/17 21:02:21, Trevor Daniels wrote: > Like Carl, I prefer two sentences. Done. http://codereview.appspot.com/3705042/diff/1/Documentation/learning/fundament... Documentation/learning/fundamental.itely:2114: objects to note heads, if you remove the @code{Note_heads_engraver} On 2010/12/17 14:21:03, Carl wrote: > I agree with Graham. You either need to change the comma to a semicolon, or add > a conjunction like "so". I think it would actually be best to put in a period > and make it two sentences. > > I also think it would be better to reword it as > > "If the @code{Note_heads_engraver} is removed" Done. http://codereview.appspot.com/3705042/diff/1/Documentation/notation/repeats.i... File Documentation/notation/repeats.itely (right): http://codereview.appspot.com/3705042/diff/1/Documentation/notation/repeats.i... Documentation/notation/repeats.itely:150: @rprogram{An extra staff appears}. Do not place bar checks outside On 2010/12/17 14:05:27, Graham Percival wrote: > Could this be in a separate @warning? I'd rather not have two totally distinct > warnings in the same box. > > I also think it's a bit wordier than it needs to be -- I really want to keep > warnings short, otherwise they lose their power. How about > "Bar checks must be placed @emph{inside} the braces of each alternate block." ? > ok, that's not quite the same thing (it says that you *must* use bar checks)... > but please try to think of a reformulation along those lines. Done. http://codereview.appspot.com/3705042/diff/1/Documentation/notation/repeats.i... Documentation/notation/repeats.itely:151: of the braces of the grouped notes within an @code{@bs{}alternative} block, otherwise you will not get the expected number of endings. Any bar checks used inside @code{@bs{}alternative} block must be placed in inside the braces of each group of notes.} On 2010/12/17 21:02:21, Trevor Daniels wrote: > Why is this all on one line? > > I agree with Graham - it's too long. Use his words with "should" instead of > "must". Done. http://codereview.appspot.com/3705042/diff/1/Documentation/notation/repeats.i... Documentation/notation/repeats.itely:151: of the braces of the grouped notes within an @code{@bs{}alternative} block, otherwise you will not get the expected number of endings. Any bar checks used inside @code{@bs{}alternative} block must be placed in inside the braces of each group of notes.} See my comment below for second patch set to Trevor. http://codereview.appspot.com/3705042/diff/8001/Documentation/notation/repeat... Documentation/notation/repeats.itely:239: @cindex repeats with ties OK, but that was not clearly stated for me in the original user thread nor implied in the last set of comments of the patch. I assumed this problem was only for bar checks, so it's only 'logically inconsistent' (as per Carl;s comment) because what was asked for was a doc entry about a common problem that Carl ran into with bar checks in alternatives. It may be obvious to some that other items outside of the braces would cause a problem, but not to me evidently. Thanks for the clarification.
Sign in to reply to this message.
I like it, and think it's ready for pushing.
Just for purposes of clarification, let me summarize the original problem, since
I didn't do a good job before.
The syntax of alternative is:
\alternative {
ending_one
ending_two
ending_three
}
NOT
\alternative {
{ ending_one }
{ ending_two }
{ ending_three }
}
So when somebody enters
\alternative {
{ c4 c c c} |
{ d4 d d d}
}
they are getting three endings:
{c4 c c c} for the first ending
| for the second ending
{d4 d d d} for the third ending
If this follows \repeat volta 2 { }, the third ending (intended second ending)
will never be printed.
So if somebody has the problem above, they're confused in one of two ways, as
far as I can tell:
1) They don't understand that a bar check is a music expression
2) They think the braces are part of the basic alternative construct, rather
than part of the alternative music expressions
What's the best way to clarify? I'm not sure. But the current patch works,
because it's always fine to have each alternative enclosed in braces, even if
it's only a single note.
So I'm happy with the current patch.
Thanks,
Carl
Sign in to reply to this message.
On Sun, Dec 19, 2010 at 02:45:14PM +0000, Carl.D.Sorensen@gmail.com wrote: > So if somebody has the problem above, they're confused in one of two > ways, as far as I can tell: > > 1) They don't understand that a bar check is a music expression > 2) They think the braces are part of the basic alternative construct, > rather than part of the alternative music expressions Interesting, I didn't know that! > What's the best way to clarify? I'm not sure. But the current patch > works, because it's always fine to have each alternative enclosed in > braces, even if it's only a single note. Sounds fine, then. We've gone with "always add {}, even if they're not strictly necessary" in the past for our docs. Cheers, - Graham
Sign in to reply to this message.
Hello, Is this OK to give someone a patch to push? James
Sign in to reply to this message.
Looks fine to me, but then you've used the wording I suggested, so it would, wouldn't it :) Best wait for someone else to approve. Trevor
Sign in to reply to this message.
On 2010/12/21 15:21:29, Trevor Daniels wrote: > Looks fine to me, but then you've used the wording I suggested, so it would, > wouldn't it :) Best wait for someone else to approve. > I believe that Graham and I have both approved it. Carl
Sign in to reply to this message.
Hi James I think a further change is needed to position the warning in repeats correctly. Trevor http://codereview.appspot.com/3705042/diff/12001/Documentation/notation/repea... File Documentation/notation/repeats.itely (right): http://codereview.appspot.com/3705042/diff/12001/Documentation/notation/repea... Documentation/notation/repeats.itely:237: @warning{If there are two or more alternatives, nothing should appear I think this warning would be better placed immediately after the discussion of \alternative, i.e. before the warning about \relative. (Sorry to have missed this earlier - it wasn't obvious until I recompiled the manual with the patch applied.)
Sign in to reply to this message.
Done.
However I think that the two 'warning' boxes will look a bit awkward, but I
understand why Graham doesn't want them all in the same @warning {}.
Sign in to reply to this message.
On 2010/12/22 12:37:41, pkx166h wrote: > I think that the two 'warning' boxes will look a bit awkward Maybe, but the placement makes the warning much more relevant. Let's leave for a couple of days, and if there are no further comments send me a patch.
Sign in to reply to this message.
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
