Re: Doc: Various to LM and NR from user email threads (issue3581041)

classic Classic list List threaded Threaded
11 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: Doc: Various to LM and NR from user email threads (issue3581041)

percival.music.ca
Thanks for this!

I've slightly modified the git-cl instructions, to include "make the CC
list lilypond-devel".  That's part of the git-cl config stage, so you
don't need to remember it for every individual patch you upload.




http://codereview.appspot.com/3581041/diff/1/Documentation/learning/fundamental.itely
File Documentation/learning/fundamental.itely (right):

http://codereview.appspot.com/3581041/diff/1/Documentation/learning/fundamental.itely#newcode2113
Documentation/learning/fundamental.itely:2113: @warning{The
@code{Stem_engraver} and @code{Beam_engraver} attach their
So far, we don't have any @warning inside the @seealso section.  I'd say
that unless you're specifically warning about one of our links (dunno
what that might mean... "warning: this link might not work?" :), this
should either be higher up in the section, or be a known issue.

Since you're talking about removing an engraver (which isn't really
standard NR-level stuff), I'd suggest making it a knownissue rather than
a warning.

http://codereview.appspot.com/3581041/diff/1/Documentation/notation/spacing.itely
File Documentation/notation/spacing.itely (right):

http://codereview.appspot.com/3581041/diff/1/Documentation/notation/spacing.itely#newcode928
Documentation/notation/spacing.itely:928: @warning{Odd page numbers are
always on the right.  If you want the
On 2010/12/11 00:36:29, Carl wrote:
> I think that this should be a known issue, rather than a warning.

Agreed, this is definitely better as a known issue.

http://codereview.appspot.com/3581041/diff/1/Documentation/notation/staff.itely
File Documentation/notation/staff.itely (right):

http://codereview.appspot.com/3581041/diff/1/Documentation/notation/staff.itely#newcode278
Documentation/notation/staff.itely:278: @code{PianoStaff} does not, by
default, accept @code{ChordNames}.
This should be a known issue, instead of a warn... oh, oops, sorry, it's
already a known issue.  :)

http://codereview.appspot.com/3581041/

_______________________________________________
lilypond-devel mailing list
[hidden email]
http://lists.gnu.org/mailman/listinfo/lilypond-devel
Reply | Threaded
Open this post in threaded view
|

Re: Doc: Various to LM and NR from user email threads (issue3581041)

pkx166h
Reviewers: Trevor Daniels, Valentin Villenave, carl.d.sorensen_gmail.com,  
Graham Percival,

Message:
I hope this is the right method to have a discussion on Rietveld, I have
added my own comments/responses to yours inline.

James


http://codereview.appspot.com/3581041/diff/1/Documentation/notation/repeats.itely
File Documentation/notation/repeats.itely (right):

http://codereview.appspot.com/3581041/diff/1/Documentation/notation/repeats.itely#newcode150
Documentation/notation/repeats.itely:150: @rprogram{An extra staff
appears}.  Do not place bar checks outside
On 2010/12/11 00:36:29, Carl wrote:
> I think this is awkward.

> I think it would be better to do the following:

> 1) Put barchecks inside the alternatives in the example.

I know that we were trying to avoid bar checks on single measures - this
was/is CG policy and bar checks are not mandatory anyway (helpful but
not a requirement) so add nothing to an example other than this one
case. If I put them here, then I have to put them throughout the other
examples. I take the point that you might think it better to show an
example which I could for instance with an @example but I'd like some
opinion on that because we could then start setting precedence on
showing what NOT to do if you see what I mean?


> 2) Show an example of the general case, which is that the first two
music
> expressions following alternative, rather than the first two braced
expressions
> following alternative, are taken as the alternatives.  For example, it
might be
> something like

> \alternative
>   {c4 c c c |}
>   \markup "This is an expression"
>   {d4 d d d |}

> which will have the same effect as the bad bar check.  In other words,
it's not
> a bar check specific problem.

http://codereview.appspot.com/3581041/diff/1/Documentation/notation/repeats.itely#newcode151
Documentation/notation/repeats.itely:151: of and in between the braces
of the grouped notes when using
On 2010/12/10 23:56:08, Valentin Villenave wrote:
> This sentence is too long and not very clear. Perhaps some additional
> punctuation might help?

> -> "When using @code{@bs{}alternative} endings, do not place bar
checks outside
> of, or in-between, the braces of the grouped notes: otherwise you will
not get
> the expected number of endings."

