Re: vert. spacing: Rename properties (lily, scm). (issue3031041)

[Carl . D . Sorensen]

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/