Re: Doc: NR 4.4.1: Rewrite. (issue2642043)

[Carl . D . Sorensen]

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/