> Besides, instead of "the braces of the grouped notes" I'd say "the
blocks
> delimited by braces" or something like that.

Valentin, I have tried to be consistent with the other descriptions
above for the different cases of \alternative repeats. We don't use
'blocks delimited..' there they are called 'grouped notes' and the word
'braces' is used. Blocks is too vague and could for instance refer to
the whole 'block' of the \alternative {..{...} {...} } construct rather
than the groups of notes within the \alternative { .. }

Description:
Doc: Various to LM and NR from user email threads

Staff.itely (NR)
@knownissues that PianoStaff does not accept ChordNames
http://lists.gnu.org/archive/html/lilypond-user/2010-09/msg00297.html


Spacing.itely (NR)
@warning that odd page numbers are always on the RH page and that you
must
have a blank second page if you want music to start on page '1' is you
have
a cover page
http://lists.gnu.org/archive/html/lilypond-user/2010-06/msg00475.html


fundamental.itely (LM)
@warning that is you remove the Notes_head_engraver that the stem and
beam
engravers have nothing to attach to so nothing is produced
http://lists.gnu.org/archive/html/lilypond-user/2010-08/msg00372.html


editorial.itely (NR)
@knownissue that using \teeny \tiny \small etc instead of explicit
fontSize
will produce inconsistent line spacing comparitively
http://lists.gnu.org/archive/html/lilypond-user/2010-09/msg00665.html

repeats.itely (NR)
@warning not to use bar checks outside of braces in grouped notes within
an \alternative contruct else the number of repeats printed will be
incorrect
http://lists.gnu.org/archive/html/lilypond-user/2010-12/msg00067.html

Please review this at http://codereview.appspot.com/3581041/

Affected files:
   M Documentation/learning/fundamental.itely
   M Documentation/notation/editorial.itely
   M Documentation/notation/repeats.itely
   M Documentation/notation/spacing.itely
   M Documentation/notation/staff.itely


Index: Documentation/learning/fundamental.itely
diff --git a/Documentation/learning/fundamental.itely  
b/Documentation/learning/fundamental.itely
index  
b80e6372b3b9eeb9cedbd59f9c19fce99241923d..f8df3f49b736b267a5c68082f91abb76c11415a2  
100644
--- a/Documentation/learning/fundamental.itely
+++ b/Documentation/learning/fundamental.itely
@@ -2110,6 +2110,10 @@ same way.
  Notation Reference: @ruser{Modifying context plug-ins},
  @ruser{Changing context default settings}.

+@warning{The @code{Stem_engraver} and @code{Beam_engraver} attach their
+objects to note heads, if you remove the @code{Note_heads_engraver}
+no note heads are produced and so no Stem or Beams are created either.}
+

  @node Extending the templates
  @section Extending the templates
Index: Documentation/notation/editorial.itely
diff --git a/Documentation/notation/editorial.itely  
b/Documentation/notation/editorial.itely
index  
c83383359c698b30c5fdbc64a49967a88208340e..8647c36deba92d083bdeb6646ba48cb6d46c0bd3  
100644
--- a/Documentation/notation/editorial.itely
+++ b/Documentation/notation/editorial.itely
@@ -138,6 +138,12 @@ Snippets:
  Internals Reference:
  @rinternals{font-interface}.

+@knownissues
+Using the font sizing commands @code{\teeny}, @code{\tiny},
+@code{\small}, @code{\normalsize}, @code{\large}, and
+@code{\huge} will lead to inconsistent line spacing when compared with
+using the @code{fontSize} property.
+

  @node Fingering instructions
  @unnumberedsubsubsec Fingering instructions
Index: Documentation/notation/repeats.itely
diff --git a/Documentation/notation/repeats.itely  
b/Documentation/notation/repeats.itely
index  
efae065ebf50dc6891eddb28f1492258357978c8..65294caab2fbf54329441f63bd0490f48108a5a1  
100644
--- a/Documentation/notation/repeats.itely
+++ b/Documentation/notation/repeats.itely
@@ -147,7 +147,11 @@ c1
  @warning{If you include @code{@bs{}relative} inside a
  @code{@bs{}repeat} without explicitly instantiating the
  @code{Voice} context, extra (unwanted) staves will appear.  See
-@rprogram{An extra staff appears}.}
+@rprogram{An extra staff appears}.  Do not place bar checks outside
+of and in between the braces of the grouped notes when using
+@code{@bs{}alternative} endings otherwise you will not get the expected
+number of endings.  If you must use bar checks then put
+them after the notes but inside the braces of each group.}

  @cindex repeat with upbeat
  @cindex upbeat in a repeat
