Thanks again for your work on this, Mark. I think the spacing-alist description belongs in one of the grob properties; it's the value of a property, not a property of an interface.
I much prefer this to the previous patch. I had an idea about the name of the StaffGrouper staff-staff-spacing property, when I was reading (and being confused about) how it worked. See my comment below. If you disagree with any of these suggestions, I'm happy to have a discussion. I don't want to force my opinion on any of these. Thanks, Carl http://codereview.appspot.com/3031041/diff/40001/lily/axis-group-interface.cc File lily/axis-group-interface.cc (right): http://codereview.appspot.com/3031041/diff/40001/lily/axis-group-interface.cc#newcode780 lily/axis-group-interface.cc:780: "The following keys may be set in the spacing alists:\n" I'd prefer to have the keys described under the staff-staff-spacing grob property, I think. http://codereview.appspot.com/3031041/diff/40001/lily/staff-grouper-interface.cc File lily/staff-grouper-interface.cc (right): http://codereview.appspot.com/3031041/diff/40001/lily/staff-grouper-interface.cc#newcode42 lily/staff-grouper-interface.cc:42: "The following keys may be set in the spacing alists:\n" As with the axis-group interface, I think the spacing-alist description belongs in the description of the staff-staff-spacing property (or perhaps the default-staff-staff-spacing property), rather than in the interface. http://codereview.appspot.com/3031041/diff/40001/scm/define-grob-properties.scm File scm/define-grob-properties.scm (right): http://codereview.appspot.com/3031041/diff/40001/scm/define-grob-properties.scm#newcode587 scm/define-grob-properties.scm:587: (nonstaff-nonstaff-spacing ,list? "The flexible distance Instead of "The flexible distance", I think I'd say "The spacing alist controlling the distance", with a reference that says "see staff-staff-spacing for a description of a spacing alist." http://codereview.appspot.com/3031041/diff/40001/scm/define-grob-properties.scm#newcode592 scm/define-grob-properties.scm:592: (nonstaff-relatedstaff-spacing ,list? "The flexible distance Again, s/flexible/spacing alist controlling/ http://codereview.appspot.com/3031041/diff/40001/scm/define-grob-properties.scm#newcode772 scm/define-grob-properties.scm:772: two staves. When applied to a @code{StaffGrouper} grob, Trevor suggested "layout object" instead of "grob". While it's longer, I think it may be clearer here. http://codereview.appspot.com/3031041/diff/40001/scm/define-grob-properties.scm#newcode774 scm/define-grob-properties.scm:774: @code{VerticalAxisGroup} grob, @q{region} refers to an entire This is confusing to me, because it seemed on first reading to imply that a setting of staff-staff-spacing for a VerticalAxisGroup would apply to all staves in a system, rather than just to the current staff. The challenge is that this is true within a StaffGrouper. To a certain extent, setting staff-staff-spacing for the StaffGrouper is equivalent to setting default-staff-staff-spacing for all the staves in the group except for the last one. Hmm -- maybe instead of calling it staff-staff-spacing, we should call it default-staff-staff-spacing. Then we'd have a system wide default-staff-staff-spacing, a staffgroup default-staff-staff-spacing, and individual staff staff-staff-spacing. Specified individual staff staff-staff-spacing always overrides the defaults, and the staffgroup default overrides the system wide default. Also, here is where I think I would like to see the description of a spacing-alist. http://codereview.appspot.com/3031041/diff/40001/scm/define-grob-properties.scm#newcode788 scm/define-grob-properties.scm:788: @code{VerticalAxisGroup} grob will be used instead for any staff Is this true for *any* staff, or just for the *last* staff? In my mind, staffgroup-staff-spacing is the spacing between the last staff of the group and the next staff below. http://codereview.appspot.com/3031041/diff/40001/scm/define-grob-properties.scm#newcode790 scm/define-grob-properties.scm:790: @code{staffgroup-staff-spacing} and @code{staff-staff-spacing} (of I think this explanation should just be part of the default-staff-staff-spacing property, rather than being listed in multiple places. http://codereview.appspot.com/3031041/ _______________________________________________ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
