Add scripts/auxiliar/guileindent.scm

Add scripts/auxiliar/guileindent.scm

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

[Carl, 2011-08-14 03:15:33]

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.

[Trevor Daniels, 2011-08-14 07:48:48]

L pretty GTM.
It would look even better with a little manual adjustment first.

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))
remove the double space, I think

http://codereview.appspot.com/4896043/diff/1/scm/music-functions.scm#newcode973
scm/music-functions.scm:973: )))
Is it out of the question to move orphaned brackets to the previous line?

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)))
Same question for bare (if and (list
I'd be happy with 1 space in this case and 2 spaces when the function is not
bare [as indent seems to do for all bare (function calls].  Don't know what
emacs does though.

[Carl, 2011-08-14 21:45:23]

New comments, and the results of fixing some non-indent spacing issues in the
source file.

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#newcode752
scm/music-functions.scm:752: (set! new-settings (delete x new-settings)))
On 2011/08/14 03:15:34, Carl wrote:
> This is too much indentation.  I'll need to figure out why. It worked well at
> line 157.

I think it's the missing space between lambda and (x).  When I added it, the
indentation worked properly.

http://codereview.appspot.com/4896043/diff/1/scm/music-functions.scm#newcode973
scm/music-functions.scm:973: )))
On 2011/08/14 07:48:48, Trevor Daniels wrote:
> Is it out of the question to move orphaned brackets to the previous line?

I think we don't want to do that in general.  The only time we want to have
orphaned brackets (parentheses in the US) is if there is a list that may be
added to.  In that case, the orphaned brackets should be all the brackets that
are not part of a list entry.

In this case, I think the first bracket on this line should be attached to the
end of the line before (to close off the labmda form).  Then the final orphaned
brackets would line up properly with the list of lambda forms.

[Neil Puttock, 2011-08-14 22:37:51]

Hi Carl,

This latest patch looks great.

I'm not sold on the change to `else', but that's mainly due to being used to how
Emacs does it.

I'll do a bit more testing on other files and report back.

Cheers,
Neil

[Graham Percival, 2011-08-15 09:06:00]

Looks awesome; I'm going to make this the next GOP-PROP issue.  It
should just be a rubber-stamping affair, but that's the best kind
of proposal to get through the systme.

I'm thinking of a grand fix-scheme.py run on 2011 Sep 01.  It
won't mess up many patches, and in any case the C++ run went
amazingly smoothly, so there isn't much point in delaying this
run.

Cheers,
- Graham

[Carl, 2011-08-16 00:19:03]

New patch set uploaded with driver script and ignoring detabify output.

Neil, what behavior would you like to see for else?

THanks,

Carl

[Neil Puttock, 2011-09-15 23:47:37]

http://codereview.appspot.com/4896043/diff/10002/scripts/auxiliar/guileindent...
File scripts/auxiliar/guileindent.scm (right):

http://codereview.appspot.com/4896043/diff/10002/scripts/auxiliar/guileindent...
scripts/auxiliar/guileindent.scm:38: (define (assf test-proc search-list)
move to lily-library.scm

http://codereview.appspot.com/4896043/diff/10002/scripts/auxiliar/guileindent...
scripts/auxiliar/guileindent.scm:65: '(block
some of these aren't Guile keywords

http://codereview.appspot.com/4896043/diff/10002/scripts/auxiliar/guileindent...
scripts/auxiliar/guileindent.scm:71: '(case
some of these aren't Guile keywords

[Neil Puttock, 2011-09-15 23:51:21]

lambda* should have same indentation as lambda

comments are broken:

; (margin comment) should be indented (not sure how many spaces emacs does it
though)

;;; (top-level comment) shouldn't be indented even inside a form

There are a few files which use margin comments by mistake, so we should fix
them ideally (e.g., define-markup-commands.scm, slashed-digit-internal).