Index: Documentation/notation/spacing.itely
diff --git a/Documentation/notation/spacing.itely  
b/Documentation/notation/spacing.itely
index  
89810f6dad17b130f1566f91d5aa28a88b2169f1..04c49dfc47d73e273b7a80aa5c39ec898cf9de4d  
100644
--- a/Documentation/notation/spacing.itely
+++ b/Documentation/notation/spacing.itely
@@ -925,6 +925,10 @@ If set to false, page numbers are not printed.
  Installed Files:
  @file{ly/paper-defaults-init.ly}.

+@warning{Odd page numbers are always on the right.  If you want the
+music to start on page 1 there must be a blank page on the back
+of the cover page so that page 1 is on the right hand side.}
+

  @node Miscellaneous \paper variables
  @unnumberedsubsubsec Miscellaneous @code{\paper} variables
Index: Documentation/notation/staff.itely
diff --git a/Documentation/notation/staff.itely  
b/Documentation/notation/staff.itely
index  
c964305f2339c334562ced8222a5e9a42b7868f3..01861c2a1470f5be0709edb9749b0077ace8377d  
100644
--- a/Documentation/notation/staff.itely
+++ b/Documentation/notation/staff.itely
@@ -274,6 +274,9 @@ Internals Reference:
  @rinternals{SystemStartBracket},
  @rinternals{SystemStartSquare}.

+@knownissues
+@code{PianoStaff} does not, by default, accept @code{ChordNames}.
+

  @node Nested staff groups
  @unnumberedsubsubsec Nested staff groups



_______________________________________________
lilypond-devel mailing list
[hidden email]
http://lists.gnu.org/mailman/listinfo/lilypond-devel
Reply | Threaded
Open this post in threaded view
|

Re: Doc: Various to LM and NR from user email threads (issue3581041)

Carl Sorensen
In reply to this post by percival.music.ca
On 2010/12/11 16:33:48, pkx166h wrote:
> I hope this is the right method to have a discussion on Rietveld, I
have added
> my own comments/responses to yours inline.

Yes, this is a very good way to do it.

You can also just add a comment in the patchset.

http://codereview.appspot.com/3581041/diff/1/Documentation/notation/repeats.itely#newcode150
> Documentation/notation/repeats.itely:150: @rprogram{An extra staff
appears}.  Do
> not place bar checks outside
> On 2010/12/11 00:36:29, Carl wrote:
> > I think this is awkward.
> >
> > I think it would be better to do the following:
> >
> > 1) Put barchecks inside the alternatives in the example.

> I know that we were trying to avoid bar checks on single measures -
this was/is
> CG policy

But this isn't a single measure.  There are three measures in the
example.  Bar checks *should* be used here, although they're not
required.


> and bar checks are not mandatory anyway (helpful but not a
> requirement) so add nothing to an example other than this one case. If
I put
> them here, then I have to put them throughout the other examples.

In my opinion, we should be moving toward bar checks in examples where
it is helpful, rather than as a mindless policy requiring consistency in
all cases.

This particular case is a case where a user made a mistake by copying an
example and doing what he thought was right.  If the bar check were
inside the alternative, the mistake would not have been made.  So I
think it's a positive change.


> I take the
> point that you might think it better to show an example which I could
for
> instance with an @example but I'd like some opinion on that because we
could
> then start setting precedence on showing what NOT to do if you see
what I mean?

To me the precedent is set by virtue of the fact that a user made the
mistake.

I agree we don't want to show what not to do in general.  We could
probably have avoided the problem if the bar check were just in the
alternatives.



http://codereview.appspot.com/3581041/

_______________________________________________
lilypond-devel mailing list
[hidden email]
http://lists.gnu.org/mailman/listinfo/lilypond-devel
Reply | Threaded
Open this post in threaded view
|

Re: Doc: Various to LM and NR from user email threads (issue3581041)

v.villenave
In reply to this post by percival.music.ca

http://codereview.appspot.com/3581041/diff/1/Documentation/notation/repeats.itely
File Documentation/notation/repeats.itely (right):

http://codereview.appspot.com/3581041/diff/1/Documentation/notation/repeats.itely#newcode151
Documentation/notation/repeats.itely:151: of and in between the braces
of the grouped notes when using
On 2010/12/11 16:33:48, pkx166h wrote:
> Valentin, I have tried to be consistent with the other descriptions
above for
> the different cases of \alternative repeats. We don't use 'blocks
delimited..'
> there they are called 'grouped notes' and the word 'braces' is used.

