Mark, I think this is a great start, and will greatly help.
The major issues I see are (1) repeating information (i.e. the meanings of space, minimum-distance, padding, and stretchability), and (2) introducing exhaustive lists into the NR. Repeating information is prohibited by policy because it becomes very difficult to keep two parallel sections in synch. Exhaustive lists belong in the IR, instead of the NR. THat way they are automatically updated. Other than that, I have some editorial comments. Great work! Thanks, Carl http://codereview.appspot.com/2642043/diff/1/Documentation/notation/spacing.itely File Documentation/notation/spacing.itely (left): http://codereview.appspot.com/2642043/diff/1/Documentation/notation/spacing.itely#oldcode1697 Documentation/notation/spacing.itely:1697: A non-staff line at the bottom of a system should have I think that this section should not be removed. http://codereview.appspot.com/2642043/diff/1/Documentation/notation/spacing.itely File Documentation/notation/spacing.itely (right): http://codereview.appspot.com/2642043/diff/1/Documentation/notation/spacing.itely#newcode1501 Documentation/notation/spacing.itely:1501: @item @emph{staff-like contexts} (such as @code{Lyrics}). I would change this to "non-staff" contexts, I think. I liked the previous terminology. http://codereview.appspot.com/2642043/diff/1/Documentation/notation/spacing.itely#newcode1506 Documentation/notation/spacing.itely:1506: I think that you should explain the different grobs that are created by each of the contexts at this point. I got lost while I was waiting. I'd like to see: Staff contexts create VerticalAxisGrouper grobs StaffGroup contexts create StaffGrouper grobs non-staff contexts create VerticalAxisGrouper grobs Or maybe the word is "result in" instead of create. http://codereview.appspot.com/2642043/diff/1/Documentation/notation/spacing.itely#newcode1510 Documentation/notation/spacing.itely:1510: the staves. WHen do the staff-groups get spaced? http://codereview.appspot.com/2642043/diff/1/Documentation/notation/spacing.itely#newcode1529 Documentation/notation/spacing.itely:1529: @subsubheading Structure of spacing alists for grob properties "Structure of alists for vertical spacing properties" is a better name, I think. http://codereview.appspot.com/2642043/diff/1/Documentation/notation/spacing.itely#newcode1554 Documentation/notation/spacing.itely:1554: @item @code{minimum-distance} -- the minimum required vertical I think I would change "minimum required" to "minimum allowable" http://codereview.appspot.com/2642043/diff/1/Documentation/notation/spacing.itely#newcode1578 Documentation/notation/spacing.itely:1578: @subsubheading Modifying spacing alists for grob properties Reference to the between system spacing, instead of repeating. Just introduce the new syntax as it applies to the in-system contexts. http://codereview.appspot.com/2642043/diff/1/Documentation/notation/spacing.itely#newcode1814 Documentation/notation/spacing.itely:1814: @item @code{ChoirStaff} In general, we don't want to make exhaustive lists in the docs, because maintaining them is a problem. http://codereview.appspot.com/2642043/diff/1/Documentation/notation/spacing.itely#newcode1836 Documentation/notation/spacing.itely:1836: The default value of @code{next-staff-spacing} is a callback This is too much talking through the code, I think. Better to just say next-staff-spacing is a procedure that will return after-last-staff-spacing if the staff is the last staff of a group, the between-staff-spacing if the staff is not the last staff of a group, and the default-next-staff-spacing if the staff is not part of a group. http://codereview.appspot.com/2642043/diff/1/Documentation/notation/spacing.itely#newcode1881 Documentation/notation/spacing.itely:1881: @item @code{ChordNames} I don't think we want exhaustive lists. (But Graham will correct me if I'm wrong.) http://codereview.appspot.com/2642043/diff/1/Documentation/notation/spacing.itely#newcode1897 Documentation/notation/spacing.itely:1897: @item @code{inter-loose-line-spacing} Since the property name is inter-loose-line-spacing, we may want to call these contexts "loose line contexts" instead of staff-like or non-staff contexts. http://codereview.appspot.com/2642043/ _______________________________________________ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel