add property label-alignments to OttavaBracket

add property label-alignments to OttavaBracket

[lemzwerg, 2019-11-21 13:48:37]

Very nice, LGTM!

[Carl, 2019-11-21 14:29:35]

I've added a comment about the docstring -- it's a request for consideration,
not a demand for change.

Also, I think a changes.tely entry should be included.

https://codereview.appspot.com/575330043/diff/551180043/scm/define-grob-prope...
File scm/define-grob-properties.scm (right):

https://codereview.appspot.com/575330043/diff/551180043/scm/define-grob-prope...
scm/define-grob-properties.scm:573: (label-alignments ,number-pair? "The
vertical alignments of OttavaBracket
In general, we like to have properties that are not defined for specific grobs
so that they can be reused.  This helps keep the namespace manageable.

Are there other grobs that have labels?  For example, Volta brackets?  Nobody I
know would want to put a Volta bracket below the staff, but with people
regularly inventing new notation I can never say never ...  Or maybe line
spanners?

I'm raising this concern not because I want to have this property applied to any
other interface as part of this patch, but because I wonder if the property
description should  say "The vertical alignment of a grob label when the grob is
placed below and above the staff", thus removing the OttaveBracket part from the
definition.  In our code, we know it applies to an OttavaBracket because we
override OttavaBracket.label-alignments.

This is not a requirement for change, just a request for consideration.

[Malte Meyn, 2019-11-22 12:56:00]

Hm, I think you’re right. I’m not sure how interface work but maybe this is a
case for the line-spanner-interface?

I just managed to do the same thing for VoltaBrackets but that did need some
code duplication so maybe there should be a new function in some interface for
that alignment … I’ll have a look at this in the next few days.

[thomasmorley651, 2019-11-22 20:34:32]

What about a regtest?

[Malte Meyn, 2019-11-23 11:18:10]

On 2019/11/22 12:56:00, Malte Meyn wrote:
> I just managed to do the same thing for VoltaBrackets but that did need some
> code duplication so maybe there should be a new function in some interface for
> that alignment … I’ll have a look at this in the next few days.

For Spanners with only one label like OttavaBracket or VoltaBracket this would
work.

All sorts of piano pedal spanners have no label as part of the spanner grob
(PianoPedalBracket) but a separate grob (SostenutoPedal, SustainPedal,
UnaCordaPedal). Why?

And then there are some Spanners which can have labels on both sides (left and
right) like TextSpanner and DynamicTextSpanner. These have the subproperties
bound-details.(left|right).stencil-align-dir-y that take only one number.

I would suggest to:
• add the label-alignments property to all spanners with exactly one label
• think about all those different piano pedal grobs, whether they are needed,
and how alignment could be done between the label grob and the line grob
• rename stencil-align-dir-y to label-alignments and let it take a pair of
numbers (is this good, to have a top-level property for some grobs with the same
name as a subproperty for others?)

Here you can see the current situation:

\version "2.21.0"

\relative {
  \override DynamicTextSpanner.color = #blue
  \override DynamicTextSpanner.bound-details.left.text = "foo"
  \override DynamicTextSpanner.bound-details.left.stencil-align-dir-y = #UP
  \override DynamicTextSpanner.bound-details.right.text = "bar"
  
  \set Staff.pedalSustainStyle = #'mixed
  \override Staff.SostenutoPedal.color = #green
  \override Staff.SustainPedal.color = #red
  \override Staff.PianoPedalBracket.color = #magenta
  
  % no effect:
  \override Staff.SustainPedal.bound-details.left.text = "foo"
  \override Staff.SustainPedal.bound-details.left.stencil-align-dir-y = #UP
  \override Staff.SustainPedal.bound-details.right.text = "bar"
  
  c'\cresc\sostenutoOn\sustainOn
  d e f g\!\sostenutoOff\sustainOff
}

[Malte Meyn, 2019-11-27 05:40:16]

On 2019/11/23 11:18:10, Malte Meyn wrote:
> I would suggest to:
> • add the label-alignments property to all spanners with exactly one label
> • think about all those different piano pedal grobs, whether they are needed,
> and how alignment could be done between the label grob and the line grob
> • rename stencil-align-dir-y to label-alignments and let it take a pair of
> numbers (is this good, to have a top-level property for some grobs with the
same
> name as a subproperty for others?)

Any opinions, helpful thoughts?

[c_sorensen, 2019-11-27 05:52:27]

On 11/26/19, 10:40 PM, "lilypond@maltemeyn.de" <lilypond@maltemeyn.de> wrote:

    On 2019/11/23 11:18:10, Malte Meyn wrote:
    > I would suggest to:
    > • add the label-alignments property to all spanners with exactly one
    label
    > • think about all those different piano pedal grobs, whether they are
    needed,
    > and how alignment could be done between the label grob and the line
    grob
    > • rename stencil-align-dir-y to label-alignments and let it take a
    pair of
    > numbers (is this good, to have a top-level property for some grobs
    with the same
    > name as a subproperty for others?)
    
    Any opinions, helpful thoughts?

I don't know if it's helpful, but if it provides the same functionality, I think
it should have the same name, regardless of whether it's a top-level property or
subproperty.

I think it's great that you are taking on the bigger picture.  My initial
comment was not trying to get you to take on the bigger picture; it was just to
make sure that the docstring for label-alignments wasn't limited to
OttavaBrackets.  Considering other spanners and trying to rationalize all the
labeling is a great contribution, beyond my expectation.

Thanks,

Carl

    
    https://codereview.appspot.com/575330043/
    

[Malte Meyn, 2020-03-29 14:01:31]

label-alignments for VoltaBrackets and line spanners, changes entry

[Malte Meyn, 2020-03-29 14:04:09]

On 2020/03/29 14:01:31, Malte Meyn wrote:
> label-alignments for VoltaBrackets and line spanners, changes entry

This needs a regtest and a convert-ly rule. Can someone help me with the latter?
All \overrides etc. have to change stencil-align-dir-y = NUMBER to
label-alignments = NUMBER-PAIR.

Also, I’m not sure whether stencil.cc is the best place for the align_label
function.

[lemzwerg, 2020-03-29 14:23:29]

Minor doc nits.

https://codereview.appspot.com/575330043/diff/549780043/Documentation/changes...
File Documentation/changes.tely (right):

https://codereview.appspot.com/575330043/diff/549780043/Documentation/changes...
Documentation/changes.tely:67: @item The labels can be changed by setting the
context property @code{ottavationMarkups}. The new default is numbers only.
Please stay within 78 characters per line.  And please use two spaces after a
full stop.

https://codereview.appspot.com/575330043/diff/549780043/Documentation/changes...
Documentation/changes.tely:70: @item The labels can be vertically aligned
differently for brackets below/above the staff by setting the grob property
@code{label-alignments}.
If you insist on using '/' (instead of simply using the word 'or'), you should
add '@/' after the slash to allow a line break.

[Malte Meyn, 2020-03-29 16:35:54]

On 2020/03/29 14:23:29, lemzwerg wrote:
> Minor doc nits.

I’ll include those fixes in the next patch set.

[Malte Meyn, 2020-04-01 08:20:24]

On 2020/03/29 16:35:54, Malte Meyn wrote:
> On 2020/03/29 14:23:29, lemzwerg wrote:
> > Minor doc nits.
> 
> I’ll include those fixes in the next patch set.

I killed my laptop using a glass of water and cannot contribute until it’s live
again. Below you can see a proposal for a regtest. I still will need help with
the convert-ly rule.

%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
\version "2.21.0"

\header {
  idontknowhowtonamethisdocstring =
  "Labels of line spanners, OttavaBrackets and VoltaBrackets
can be differently aligned for spanners below and above the
staff by overriding the property label-alignments."
}

\relative {
  \override Score.VoltaBracket.direction = #DOWN
  \override Score.VoltaBracketSpanner.direction = #DOWN
  \override TextSpanner.bound-details.left.text = "FOO"
  \override TextSpanner.bound-details.left.label-alignments = #(cons DOWN UP)
  \repeat volta 2 {
    \ottava 1
    c'''_\startTextSpan a f d\stopTextSpan
    \ottava -1
    c,^\startTextSpan a f d\stopTextSpan
    \ottava 0
  }
  \alternative {
    g'1
    g
  }
}
%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%