Point taken. But my main concern was about punctuation, not wording :-)

http://codereview.appspot.com/3581041/

_______________________________________________
lilypond-devel mailing list
[hidden email]
http://lists.gnu.org/mailman/listinfo/lilypond-devel
Reply | Threaded
Open this post in threaded view
|

Re: Doc: Various to LM and NR from user email threads (issue3581041)

pkx166h
In reply to this post by percival.music.ca
Sorry I didn't read Graham;s comment first time round.


http://codereview.appspot.com/3581041/diff/1/Documentation/learning/fundamental.itely
File Documentation/learning/fundamental.itely (right):

http://codereview.appspot.com/3581041/diff/1/Documentation/learning/fundamental.itely#newcode2113
Documentation/learning/fundamental.itely:2113: @warning{The
@code{Stem_engraver} and @code{Beam_engraver} attach their
On 2010/12/11 14:01:10, Graham Percival wrote:
> So far, we don't have any @warning inside the @seealso section.  I'd
say that
> unless you're specifically warning about one of our links (dunno what
that might
> mean... "warning: this link might not work?" :), this should either be
higher up
> in the section, or be a known issue.

Yes sorry I meant this to be an @knownissue then changed my mind but not
the position.


> Since you're talking about removing an engraver (which isn't really
standard
> NR-level stuff), I'd suggest making it a knownissue rather than a
warning.

This is LM Graham. Not that that makes a difference to me if you want it
to be @warning or @knownissue. We do mention how to remove
engravers/contexts in the LM as a gentle introduction, and I was
thinking that 'removing the noteheads' is kind of what anyone new to LP
might do, just because they can...but also we show in the LM how to
create scores for 'teaching music' (remove bars for putting back in,
remove notes to teach rhythm etc) so I thought this appropriate in the
LM, but your comment about thinking it was NR now leads me to think
maybe it should be in there instead?

james

http://codereview.appspot.com/3581041/

_______________________________________________
lilypond-devel mailing list
[hidden email]
http://lists.gnu.org/mailman/listinfo/lilypond-devel
Reply | Threaded
Open this post in threaded view
|

Re: Doc: Various to LM and NR from user email threads (issue3581041)

Neil Puttock
In reply to this post by percival.music.ca

http://codereview.appspot.com/3581041/diff/1/Documentation/learning/fundamental.itely
File Documentation/learning/fundamental.itely (right):

http://codereview.appspot.com/3581041/diff/1/Documentation/learning/fundamental.itely#newcode2115
Documentation/learning/fundamental.itely:2115: no note heads are
produced and so no Stem or Beams are created either.}
stems or beams

http://codereview.appspot.com/3581041/diff/1/Documentation/notation/editorial.itely
File Documentation/notation/editorial.itely (right):

http://codereview.appspot.com/3581041/diff/1/Documentation/notation/editorial.itely#newcode145
Documentation/notation/editorial.itely:145: using the @code{fontSize}
property.
This is a problem in markup only, thus applies to the \fontsize markup
command (not the context property fontSize).

I suggested changing the definitions of the named commands to remove the
inconsistency (via \abs-fontsize), but if we'd rather leave them as they
are this knowissue should be moved to text.itely.

http://codereview.appspot.com/3581041/

_______________________________________________
lilypond-devel mailing list
[hidden email]
http://lists.gnu.org/mailman/listinfo/lilypond-devel
Reply | Threaded
Open this post in threaded view
|

Re: Doc: Various to LM and NR from user email threads (issue3581041)

percival.music.ca
In reply to this post by percival.music.ca
Lots of good discussion here, but I'm finding a bit confusing.  James,
could you extract whatever part(s) of your first patch that everybody
agreed on, and make a separate patch that only does those?  That way, we
can get the "obvious" stuff pushed, and then focus on whatever "not
obvious" stuff is left.  I'd also suggest that once the "obvious" stuff
is pushed, you create a patch for only 1 of the remaining files -- that
will let us focus even more.

Remember that patches should not only *be* good, they must be *seen* to
be good.  When there's a lot of unrelated stuff in the same patch, it's
harder to see that stuff is good.

http://codereview.appspot.com/3581041/

_______________________________________________
lilypond-devel mailing list
[hidden email]
http://lists.gnu.org/mailman/listinfo/lilypond-devel
Reply | Threaded
Open this post in threaded view
|

