lilypond-devel
[Top][All Lists]
Advanced

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

Re: Doc: NR: Reformat ly code. (issue1242044)


From: percival . music . ca
Subject: Re: Doc: NR: Reformat ly code. (issue1242044)
Date: Tue, 25 May 2010 18:52:01 +0000

I managed to trudge through another 4 or 5 files.  Look, this is
ridiculous.  I'm officially rejecting this patch because it's **WAY**
too long for anybody to look at.

Please break this into separate patches, where each patch is 1500 lines
or less.  If a single file has more than 1500 diff-lines, you can exceed
the 1500-line limit in order to send a patch for that file.


Start from pitches.itely, and go in order of appearance in the NR.
Don't submit a new patch until the previous one is accepted and
committed.

I'm sorry that this makes a lot more work for you, but I tried to warn
you after the LM-patch.  As we state on the "doc additions and
suggestions" page, **ask** before undertaking a huge doc-writing
project.



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
On 2010/05/22 18:08:30, Mark Polesky wrote:
On 2010/05/22 08:10:42, Graham Percival wrote:
> err, why?

why what?

... um.  I have to admit that I can't remember.  Sorry.

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 }
On 2010/05/22 18:08:30, Mark Polesky wrote:
The current version uses \StemDown and \StemUp for the main
voice, and relies on the cue pitches for its stem
directions, which all happen to point up in this particular
case.  I think the \voiceOne (etc.) commands are preferable
here.

Oops, I missed that.  Yeah, \voiceEtc is definitely better than
\stemEtc.

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

http://codereview.appspot.com/1242044/diff/30002/34003#newcode60
Documentation/notation/chords.itely:60: \chordmode { c1 | g1 | a1 | g1 |
c1 | }
Ick, this looks much harder to understand.  As I said earlier, I'm not
going to insist that you remove this change, but I really hope that you
don't continue working on stuff like this.  It will be discussed during
GLISS:
http://lilypond.org/~graham/gliss/specifics.html

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

http://codereview.appspot.com/1242044/diff/30002/34005#newcode151
Documentation/notation/expressive.itely:151: c4-> c-. c2-_ |
Could you switch the order of the c-| and c->   ?  Having the barline
right after c-|  doesn't look good.  Also, could you line the second
barline under the first?  That would help the eye distinguish the
barline-punctuation from the accent-punctuation.

http://codereview.appspot.com/1242044/diff/30002/34005#newcode265
Documentation/notation/expressive.itely:265: c2_\spp c^\ff |
Hmm.  It might be better if a barline check at the end of the line
always had two (or more) spaces before it, and if they lined up whenever
possible.

Don't change anything now, but that's definitely an idea to discuss in
GLISS.  I've added it to the list.

http://codereview.appspot.com/1242044/diff/30002/34005#newcode320
Documentation/notation/expressive.itely:320: c2 b4 a | g1\espressivo |
I think this looked better on two lines.

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

http://codereview.appspot.com/1242044/diff/30002/34006#newcode315
Documentation/notation/fretted-strings.itely:315: \relative c' {
the \relative should be on the same line as the "ties =".

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



reply via email to

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