[frogs] Re: T1247 - Conditionally do (use-modules (ice-9 curried-definitions)) if running with Guile V2, (issue2219044) |
[ Thread Index |
Date Index
| More lilynet.net/frogs Archives
]
I like the code for choosing Guile V2.
I'm concerned about a bunch of the re-indentation changes.
Thanks,
Carl
http://codereview.appspot.com/2219044/diff/1/scm/lily.scm
File scm/lily.scm (right):
http://codereview.appspot.com/2219044/diff/1/scm/lily.scm#newcode197
scm/lily.scm:197: ;(set-debug-cell-accesses! 1000)
Why is this shoved over to the right?
http://codereview.appspot.com/2219044/diff/1/scm/lily.scm#newcode199
scm/lily.scm:199: ;;; Boolean thunk - are we integrating Guile V2.0 or
higher with Lilypond?
It should either be LilyPond (the application) or lilypond (the
executable).
I don't think it should ever be Lilypond.
http://codereview.appspot.com/2219044/diff/1/scm/lily.scm#newcode204
scm/lily.scm:204: (ice-9 safe)
These should all be aligned with (ice-9 regex) as in the old code.
http://codereview.appspot.com/2219044/diff/1/scm/lily.scm#newcode300
scm/lily.scm:300: (vector-ref (uname) 0) char-set:letter+digit)))
(vector-ref should be indented; it's an argument to string-tokenize.
http://codereview.appspot.com/2219044/diff/1/scm/lily.scm#newcode337
scm/lily.scm:337: iface))
Looks like there are trailing spaces here?
http://codereview.appspot.com/2219044/diff/1/scm/lily.scm#newcode568
scm/lily.scm:568: r5rs-secondary-predicates
Why the move from spaces to tabs? As far as I know, the tabs-vs-spaces
discussion is not settled. Given that it's not, I don't think we should
be changing existing code when that's the only change.
http://codereview.appspot.com/2219044/diff/1/scm/lily.scm#newcode829
scm/lily.scm:829: (ly:stderr-redirect (format "~a.log" base) "w"))
The previous indentation here was correct. Then clause should be
aligned with if clause
http://codereview.appspot.com/2219044/diff/1/scm/lily.scm#newcode831
scm/lily.scm:831: (format ping-log "Processing ~a\n" base))
Same here
http://codereview.appspot.com/2219044/diff/1/scm/lily.scm#newcode848
scm/lily.scm:848: (ly:reset-all-fonts))))
Else clause should be aligned with if clause.
http://codereview.appspot.com/2219044/
---
----
Join the Frogs!