|
|
Descriptionadds documentation for the new bar line interface
Patch Set 1 #Patch Set 2 : missing "" added #
Total comments: 12
Patch Set 3 : rebased to current master #Patch Set 4 : changes according to James' proposals #Patch Set 5 : this goes to eleven ;-) #
Total comments: 10
Patch Set 6 : changes according to James' proposals #
Total comments: 2
Patch Set 7 : Minor typos. #MessagesTotal messages: 7
Thanks for doing this. Some comments (it's the one thing I feel competent enough to join in and make suggestions on). James https://codereview.appspot.com/6742061/diff/2001/Documentation/notation/rhyth... File Documentation/notation/rhythms.itely (right): https://codereview.appspot.com/6742061/diff/2001/Documentation/notation/rhyth... Documentation/notation/rhythms.itely:2765: New bar line styles can be defined with @code{\defineBarLine}. Could we have some @funindex \defineBarLine @cindex bar lines, defining @cindex defining bar lines or some similar case - I haven't checked this in the current documents to see what else we are using. https://codereview.appspot.com/6742061/diff/2001/Documentation/notation/rhyth... Documentation/notation/rhythms.itely:2779: The second argument is a list containing three entries I think you need to clarify this more. I cannot see what the 'second argument' is. So for example if you look here: http://lilypond.org/doc/v2.16/Documentation/notation/the-set-command you can see that we list the command with appropriate @var{..}. That would help the non-technical user to at least get a handle on the command in full. Then move this para above the @lilypond (after the \defineBarLine @var{} @var{} and this is more coherent to read. https://codereview.appspot.com/6742061/diff/2001/Documentation/notation/rhyth... Documentation/notation/rhythms.itely:2781: beginning of the next line and as a span bar, respectively. Is there any merit in @ref{}ing http://lilypond.org/doc/v2.16/Documentation/notation/visibility-of-objects#us... Or perhaps giving an equivalence? https://codereview.appspot.com/6742061/diff/2001/Documentation/notation/rhyth... Documentation/notation/rhythms.itely:2783: There are currently eleven glyphs available: See my comment below about the term 'glyphs'. https://codereview.appspot.com/6742061/diff/2001/Documentation/notation/rhyth... Documentation/notation/rhythms.itely:2789: segno sign, and @code{"["} and @code{"]"} for the brackets. Would it be too much to have an @lilypond example for all of them? Instead of just listing them https://codereview.appspot.com/6742061/diff/2001/Documentation/notation/rhyth... Documentation/notation/rhythms.itely:2796: The @code{"-"} sign allows to distinguish bar lines with The @code{"-"} sign distinguishes bar lines that have identical... https://codereview.appspot.com/6742061/diff/2001/Documentation/notation/rhyth... Documentation/notation/rhythms.itely:2802: way to define new bar line glyphs. For more informations are these actually 'glyphs'? I ask because if they are then they need to appear somewhere like here: http://lilypond.org/doc/v2.16/Documentation/notation/the-feta-font I've never really understood why bar lines (apart from the very fancy scripts.varsegno) are not listed here. I am guessing they are not glyphs and this therefore needs to be corrected?
Sign in to reply to this message.
James, thanks for your remarks and corrections! Some answers (partly preliminal) are listed below. https://codereview.appspot.com/6742061/diff/2001/Documentation/notation/rhyth... File Documentation/notation/rhythms.itely (right): https://codereview.appspot.com/6742061/diff/2001/Documentation/notation/rhyth... Documentation/notation/rhythms.itely:2765: New bar line styles can be defined with @code{\defineBarLine}. On 2012/10/22 11:09:58, J_lowe wrote: > Could we have some > > @funindex \defineBarLine > @cindex bar lines, defining > @cindex defining bar lines > > or some similar case - I haven't checked this in the current documents to see > what else we are using. Ok, I'll take a look. https://codereview.appspot.com/6742061/diff/2001/Documentation/notation/rhyth... Documentation/notation/rhythms.itely:2779: The second argument is a list containing three entries On 2012/10/22 11:09:58, J_lowe wrote: > I think you need to clarify this more. I cannot see what the 'second argument' > is. > > So for example if you look here: > > http://lilypond.org/doc/v2.16/Documentation/notation/the-set-command > > you can see that we list the command with appropriate @var{..}. That would help > the non-technical user to at least get a handle on the command in full. > > Then move this para above the @lilypond (after the \defineBarLine @var{} @var{} > > and this is more coherent to read. Ok, good point. https://codereview.appspot.com/6742061/diff/2001/Documentation/notation/rhyth... Documentation/notation/rhythms.itely:2789: segno sign, and @code{"["} and @code{"]"} for the brackets. On 2012/10/22 11:09:58, J_lowe wrote: > Would it be too much to have an @lilypond example for all of them? Instead of > just listing them There is no predefined \bar "[" or something similar, so one cannot show all "glyphs" without defining additional bar lines. I don't know whether it worth it, but it would be doable. I'll play around and see how far I can get with that. https://codereview.appspot.com/6742061/diff/2001/Documentation/notation/rhyth... Documentation/notation/rhythms.itely:2796: The @code{"-"} sign allows to distinguish bar lines with On 2012/10/22 11:09:58, J_lowe wrote: > The @code{"-"} sign distinguishes bar lines that have identical... Yes, much better. Thanks! https://codereview.appspot.com/6742061/diff/2001/Documentation/notation/rhyth... Documentation/notation/rhythms.itely:2802: way to define new bar line glyphs. For more informations On 2012/10/22 11:09:58, J_lowe wrote: > are these actually 'glyphs'? I ask because if they are then they need to appear > somewhere like here: > > http://lilypond.org/doc/v2.16/Documentation/notation/the-feta-font > > I've never really understood why bar lines (apart from the very fancy > scripts.varsegno) are not listed here. I am guessing they are not glyphs and > this therefore needs to be corrected? No, they aren't. The term "glyph" is taken from the grob property BarLine #'glyph which holds the string that describes the bar line. This string is transformed into a series of drawing routines that build up the final stencil. I am not very happy with this naming convention and tried to explain it in scm/define-grob-properties.scm (see glyph and glyph-name). How shall I proceed? Use the term "glyph" as defined in the grob properties, or replace it by a better term?
Sign in to reply to this message.
Mark, Nearly there. Some more general comments - some are Doc Policy, some are my own views on how to organize this section, also can you make sure you use two spaces after a full point (full stop) as this is also the CG policy (even if it does go against every grain in my typographically-based bones!). James https://codereview.appspot.com/6742061/diff/8001/Documentation/notation/rhyth... File Documentation/notation/rhythms.itely (right): https://codereview.appspot.com/6742061/diff/8001/Documentation/notation/rhyth... Documentation/notation/rhythms.itely:2693: Further details of this notation are explained in @ref{Typesetting Kievan square notation}. Make sure you only use max 72 chars per line and dont break @ref{...} over two lines http://lilypond.org/doc/v2.16/Documentation/contributor/syntax-survey#cross-r... Causes problems in texinfo output https://codereview.appspot.com/6742061/diff/8001/Documentation/notation/rhyth... Documentation/notation/rhythms.itely:2780: \defineBarLine @var{bartype} #'(@var{bar-line-at-end-of-line} @var{bar-line-at-begin-of-line} @var{span-bar-line}) You probably aught to shorten the variable names for the doc (even if the function names internally are called this, most users don't care and you always have the @ref{} under the @seealso at the end of the section to link to the ly: file for those that are really curious. @var{type} @var{end} @var{begin} @var{span} else if you do end up going over 72 chars, break and indent the line as you would a lilypond example. Makes it clearer for readers. https://codereview.appspot.com/6742061/diff/8001/Documentation/notation/rhyth... Documentation/notation/rhythms.itely:2784: @samp{\bar "bartype"}. Use @code{\bar} @var{bartype} https://codereview.appspot.com/6742061/diff/8001/Documentation/notation/rhyth... Documentation/notation/rhythms.itely:2792: bar line. Move the Para above the previous (just after the @example). The @code{\defineBarline} variables can include the @q{empty} string @code{""}, which is equivalent to an invisible bar line being printed. Or they can be set to @code{#f} which prints no bar line at all. -- The implication being that an invisible bar line uses normal padding/spacing. This also saves you repeating all the @var{}s again. https://codereview.appspot.com/6742061/diff/8001/Documentation/notation/rhyth... Documentation/notation/rhythms.itely:2822: s1 \bar "]" \mark \markup { \typewriter "]" } I am guessing that \typewriter is used here to prevent century schoolbook and so make the characters more obvious, however there is no need if you use the normal @lilypond[verbatim, quote etc.] as this *is* printed in 'typewriter' font in the doc (and is why we do this), so you can rmeove all the \mark \markup { } from this if you use the correct @lilypond variables. Also, could we move this whole @lilypond section right up to the (almost) start, under the para 'After the definiton, the new bar line can be used by @samp{\bar "bartype"}.' --- That way we get right to the point and then can do the detail afterwards, so to speak. It's not complicated to understand and so the finer points can be explained as the reader reads through. https://codereview.appspot.com/6742061/diff/8001/Documentation/notation/rhyth... Documentation/notation/rhythms.itely:2828: combination with the segno sign. It is not recommended for use as could we have a short reason it is not recommended? Else I always just say 'Do not use ....'. I prefer to be led than given choice if it's that subtle or there is no explanation on why I might not 'prefer' to do something. https://codereview.appspot.com/6742061/diff/8001/Documentation/notation/rhyth... Documentation/notation/rhythms.itely:2829: a standalone double thin bar line; here, @samp{\bar "||"} is ... here, @code{\bar} @var{"||"} is ... https://codereview.appspot.com/6742061/diff/8001/Documentation/notation/rhyth... Documentation/notation/rhythms.itely:2832: The @code{"-"} sign distinguishes bar lines with The @code{"-"} sign separates (?). Could we have an @lilypond example here to show like you do with the @code{" "} example below? https://codereview.appspot.com/6742061/diff/8001/Documentation/notation/rhyth... Documentation/notation/rhythms.itely:2854: If additional elements are needed, Lilypond provides a simple LilyPond not Lilypond. https://codereview.appspot.com/6742061/diff/8001/Documentation/notation/rhyth... Documentation/notation/rhythms.itely:2856: bar lines, see file @file{scm/bar-line.scm}. Add this @file{} reference to an @seealso at the end. See: http://lilypond.org/doc/v2.16/Documentation/contributor/syntax-survey#cross-r... and http://lilypond.org/doc/v2.16/Documentation/contributor/section-organization
Sign in to reply to this message.
On 2012/10/23 09:03:32, J_lowe wrote: > Mark, > > Nearly there. Hey, sounds prominsing ;-) > > Some more general comments - some are Doc Policy, some are my own views on how > to organize this section, also can you make sure you use two spaces after a full > point (full stop) as this is also the CG policy (even if it does go against > every grain in my typographically-based bones!). > > James > > https://codereview.appspot.com/6742061/diff/8001/Documentation/notation/rhyth... > File Documentation/notation/rhythms.itely (right): > > https://codereview.appspot.com/6742061/diff/8001/Documentation/notation/rhyth... > Documentation/notation/rhythms.itely:2693: Further details of this notation are > explained in @ref{Typesetting Kievan square notation}. > Make sure you only use max 72 chars per line and dont break @ref{...} over two > lines > > http://lilypond.org/doc/v2.16/Documentation/contributor/syntax-survey#cross-r... > > Causes problems in texinfo output Ok. > > https://codereview.appspot.com/6742061/diff/8001/Documentation/notation/rhyth... > Documentation/notation/rhythms.itely:2780: \defineBarLine @var{bartype} > #'(@var{bar-line-at-end-of-line} @var{bar-line-at-begin-of-line} > @var{span-bar-line}) > You probably aught to shorten the variable names for the doc (even if the > function names internally are called this, most users don't care and you always > have the @ref{} under the @seealso at the end of the section to link to the ly: > file for those that are really curious. > > @var{type} @var{end} @var{begin} @var{span} else if you do end up going over 72 > chars, break and indent the line as you would a lilypond example. > > Makes it clearer for readers. Ok, will do. > https://codereview.appspot.com/6742061/diff/8001/Documentation/notation/rhyth... > Documentation/notation/rhythms.itely:2784: @samp{\bar "bartype"}. > Use @code{\bar} @var{bartype} Ok. I found examples where @samp is used in similar circumstances, but I trust you on this. > https://codereview.appspot.com/6742061/diff/8001/Documentation/notation/rhyth... > Documentation/notation/rhythms.itely:2792: bar line. > Move the Para above the previous (just after the @example). > > The @code{\defineBarline} variables can include the @q{empty} string @code{""}, > which is equivalent to an invisible bar line being printed. Or they can be set > to @code{#f} which prints no bar line at all. > > -- > > The implication being that an invisible bar line uses normal padding/spacing. > This also saves you repeating all the @var{}s again. > > https://codereview.appspot.com/6742061/diff/8001/Documentation/notation/rhyth... > Documentation/notation/rhythms.itely:2822: s1 \bar "]" \mark \markup { > \typewriter "]" } > I am guessing that \typewriter is used here to prevent century schoolbook and so > make the characters more obvious, however there is no need if you use the normal > @lilypond[verbatim, quote etc.] as this *is* printed in 'typewriter' font in the > doc (and is why we do this), so you can rmeove all the \mark \markup { } from > this if you use the correct @lilypond variables. I used typewriter and did not use verbatim, because I have to define several bar lines which clutters the output IMHO. But in combination with the proposed reordering, it may make more sense. > > Also, could we move this whole @lilypond section right up to the (almost) > start, under the para > > 'After the definiton, the new bar line can be used by @samp{\bar "bartype"}.' > > --- > > That way we get right to the point and then can do the detail afterwards, so to > speak. It's not complicated to understand and so the finer points can be > explained as the reader reads through. > > https://codereview.appspot.com/6742061/diff/8001/Documentation/notation/rhyth... > Documentation/notation/rhythms.itely:2828: combination with the segno sign. It > is not recommended for use as > could we have a short reason it is not recommended? Else I always just say 'Do > not use ....'. I prefer to be led than given choice if it's that subtle or there > is no explanation on why I might not 'prefer' to do something. The reason is that the "=" is spaced according to the width of the "S" sign; if you use this as a normal bar line, the internal spacing in combination with the bar line padding looks way too ugly. But I see your point: this explanation is probably too complicated; I'll go for 'Do not use ...' > > https://codereview.appspot.com/6742061/diff/8001/Documentation/notation/rhyth... > Documentation/notation/rhythms.itely:2829: a standalone double thin bar line; > here, @samp{\bar "||"} is > ... here, @code{\bar} @var{"||"} is ... > > https://codereview.appspot.com/6742061/diff/8001/Documentation/notation/rhyth... > Documentation/notation/rhythms.itely:2832: The @code{"-"} sign distinguishes bar > lines with > The @code{"-"} sign separates (?). Could we have an @lilypond example here to > show like you do with the @code{" "} example below? Oh, I was somewhat proud to have *one* example where the '-' *and* the ' ' were explained simultaneously and quite naturally (at least IMHO). > https://codereview.appspot.com/6742061/diff/8001/Documentation/notation/rhyth... > Documentation/notation/rhythms.itely:2854: If additional elements are needed, > Lilypond provides a simple > LilyPond not Lilypond. Oops. > > https://codereview.appspot.com/6742061/diff/8001/Documentation/notation/rhyth... > Documentation/notation/rhythms.itely:2856: bar lines, see file > @file{scm/bar-line.scm}. > Add this @file{} reference to an @seealso at the end. > > See: > http://lilypond.org/doc/v2.16/Documentation/contributor/syntax-survey#cross-r... > > and > > http://lilypond.org/doc/v2.16/Documentation/contributor/section-organization Ok.
Sign in to reply to this message.
One typo so, as we need to fix this, I also included a minor grammar change. Otherwise LGTM. https://codereview.appspot.com/6742061/diff/10001/Documentation/notation/rhyt... File Documentation/notation/rhythms.itely (right): https://codereview.appspot.com/6742061/diff/10001/Documentation/notation/rhyt... Documentation/notation/rhythms.itely:2815: The @code{"="} bar line provides the double span bar line used ...bar line, used in combination... https://codereview.appspot.com/6742061/diff/10001/Documentation/notation/rhyt... Documentation/notation/rhythms.itely:2821: are useful to distinguish bar lines with identical appearance ...to distinguish those with identical...
Sign in to reply to this message.
On 2012/10/23 23:22:03, J_lowe wrote: > One typo so, as we need to fix this, I also included a minor grammar change. > > Otherwise LGTM. > > https://codereview.appspot.com/6742061/diff/10001/Documentation/notation/rhyt... > File Documentation/notation/rhythms.itely (right): > > https://codereview.appspot.com/6742061/diff/10001/Documentation/notation/rhyt... > Documentation/notation/rhythms.itely:2815: The @code{"="} bar line provides the > double span bar line used > ...bar line, used in combination... Done. > > https://codereview.appspot.com/6742061/diff/10001/Documentation/notation/rhyt... > Documentation/notation/rhythms.itely:2821: are useful to distinguish bar lines > with identical appearance > ...to distinguish those with identical... Done. Thanks for your comments and improvements!
Sign in to reply to this message.
|