lilypond-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Doc: NR: Reformat ly code. (issue1242044)


From: percival . music . ca
Subject: Doc: NR: Reformat ly code. (issue1242044)
Date: Sat, 22 May 2010 08:10:42 +0000

Well, there's 40 minutes of my 60 minutes of lilypond work for today
done.  What's the first thing we tell new doc contributors?  Send small
patches.  I think this applies here.


http://codereview.appspot.com/1242044/diff/2001/3001
File Documentation/notation/changing-defaults.itely (right):

http://codereview.appspot.com/1242044/diff/2001/3001#newcode356
Documentation/notation/changing-defaults.itely:356: music = \relative
c'' @{ c4 c2. @}
Doc policy forbids this kind of @example ... @lilypond construct, and I
see no reason to override it for pedagogical reasons here.

http://codereview.appspot.com/1242044/diff/2001/3001#newcode1675
Documentation/notation/changing-defaults.itely:1675: {
Doc policy is to omit useless {} around an entire @lilypond block if we
have relative=2, which we do.

Doc policy is also to omit [fragment] if it's useless, and it's useless
if we have [relative].

http://codereview.appspot.com/1242044/diff/2001/3003
File Documentation/notation/chords.itely (right):

http://codereview.appspot.com/1242044/diff/2001/3003#newcode1019
Documentation/notation/chords.itely:1019: \new Staff = myStaff
Shouldn't we have a { at the end of this line?

http://codereview.appspot.com/1242044/diff/2001/3003#newcode1128
Documentation/notation/chords.itely:1128: \figures {
Indentation mistake.

http://codereview.appspot.com/1242044/diff/2001/3004
File Documentation/notation/editorial.itely (right):

http://codereview.appspot.com/1242044/diff/2001/3004#newcode677
Documentation/notation/editorial.itely:677: c2\startGroup d\stopGroup
It's more clear the other way.  Could you change it to c1 d1 instead?

http://codereview.appspot.com/1242044/diff/2001/3005
File Documentation/notation/expressive.itely (right):

http://codereview.appspot.com/1242044/diff/2001/3005#newcode291
Documentation/notation/expressive.itely:291: c2\< c\!  | d2\< d\f |
Why make this change?  I thought it was fine the other way.

http://codereview.appspot.com/1242044/diff/2001/3005#newcode316
Documentation/notation/expressive.itely:316: c2 b4 a | g1\espressivo |
I'm ok with this change.

http://codereview.appspot.com/1242044/diff/2001/3005#newcode467
Documentation/notation/expressive.itely:467: \dynamic f
Don't invent a new indentation standard.  This is lilypond code; we use
two-space indents.  Don't suddenly pretend it's scheme.

http://codereview.appspot.com/1242044/diff/2001/3005#newcode504
Documentation/notation/expressive.itely:504: \dynamic f
ditto.

http://codereview.appspot.com/1242044/diff/2001/3005#newcode532
Documentation/notation/expressive.itely:532: (markup #:normal-text
"molto" #:dynamic "f"))
Weren't you the person suggesting that we standardize on
foo =
#(scheme-stuff

?!

http://codereview.appspot.com/1242044/diff/2001/3005#newcode583
Documentation/notation/expressive.itely:583: f4( g a) a8 b( | a4 g2 f4)
| <c e>2( <b d>2) |
I thought the original was fine.

http://codereview.appspot.com/1242044/diff/2001/3006
File Documentation/notation/fretted-strings.itely (right):

http://codereview.appspot.com/1242044/diff/2001/3006#newcode1340
Documentation/notation/fretted-strings.itely:1340: b16 d g b
Are you a guitarist?  I'm loathe to change an example that a skilled
instrumentalist sent to me in response to a question about the proper
use of guitar barre position.

I mean, does it make sense to start the barre on the b16 instead of the
g16, as before?

... oh wait, I see.  Err, this is worse.  IMO the override is better if
it's right before the relevant command.

http://codereview.appspot.com/1242044/diff/2001/3006#newcode1372
Documentation/notation/fretted-strings.itely:1372: d^\markup { \italic {
\fontsize #-2 { "harm. 12" }}} <g b>2.
shouldn't the d be d4?

... doing this manually is really stupid.  It would take 10 lines of
python to automatically check for missing durations in the docs.
no wait, I forgot about chords.  ok, 15 lines.

http://codereview.appspot.com/1242044/diff/2001/3007
File Documentation/notation/input.itely (right):

http://codereview.appspot.com/1242044/diff/2001/3007#newcode1433
Documentation/notation/input.itely:1433: c8 d b bes a g | c2 r |
I don't think the c2 r adds anything to the example; just delete it.

http://codereview.appspot.com/1242044/diff/2001/3010
File Documentation/notation/pitches.itely (right):

http://codereview.appspot.com/1242044/diff/2001/3010#newcode2724
Documentation/notation/pitches.itely:2724: \aikenHeadsMinor
err, why?

http://codereview.appspot.com/1242044/diff/2001/3012
File Documentation/notation/rhythms.itely (right):

http://codereview.appspot.com/1242044/diff/2001/3012#newcode785
Documentation/notation/rhythms.itely:785: \time 3/4  R2. | R2.*2 |
No.  \time should be on a line by itself.

http://codereview.appspot.com/1242044/diff/2001/3012#newcode795
Documentation/notation/rhythms.itely:795: \time 6/4 R1*3/2 |
No; \time goes by itself.

http://codereview.appspot.com/1242044/diff/2001/3012#newcode1326
Documentation/notation/rhythms.itely:1326: \new Staff { \time 3/4 c4 c c
| c4 c c | }
unless \time occurs in a condensed passage; this one's fine.

http://codereview.appspot.com/1242044/diff/2001/3012#newcode1883
Documentation/notation/rhythms.itely:1883: \time 2/4 c8 c\noBeam c c
line by itself.

http://codereview.appspot.com/1242044/diff/2001/3012#newcode2475
Documentation/notation/rhythms.itely:2475: c1 |
These two examples should be the same; you've made them 4 c and 5 c.
Don't.

http://codereview.appspot.com/1242044/diff/2001/3014
File Documentation/notation/spacing.itely (right):

http://codereview.appspot.com/1242044/diff/2001/3014#newcode708
Documentation/notation/spacing.itely:708:
doc policy forbids text running directly into an example; use some kind
of punctuation.  see the list in the CG.

http://codereview.appspot.com/1242044/diff/2001/3014#newcode718
Documentation/notation/spacing.itely:718: \header { tagline = ##f }
our semi-official order of sections places \header before \paper.

http://codereview.appspot.com/1242044/diff/2001/3014#newcode1611
Documentation/notation/spacing.itely:1611: \new Staff @{ c @}
c4

http://codereview.appspot.com/1242044/diff/2001/3014#newcode2215
Documentation/notation/spacing.itely:2215: c8[ c \clef "alto" c8 \grace
{ c16[ c] } c8 c c] c32[ c]
Aren't clefs supposed to have their own line?

http://codereview.appspot.com/1242044/diff/2001/3015
File Documentation/notation/staff.itely (right):

http://codereview.appspot.com/1242044/diff/2001/3015#newcode1373
Documentation/notation/staff.itely:1373: { \voiceTwo d4. bes4 c8 }
WTM are the \voiceTwo and \voiceOne doing here?  Do they contribute to a
tiny example?

http://codereview.appspot.com/1242044/diff/2001/3018
File Documentation/notation/vocal.itely (right):

http://codereview.appspot.com/1242044/diff/2001/3018#newcode9
Documentation/notation/vocal.itely:9:
This whole file is a mess, doesn't follow doc policy, and is slated for
a compelete rewrite, so I won't bother looking at changes here.

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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]