lilypond-devel
[Top][All Lists]
Advanced

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

Add scripts/auxiliar/guileindent.scm (issue 4896043)


From: Carl . D . Sorensen
Subject: Add scripts/auxiliar/guileindent.scm (issue 4896043)
Date: Sun, 14 Aug 2011 03:15:34 +0000

Reviewers: Neil Puttock, Reinhold, Graham Percival,

Message:
Here's the latest version of guileindent.scm.

It's been added to scripts/auxiliar.

After doing a tab->space conversion on scm/music-functions.scm, I ran it
through guileindent.scm.

I've made comments about the places where the spacing looks like it
might be questionable.

In general, I think that virtually all of the changes are an improvement
on the original spacing.

Thanks,

Carl



http://codereview.appspot.com/4896043/diff/1/scm/music-functions.scm
File scm/music-functions.scm (right):

http://codereview.appspot.com/4896043/diff/1/scm/music-functions.scm#newcode673
scm/music-functions.scm:673: (make-music 'RestEvent 'duration
(ly:music-property mus 'duration))
This is misaligned because there are two spaces after the if.  It would
be better to have the script automatically change the two spaces to one.

Alternatively, perhaps the (make-music  should be lined up with the
(memq.

http://codereview.appspot.com/4896043/diff/1/scm/music-functions.scm#newcode752
scm/music-functions.scm:752: (set! new-settings (delete x
new-settings)))
This is too much indentation.  I'll need to figure out why. It worked
well at line 157.

http://codereview.appspot.com/4896043/diff/1/scm/music-functions.scm#newcode973
scm/music-functions.scm:973: )))
I think this is an improvement?

http://codereview.appspot.com/4896043/diff/1/scm/music-functions.scm#newcode1054
scm/music-functions.scm:1054: ;; If no key signature match is found from
localKeySignature, we may have a custom
Moving the comments in to the indent level of the code is probably a
good thing, I think.

http://codereview.appspot.com/4896043/diff/1/scm/music-functions.scm#newcode1279
scm/music-functions.scm:1279: Staff ,(make-accidental-rule 'same-octave
0)
The original spacing is non-standard Scheme spacing, and would normally
not be allowed.  In this case, it probably makes sense.

This indicates to me that the structure for the argument to this
function should probably be revised, rather than just being a long flat
list.

http://codereview.appspot.com/4896043/diff/1/scm/music-functions.scm#newcode1282
scm/music-functions.scm:1282: ,neo-modern-accidental-rule)
Again, this non-standard spacing indicates that the argument list should
have some kind of structure, I think.

http://codereview.appspot.com/4896043/diff/1/scm/music-functions.scm#newcode1353
scm/music-functions.scm:1353: `(Staff ,(make-accidental-rule
'same-octave 0))
Here, with the structure in the argument list, everything works out
well.

http://codereview.appspot.com/4896043/diff/1/scm/music-functions.scm#newcode1470
scm/music-functions.scm:1470: (lambda (m) (eq? 'NoteEvent
(ly:music-property m 'name)))
Should filter have a two-space indent?  I can easily add it if desired.

Description:
Add scripts/auxiliar/guileindent.scm

Also, run guileindent.scm on scm/music-functions.scm after
first expanding all tabs to spaces.

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

Affected files:
  M scm/music-functions.scm
  A scripts/auxiliar/guileindent.scm





reply via email to

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