Re: Doc: LM: Reformat ly code. (issue1056041)

[percival . music . ca]

Looking much closer now.


http://codereview.appspot.com/1056041/diff/24001/25001
File Documentation/learning/common-notation.itely (right):

http://codereview.appspot.com/1056041/diff/24001/25001#newcode147
Documentation/learning/common-notation.itely:147: a1 |
This example doesn't need a bar check.

http://codereview.appspot.com/1056041/diff/24001/25001#newcode541
Documentation/learning/common-notation.itely:541: c2 \grace { a32[ b] }
c2 |
I don't think we need bar checks here, either.

http://codereview.appspot.com/1056041/diff/24001/25001#newcode1088
Documentation/learning/common-notation.itely:1088: d4 | g4 g a8( b) | g4
g b8( c) |
Since the first \partial example puts the note on the same line as the
\partial, let's do the same thing here:
  \partial 4 d4 |

http://codereview.appspot.com/1056041/diff/24001/25001#newcode1359
Documentation/learning/common-notation.itely:1359: }
On 2010/05/06 08:48:46, Trevor Daniels wrote:
> I agree with Carl and Graham - it looked better as it was originally,
on single
> lines.  It is a scale, not barred music, although I've no problem with
the bar
> checks.

I agree with Trevor agreeing with me.  :)

http://codereview.appspot.com/1056041/diff/24001/25001#newcode1370
Documentation/learning/common-notation.itely:1370: b'8. cis''16 b'8 d''4
d''8 |
For the record, these bar checks are very good.

http://codereview.appspot.com/1056041/diff/24001/25002
File Documentation/learning/fundamental.itely (right):

http://codereview.appspot.com/1056041/diff/24001/25002#newcode239
Documentation/learning/fundamental.itely:239: @code{keyTime},
@code{pianorighthand}, or @code{foofoobarbaz}.
Could we change the foofoobarbaz to something else?  In general, I'd
rather avoid programming jokes in the docs.

(I know that I've added "foo" a few times, but I try not to... this is
something on my todo list for GDP2, but since you're changing this spot
anyway...)

http://codereview.appspot.com/1056041/diff/24001/25002#newcode1255
Documentation/learning/fundamental.itely:1255: }
On 2010/05/06 08:48:46, Trevor Daniels wrote:
> I think this is one case where the final bar check should be added.

+1

http://codereview.appspot.com/1056041/diff/24001/25002#newcode1304
Documentation/learning/fundamental.itely:1304: AltoMusic  = \relative c'
{ c4 | c4. c8 e4  e    | f4   f   e  }
Same here -- please keep the final barline check.

http://codereview.appspot.com/1056041/diff/24001/25002#newcode1702
Documentation/learning/fundamental.itely:1702: \key g \minor
On 2010/05/06 08:48:46, Trevor Daniels wrote:
> Since we placed \clef "treble" in the LM I don't remember any
questions on -user
> about G_8 failing.  We're adding many superfluous braces, quotes round
variable
> names, etc, why leave these out?  But if the quotes are to be removed
we need to
> add a remark somewhere about them being required sometimes.

We already have that remark in Notation.  I'm not too fussed whether we
have quotes or not, but it _does_ seem weird to remove the quotes if
they're already there.

http://codereview.appspot.com/1056041/diff/24001/25002#newcode2481
Documentation/learning/fundamental.itely:2481: <<
Indentation mistake.

http://codereview.appspot.com/1056041/diff/24001/25002#newcode2915
Documentation/learning/fundamental.itely:2915: \key c \minor
I think this one can be condensed onto one line as well.

http://codereview.appspot.com/1056041/diff/24001/25002#newcode3053
Documentation/learning/fundamental.itely:3053: \key c \minor
Condense.

http://codereview.appspot.com/1056041/diff/24001/25002#newcode3133
Documentation/learning/fundamental.itely:3133: \fragmentA \fragmentA |
Oooh, I really like the barline check there.  It hadn't occurred to me
you could do that.

http://codereview.appspot.com/1056041/diff/24001/25002#newcode3163
Documentation/learning/fundamental.itely:3163: }
On 2010/05/06 08:48:46, Trevor Daniels wrote:
> There's no point in fiddling with this example - it needs replacing.

+1
that said, there's also no point debating the formatting.  :)

http://codereview.appspot.com/1056041/diff/24001/25002#newcode3249
Documentation/learning/fundamental.itely:3249: cis4 f |
I don't see the point of exapanding these examples, but I won't insist
that you change them back.

http://codereview.appspot.com/1056041/diff/24001/25002#newcode3352
Documentation/learning/fundamental.itely:3352: r4 f8 a |
Actually, I take that back -- this example looks more complicated than
it needs to be.  I think it looked better in the original version, and
since it's comprised of sticking together the previous examples, those
should also be retained in "condensed" form.

http://codereview.appspot.com/1056041/diff/24001/25004
File Documentation/learning/tweaks.itely (right):

http://codereview.appspot.com/1056041/diff/24001/25004#newcode388
Documentation/learning/tweaks.itely:388: a4^Black
sweet mao, this actually works?!  ick.

Please add "" around the "Black".  Ditto for Red and Green.

http://codereview.appspot.com/1056041/diff/24001/25004#newcode1651
Documentation/learning/tweaks.itely:1651: c2^"Text3" c^"Text4" |
On 2010/05/06 08:48:46, Trevor Daniels wrote:
> I think in this case the original layout illustrated the point more
clearly

+1

http://codereview.appspot.com/1056041/show