Re: Doc: Various to LM and NR from user email threads (issue3581041)

pkx166h
In reply to this post by percival.music.ca

http://codereview.appspot.com/3581041/diff/1/Documentation/learning/fundamental.itely
File Documentation/learning/fundamental.itely (right):

http://codereview.appspot.com/3581041/diff/1/Documentation/learning/fundamental.itely#newcode2115
Documentation/learning/fundamental.itely:2115: no note heads are
produced and so no Stem or Beams are created either.}
On 2010/12/11 22:56:02, Neil Puttock wrote:
> stems or beams

Leaving this file unedited for another patch until those other files
that have
consensus have been pushed.

http://codereview.appspot.com/3581041/diff/1/Documentation/notation/editorial.itely
File Documentation/notation/editorial.itely (right):

http://codereview.appspot.com/3581041/diff/1/Documentation/notation/editorial.itely#newcode145
Documentation/notation/editorial.itely:145: using the @code{fontSize}
property.
On 2010/12/11 22:56:02, Neil Puttock wrote:
> This is a problem in markup only, thus applies to the \fontsize markup
command
> (not the context property fontSize).

> I suggested changing the definitions of the named commands to remove
the
> inconsistency (via \abs-fontsize), but if we'd rather leave them as
they are
> this knowissue should be moved to text.itely.

Done.

http://codereview.appspot.com/3581041/diff/1/Documentation/notation/repeats.itely
File Documentation/notation/repeats.itely (right):

http://codereview.appspot.com/3581041/diff/1/Documentation/notation/repeats.itely#newcode153
Documentation/notation/repeats.itely:153: number of endings.  If you
must use bar checks then put
On 2010/12/10 23:41:50, Trevor Daniels wrote:
> If you do use bar checks ...

Leaving this file unedited for another patch until those other files
that have consensus have been pushed.

http://codereview.appspot.com/3581041/diff/1/Documentation/notation/spacing.itely
File Documentation/notation/spacing.itely (right):

http://codereview.appspot.com/3581041/diff/1/Documentation/notation/spacing.itely#newcode928
Documentation/notation/spacing.itely:928: @warning{Odd page numbers are
always on the right.  If you want the
On 2010/12/11 14:01:10, Graham Percival wrote:
> On 2010/12/11 00:36:29, Carl wrote:
> > I think that this should be a known issue, rather than a warning.

> Agreed, this is definitely better as a known issue.

Done.

http://codereview.appspot.com/3581041/diff/1/Documentation/notation/staff.itely
File Documentation/notation/staff.itely (right):

http://codereview.appspot.com/3581041/diff/1/Documentation/notation/staff.itely#newcode278
Documentation/notation/staff.itely:278: @code{PianoStaff} does not, by
default, accept @code{ChordNames}.
On 2010/12/11 14:01:10, Graham Percival wrote:
> This should be a known issue, instead of a warn... oh, oops, sorry,
it's already
> a known issue.  :)

Done.

http://codereview.appspot.com/3581041/

_______________________________________________
lilypond-devel mailing list
[hidden email]
http://lists.gnu.org/mailman/listinfo/lilypond-devel
Reply | Threaded
Open this post in threaded view
|

Re: Doc: Various to LM and NR from user email threads (issue3581041)

pkx166h
In reply to this post by percival.music.ca
Is this ok to push?

James

http://codereview.appspot.com/3581041/

_______________________________________________
lilypond-devel mailing list
[hidden email]
http://lists.gnu.org/mailman/listinfo/lilypond-devel
Reply | Threaded
Open this post in threaded view
|

Re: Doc: Various to LM and NR from user email threads (issue3581041)

tdanielsmusic
In reply to this post by percival.music.ca
Looks fine to me.

http://codereview.appspot.com/3581041/

_______________________________________________
lilypond-devel mailing list
[hidden email]
http://lists.gnu.org/mailman/listinfo/lilypond-devel
Reply | Threaded
Open this post in threaded view
|

Re: Doc: Various to LM and NR from user email threads (issue3581041)

percival.music.ca
In reply to this post by percival.music.ca
On 2010/12/14 22:45:26, Trevor Daniels wrote:
> Looks fine to me.

LGTM too.  James, could you send the git format-patch to Trevor, who
will push it for you?

Cheers,
- Graham

http://codereview.appspot.com/3581041/

_______________________________________________
lilypond-devel mailing list
[hidden email]
http://lists.gnu.org/mailman/listinfo/lilypond-devel